On 2010-12-15 16:11, Fred Isaman wrote:
>
> On Dec 15, 2010, at 8:57 AM, Benny Halevy wrote:
>
>> On 2010-12-10 03:22, Fred Isaman wrote:
>>> It was checking that at least one known bit was set. It needs to check
>>> no unknown bit was set. From RFC5661, section 20.6.3:
>>>
>>> When a bit is set in the type mask that corresponds to an undefined
>>> type of recallable object, NFS4ERR_INVAL MUST be returned.
>>>
>>> Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx>
>>> ---
>>> fs/nfs/callback.h | 1 +
>>> fs/nfs/callback_proc.c | 27 ++++-----------------------
>>> 2 files changed, 5 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
>>> index b16dd1f..616c5c1 100644
>>> --- a/fs/nfs/callback.h
>>> +++ b/fs/nfs/callback.h
>>> @@ -126,6 +126,7 @@ extern int nfs41_validate_delegation_stateid(struct nfs_delegation *delegation,
>>> #define RCA4_TYPE_MASK_OBJ_LAYOUT_MAX 9
>>> #define RCA4_TYPE_MASK_OTHER_LAYOUT_MIN 12
>>> #define RCA4_TYPE_MASK_OTHER_LAYOUT_MAX 15
>>> +#define RCA4_TYPE_MASK_ALL 0xf31f
>>>
>>> struct cb_recallanyargs {
>>> struct sockaddr *craa_addr;
>>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>>> index 61b3c66..d4aec46 100644
>>> --- a/fs/nfs/callback_proc.c
>>> +++ b/fs/nfs/callback_proc.c
>>> @@ -661,28 +661,10 @@ out_putclient:
>>> goto out;
>>> }
>>>
>>> -static inline bool
>>> -validate_bitmap_values(const unsigned long *mask)
>>> +static bool
>>> +validate_bitmap_values(unsigned long mask)
>>> {
>>> - int i;
>>> -
>>> - if (*mask == 0)
>>> - return true;
>>> - if (test_bit(RCA4_TYPE_MASK_RDATA_DLG, mask) ||
>>> - test_bit(RCA4_TYPE_MASK_WDATA_DLG, mask) ||
>>> - test_bit(RCA4_TYPE_MASK_DIR_DLG, mask) ||
>>> - test_bit(RCA4_TYPE_MASK_FILE_LAYOUT, mask) ||
>>> - test_bit(RCA4_TYPE_MASK_BLK_LAYOUT, mask))
>>> - return true;
>>> - for (i = RCA4_TYPE_MASK_OBJ_LAYOUT_MIN;
>>> - i <= RCA4_TYPE_MASK_OBJ_LAYOUT_MAX; i++)
>>> - if (test_bit(i, mask))
>>> - return true;
>>> - for (i = RCA4_TYPE_MASK_OTHER_LAYOUT_MIN;
>>> - i <= RCA4_TYPE_MASK_OTHER_LAYOUT_MAX; i++)
>>> - if (test_bit(i, mask))
>>> - return true;
>>> - return false;
>>> + return mask & ~RCA4_TYPE_MASK_ALL;
>>
>> Hmm, shouldn't that be
>> return (mask & ~RCA4_TYPE_MASK_ALL) == 0;
>>
>> Benny
>>
>
> Yes, you are right.
OK. This is fixed in my branch to be released asap.
I've reverted large parts of this patchset in the post-submit stream
to restore layoutcommit and layoutreturn, but not their embedding in
the CLOSE compound. I also kept the cleanups and bug fixes.
I'll send out the post-submit when it's ready.
Some more work will be required to restore the original patches
author and signoffees.
This is the list as of now:
pick af44531 Revert "pnfs-submit: wave2: remove forgotten layoutreturn struct definitions"
pick c465549 Revert "pnfs-submit: Turn off layoutcommits"
pick 0f4ba67 Revert "pnfs-submit: wave2: remove all LAYOUTRETURN code"
pick 486db47 Revert "pnfs-submit: wave2: Remove LAYOUTRETURN from return on close"
pick 484c935 FIXME: roc should return layout on last close
(This patch just adds a FIXME comment.)
pick 8698772 Revert "pnfs-submit: wave2: remove cl_layoutrecalls list"
pick 263879b Revert "pnfs-submit: wave2: Pull out all recall initiated LAYOUTRETURNS"
pick 693765f Revert "pnfs-submit: wave2: Don't wait in layoutget"
pick de56e11 Revert "pnfs-submit: wave2: check that partial LAYOUTGET return is ignored"
Anything else you had in mind?
Benny
>
> Fred
>
>>> }
>>>
>>> __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy,
>>> @@ -702,8 +684,7 @@ __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy,
>>> rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR));
>>>
>>> status = cpu_to_be32(NFS4ERR_INVAL);
>>> - if (!validate_bitmap_values((const unsigned long *)
>>> - &args->craa_type_mask))
>>> + if (!validate_bitmap_values(args->craa_type_mask))
>>> goto out;
>>>
>>> status = cpu_to_be32(NFS4_OK);
>
> --
> 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
--
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
