Re: [PATCH v5 01/11] nfsd: rename ALLOWED_WITHOUT_FH to ALLOWED_WITHOUT_LOCAL_FH and revise use

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-11-19 at 14:28 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> nfsdv4 ops which do not have ALLOWED_WITHOUT_FH can assume that a PUTFH
> has been called and may assume that current_fh->fh_dentry is non-NULL.
> nfsd4_setattr(), for example, assumes fh_dentry != NULL.
> 
> However the possibility of foreign filehandles (needed for v4.2 COPY)
> means that there maybe a filehandle present while fh_dentry is NULL.
> 
> Sending a COMPOUND containing:
>    SEQUENCE
>    PUTFH - foreign filehandle
>    SETATTR - new mode
>    SAVEFH
>    COPY - with non-empty server list
> 
> to an NFS server with inter-server copy enabled will cause a NULL
> pointer dereference when nfsd4_setattr() calls fh_want_write().
> 
> Most NFSv4 ops actually want a "local" filehandle, not just any
> filehandle.  So this patch renames ALLOWED_WITHOUT_FH to
> ALLOWED_WITHOUT_LOCAL_FH and sets it on those which don't require a local
> filehandle.  That is all that don't require any filehandle together with
> SAVEFH, which is the only OP which needs to handle a foreign current_fh.
> (COPY must handle a foreign save_fh, but all ops which access save_fh
> already do any required validity tests themselves).
> 
> nfsd4_savefh() is changed to validate the filehandle itself as the
> caller no longer validates it.
> 
> nfsd4_proc_compound no longer allows ops without
> ALLOWED_WITHOUT_LOCAL_FH to be called with a foreign fh - current_fh
> must be local and ->fh_dentry must be non-NULL.  This protects
> nfsd4_setattr() and any others that might use ->fh_dentry without
> checking.
> 
> The
>        current_fh->fh_export &&
> test is removed from an "else if" because that condition is now only
> tested when current_fh->fh_dentry is not NULL, and in that case
> current_fh->fh_export is also guaranteed to not be NULL.
> 
> Fixes: b9e8638e3d9e ("NFSD: allow inter server COPY to have a STALE source server fh")
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/nfsd/nfs4proc.c | 58 ++++++++++++++++++++++++++--------------------
>  fs/nfsd/xdr4.h     |  2 +-
>  2 files changed, 34 insertions(+), 26 deletions(-)
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>