Re: [PATCH 1/3] NFSv4: Fix a slot leak in nfs40_sequence_done

Trond Myklebust
2014-01-30T17:06:16Z

On Jan 30, 2014, at 12:03, Trond Myklebust <trond.myklebust@primarydata.com> wrote:

>
> On Jan 30, 2014, at 12:02, Chuck Lever <chuck.lever@oracle.com> wrote:
>
>>
>> On Jan 30, 2014, at 8:55 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>
>>>
>>> On Jan 30, 2014, at 11:49, Chuck Lever <chuck.lever@oracle.com> wrote:
>>>
>>>>
>>>> On Jan 29, 2014, at 9:36 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
>>>>
>>>>> The check for whether or not we sent an RPC call in nfs40_sequence_done
>>>>> is insufficient to decide whether or not we are holding a session slot,
>>>>> and thus should not be used to decide when to free that slot.
>>>>>
>>>>> This patch replaces the RPC_WAS_SENT() test with the correct test for
>>>>> whether or not slot == NULL.
>>>>>
>>>>> Cc: Chuck Lever <chuck.lever@oracle.com>
>>>>> Cc: stable@vger.kernel.org # 3.12+
>>>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>>>>> ---
>>>>> fs/nfs/nfs4proc.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>> index ae00c3ed733f..493e9cce1f11 100644
>>>>> --- a/fs/nfs/nfs4proc.c
>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>> @@ -539,7 +539,7 @@ static int nfs40_sequence_done(struct rpc_task *task,
>>>>> struct nfs4_slot *slot = res->sr_slot;
>>>>> struct nfs4_slot_table *tbl;
>>>>>
>>>>> - if (!RPC_WAS_SENT(task))
>>>>> + if (slot == NULL)
>>>>> goto out;
>>>>
>>>> When CONFIG_NFS_V4_1 is enabled, nfs4_sequence_done() already does the slot == NULL test, though the other nfs40_sequence_done() call sites do not. This patch should clean that up?
>>>
>>> Unfortunately we can’t touch nfs4_sequence_done: it wants to test for whether or not a NFSv4.1 session exists, which requires it to look at slot->table->session. :-(
>>
>> Move the slot == NULL check to all nfs40_sequence_done() call sites? <shrug>
>
> No...

We could move the entire function minus the slot == NULL test into a helper that could be called by nfs4_sequence_done, but I’m not convinced that particular micro-optimisation is really worth it.

--
Trond Myklebust
Linux NFS client maintainer