Re: [PATCH 1/2] nfsd: fix type mismatch and explain host-endian xdr buffer usage

On Mon, Aug 3, 2026, at 10:49 AM, Jeff Layton wrote:
> sparse flagged this type mismatch. Also the xdr buffer usage is a bit
> non-standard, so explain what we're doing and why.
>
> Fixes: 4b64b811f368 ("nfsd: add notification handlers for dir events")
> Reported-by: kernel test robot <[email protected]>
> Closes: 
> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Jeff Layton <[email protected]>
> ---
>  fs/nfsd/nfs4xdr.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index a47eb544b99f..5cbb4a415384 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -4383,13 +4383,18 @@ nfsd4_setup_notify_entry4(struct notify_entry4 
> *ne, struct xdr_stream *xdr,
>  	struct path path = nf->nf_file->f_path;
>  	struct nfsd4_fattr_args args = { };
>  	const u32 *reqmask;
> -	uint32_t *attrmask;
> +	u32 *attrmask;
>  	__be32 status;
>  	bool parent;
>  	int ret;
> 
> -	/* Reserve space for attrmask */
> -	attrmask = xdr_reserve_space(xdr, 3 * sizeof(uint32_t));
> +	/*
> +	 * attrmask is the host-order bmval[3] that nfsd4_encode_attr_vals()
> +	 * consumes and that ne_attrs.attrmask.element points at. It must
> +	 * outlive this call, so steal a few words from the xdr stream to hold
> +	 * it.
> +	 */
> +	attrmask = (u32 *)xdr_reserve_space(xdr, 3 * sizeof(*attrmask));
>  	if (!attrmask)
>  		return false;

I still don't understand what's going on. "Steal a few words to hold it"
sounds like you're trying to avoid a kmalloc call. Is this code reserving
space in the XDR stream, or isn't it? If it is, then accessing those bytes
has to be done with a big-endian pointer, the same way it is done at every
other xdr_reserve_space() call site. Storing host-order bytes that are not
part of an opaque into an XDR stream is just... wrong.

If you're doing the usual "reserve and backfill the encoded data later"
dance, then spell it the way everyone else does so us poor human readers
can recognize that's what's going on.


-- 
Chuck Lever