Re: [PATCH RFCv3] NFS: Fix an LOCK/OPEN race when unlinking an open file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chuck,

Any news about this patch?

On Thu, Mar 31, 2016 at 6:40 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
>> On Mar 31, 2016, at 12:38 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>>
>> At Connectathon 2016, we found that recent upstream Linux clients
>> would occasionally send a LOCK operation with a zero stateid. This
>> appeared to happen in close proximity to another thread returning
>> a delegation before unlinking the same file while it remained open.
>>
>> Earlier, the client received a write delegation on this file and
>> returned the open stateid. Now, as it is getting ready to unlink the
>> file, it returns the write delegation. But there is still an open
>> file descriptor on that file, so the client must OPEN the file
>> again before it returns the delegation.
>>
>> Since commit 24311f884189 ('NFSv4: Recovery of recalled read
>> delegations is broken'), nfs_open_delegation_recall() clears the
>> NFS_DELEGATED_STATE flag _before_ it sends the OPEN. This allows a
>> racing LOCK on the same inode to be put on the wire before the OPEN
>> operation has returned a valid open stateid.
>>
>> To eliminate this race, serialize delegation return with the
>> acquisition of a file lock on the same file. Adopt the same approach
>> as is used in the unlock path.
>>
>> Fixes: 24311f884189 ('NFSv4: Recovery of recalled read ... ')
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>
> Previously reported issues have been addressed.
>
>
>> fs/nfs/nfs4proc.c |    4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 01bef06..c40f1b6 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -6054,6 +6054,7 @@ static int nfs41_lock_expired(struct nfs4_state *state, struct file_lock *reques
>> static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
>> {
>>       struct nfs_inode *nfsi = NFS_I(state->inode);
>> +     struct nfs4_state_owner *sp = state->owner;
>>       unsigned char fl_flags = request->fl_flags;
>>       int status = -ENOLCK;
>>
>> @@ -6068,6 +6069,7 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
>>       status = do_vfs_lock(state->inode, request);
>>       if (status < 0)
>>               goto out;
>> +     mutex_lock(&sp->so_delegreturn_mutex);
>>       down_read(&nfsi->rwsem);
>>       if (test_bit(NFS_DELEGATED_STATE, &state->flags)) {
>>               /* Yes: cache locks! */
>> @@ -6075,9 +6077,11 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
>>               request->fl_flags = fl_flags & ~FL_SLEEP;
>>               status = do_vfs_lock(state->inode, request);
>>               up_read(&nfsi->rwsem);
>> +             mutex_unlock(&sp->so_delegreturn_mutex);
>>               goto out;
>>       }
>>       up_read(&nfsi->rwsem);
>> +     mutex_unlock(&sp->so_delegreturn_mutex);
>>       status = _nfs4_do_setlk(state, cmd, request, NFS_LOCK_NEW);
>> out:
>>       request->fl_flags = fl_flags;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
William
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux