On Thu, 2022-03-31 at 09:47 +0800, Xiubo Li wrote:
> On 3/31/22 3:04 AM, 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.
> >
> > Cc: Luís Henriques <lhenriques@xxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/ceph/mds_client.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index 840a60b812fc..b03128fdbb07 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -3504,13 +3504,19 @@ 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) {
> > + iput(in);
>
> If the 'in' has a delegated ino, should we give it back here ?
>
> -- Xiubo
>
>
This really shouldn't be a delegated ino. We only grab a delegated ino
if we're doing an async create, and in that case we should know that the
dentry doesn't exist and the create will succeed or fail without opening
the file.
It's probably worth throwing a warning though if we ever _do_ get a
delegated ino here. Let me consider how best to catch that situation.
> > + in = NULL;
> > + }
> > +
> > + in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> > if (IS_ERR(in)) {
> > err = PTR_ERR(in);
> > mutex_lock(&session->s_mutex);
>
Thanks,
--
Jeff Layton <jlayton@xxxxxxxxxx>
