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. 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