| Message ID | 20220331095931.6261-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ceph: discard r_new_inode if open O_CREAT opened existing inode | expand |
On 3/31/22 5:59 PM, Jeff Layton wrote: > When we do an unchecked create, we optimistically pre-create an inode > and populate it, including its fscrypt context. It's possible though > that we'll end up opening an existing inode, in which case the > precreated inode will have a crypto context that doesn't match the > existing data. > > If we're issuing an O_CREAT open and find an existing inode, just > discard the precreated inode and create a new one to ensure the context > is properly set. > > Also, we should never end up opening an existing file on an async > create, since we should know that the dentry doesn't exist. Throw a > warning if that ever does occur. > > Cc: Luís Henriques <lhenriques@suse.de> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/mds_client.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > v2: WARN if this ever happens on an async create > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 840a60b812fc..b3cf3a22fa2a 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -3504,13 +3504,21 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) > /* Must find target inode outside of mutexes to avoid deadlocks */ > rinfo = &req->r_reply_info; > if ((err >= 0) && rinfo->head->is_target) { > - struct inode *in; > + struct inode *in = xchg(&req->r_new_inode, NULL); > struct ceph_vino tvino = { > .ino = le64_to_cpu(rinfo->targeti.in->ino), > .snap = le64_to_cpu(rinfo->targeti.in->snapid) > }; > > - in = ceph_get_inode(mdsc->fsc->sb, tvino, xchg(&req->r_new_inode, NULL)); > + /* If we ended up opening an existing inode, discard r_new_inode */ > + if (req->r_op == CEPH_MDS_OP_CREATE && !req->r_reply_info.has_create_ino) { > + /* This should never happen on an async create */ > + WARN_ON_ONCE(req->r_deleg_ino); > + iput(in); > + in = NULL; > + } > + > + in = ceph_get_inode(mdsc->fsc->sb, tvino, in); > if (IS_ERR(in)) { > err = PTR_ERR(in); > mutex_lock(&session->s_mutex); LGTM. Reviewed-by: Xiubo Li <xiubli@redhat.com>
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 840a60b812fc..b3cf3a22fa2a 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -3504,13 +3504,21 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg) /* Must find target inode outside of mutexes to avoid deadlocks */ rinfo = &req->r_reply_info; if ((err >= 0) && rinfo->head->is_target) { - struct inode *in; + struct inode *in = xchg(&req->r_new_inode, NULL); struct ceph_vino tvino = { .ino = le64_to_cpu(rinfo->targeti.in->ino), .snap = le64_to_cpu(rinfo->targeti.in->snapid) }; - in = ceph_get_inode(mdsc->fsc->sb, tvino, xchg(&req->r_new_inode, NULL)); + /* If we ended up opening an existing inode, discard r_new_inode */ + if (req->r_op == CEPH_MDS_OP_CREATE && !req->r_reply_info.has_create_ino) { + /* This should never happen on an async create */ + WARN_ON_ONCE(req->r_deleg_ino); + iput(in); + in = NULL; + } + + in = ceph_get_inode(mdsc->fsc->sb, tvino, in); if (IS_ERR(in)) { err = PTR_ERR(in); mutex_lock(&session->s_mutex);
When we do an unchecked create, we optimistically pre-create an inode and populate it, including its fscrypt context. It's possible though that we'll end up opening an existing inode, in which case the precreated inode will have a crypto context that doesn't match the existing data. If we're issuing an O_CREAT open and find an existing inode, just discard the precreated inode and create a new one to ensure the context is properly set. Also, we should never end up opening an existing file on an async create, since we should know that the dentry doesn't exist. Throw a warning if that ever does occur. Cc: Luís Henriques <lhenriques@suse.de> Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/mds_client.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) v2: WARN if this ever happens on an async create