Re: [PATCH] ceph: discard r_new_inode if open O_CREAT opened existing inode

["Luís Henriques" <lhenriques@suse.de>]

Jeff Layton <jlayton@kernel.org> writes:

> On Wed, 2022-03-30 at 15:04 -0400, 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@suse.de>
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>>  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);
>> +			in = NULL;
>> +		}
>> +
>> +		in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
>>  		if (IS_ERR(in)) {
>>  			err = PTR_ERR(in);
>>  			mutex_lock(&session->s_mutex);
>
> Forgot to mention that this one is for the fscrypt pile. This patch
> fixes generic/595 for me.

Wow!  I must say it: the issue is very far from the places I was looking
at.  Thanks!

Reviewed-and-tested-by: Luís Henriques <lhenriques@suse.de>

Cheers,
-- 
Luís