Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 23 Nov 2020 14:18:40 -0500
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH v2 005/118] NFSD: Replace the internals of the READ_BUF()
 macro
Message-ID: <20201123191840.GH32599@fieldses.org>
References: <160590425404.1340.8850646771948736468.stgit@klimt.1015granger.net>
 <160590445271.1340.9408337302317384948.stgit@klimt.1015granger.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <160590445271.1340.9408337302317384948.stgit@klimt.1015granger.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
From:   bfields@fieldses.org (J. Bruce Fields)
Precedence: bulk

On Fri, Nov 20, 2020 at 03:34:12PM -0500, Chuck Lever wrote:
> @@ -396,7 +281,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
>  		READ_BUF(4); len += 4;
>  		nace = be32_to_cpup(p++);
>  
> -		if (nace > compoundargs_bytes_left(argp)/20)
> +		if (nace > xdr_stream_remaining(argp->xdr) / sizeof(struct nfs4_ace))

Picky C question: is the compiler guaranteed to pack that struct in the
obvious way?

That aside, I'm not comfortable assuming the struct could never change
to, say, include something that's useful during processing but doesn't
appear on the wire.

Also, that change isn't actually logically related to the rest of the
patch, so it's the sort of thing I'd expect separated out.

--b.