From: Andy Adamson <[email protected]>
_nfs41_proc_sequence() kmallocs the calldata but does not set
calldata->res.slotid to NFS4_MAX_SLOT_TABLE, so nfs41_sequence_prepare() calls
nfs41_setup_sequence() with the res.sr_slotid set to a random value.
If nfs41_setup_sequence() sees res.sr_slotid as not set to NFS4_MAX_SLOT_TABLE,
it returns without actually setting up the sequence.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 04eff86..243d6c7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5142,6 +5142,7 @@ static struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp, struct rpc_
nfs_put_client(clp);
return ERR_PTR(-ENOMEM);
}
+ calldata->res.sr_slotid = NFS4_MAX_SLOT_TABLE;
msg.rpc_argp = &calldata->args;
msg.rpc_resp = &calldata->res;
calldata->clp = clp;
--
1.6.6
On Tue, 2010-06-15 at 18:31 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> _nfs41_proc_sequence() kmallocs the calldata but does not set
> calldata->res.slotid to NFS4_MAX_SLOT_TABLE, so nfs41_sequence_prepare() calls
> nfs41_setup_sequence() with the res.sr_slotid set to a random value.
> If nfs41_setup_sequence() sees res.sr_slotid as not set to NFS4_MAX_SLOT_TABLE,
> it returns without actually setting up the sequence.
Looks like a typo in my patch "NFSv41: Fix a memory leak in
nfs41_proc_async_sequence()". I'll squash your patch into that. Please
note that you will have to rebase if you try to pull the nfs-for-2.6.36
branch again.
Now.... With that said, do we really need to allow RPC calls to call
nfs41_setup_sequence with an initialised slotid? This initialisation is
a serious hassle, and is easy to forget.
When do we expect to end up hitting the res.sr_slotid !=
NFS4_MAX_SLOT_TABLE condition in real life?
Cheers
Trond
On Wed, Jun 16, 2010 at 9:41 AM, Trond Myklebust
<[email protected]> wrote:
> On Tue, 2010-06-15 at 18:31 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> _nfs41_proc_sequence() kmallocs the calldata but does not set
>> calldata->res.slotid to NFS4_MAX_SLOT_TABLE, so nfs41_sequence_prepare() calls
>> nfs41_setup_sequence() with the res.sr_slotid set to a random value.
>> If nfs41_setup_sequence() sees res.sr_slotid as not set to NFS4_MAX_SLOT_TABLE,
>> it returns without actually setting up the sequence.
>
> Looks like a typo in my patch "NFSv41: Fix a memory leak in
> nfs41_proc_async_sequence()". I'll squash your patch into that. Please
> note that you will have to rebase if you try to pull the nfs-for-2.6.36
> branch again.
>
> Now.... With that said, do we really need to allow RPC calls to call
> nfs41_setup_sequence with an initialised slotid?
nfs41_setup_sequence needs to know when a slot has already been
initialized which happens when the error handlers resend.
> This initialisation is
> a serious hassle, and is easy to forget.
Agreed. I'll make it easy to remember by coding an initialization
helper function.
> When do we expect to end up hitting the res.sr_slotid !=
> NFS4_MAX_SLOT_TABLE condition in real life?
What we don't expect in real life is for session->max_req ==
NFS4_MAX_SLOT_TABLE, unless it is not initialized.
A valid slotid is any value between 0 and session->max_req e.g. slotid
!= NFS4_MAX_SLOT_TABLE, so we hit that condition every valid SEQUENCE
op.
-->Andy
>
> Cheers
> Trond
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>