Re: [RFC v3 2/2] SUNRPC: Mask XIDs to prevent replay cache collision

From: NeilBrown <neilb@suse.com>
To: Bennett Amodio <bamodio@purestorage.com>, linux-nfs@vger.kernel.org
Cc: anna.schumaker@netapp.com, trond.myklebust@primarydata.com,
	Igor Ostrovsky <igor@purestorage.com>,
	Vas Chellappa <vas@purestorage.com>,
	Jui-Yu Chang <juchang@purestorage.com>
Subject: Re: [RFC v3 2/2] SUNRPC: Mask XIDs to prevent replay cache collision
Date: Thu, 17 Aug 2017 13:23:58 +1000	[thread overview]
Message-ID: <87pobunagh.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <CAKk7tABaTxA102tUHEuYyC8rtbNDjzHX6MdVcCkTy_26yfVkvA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Tue, Aug 15 2017, Bennett Amodio wrote:

> + xprt->xid_mask = 0xffffffff >> args->bitmask_len;
> + xprt->masked_id = args->transport_id << (32 - args->bitmask_len);

hi,
 the above isn't safe when bitmask_len is zero.
 Shifting a u32 32 bits to the left is undefined in C.

        The result is undefined if the right operand is negative, or
        greater than or equal to the number of bits in the left
        expression’s type.

 if (args->bitmask_len)
	xprt->masked_id = args->transport_id << (32 - args->bitmask_len);
 else
        xprt->masked_id = 0;

Thanks,
NeilBrown

> + xprt->xid = args->init_xid;
> +

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]