Re: [PATCH v5 03/11] nfsd: simplify foreign-filehandle handling to better match RFC-7862

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-11-19 at 14:28 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> When the COPY v4.2 op is used in the inter-server copy mode, the file
> handle of the source file (presented as the saved filehandle to COPY)
> is for a different ("foreign") file server which would not be expected
> to be understood by this server.  fh_verify() might return nfserr_stale
> or nfserr_badhandle.
> 
> In order of this filehandle to still be available to COPY, both PUTFH
> and SAVEFH much not trigger an error.
> 
> RFC 7862 section 15.2.3 says:
> 
>    If the request is for an inter-server copy, the source-fh is a
>    filehandle from the source server and the COMPOUND procedure is being
>    executed on the destination server.  In this case, the source-fh is a
>    foreign filehandle on the server receiving the COPY request.  If
>    either PUTFH or SAVEFH checked the validity of the filehandle, the
>    operation would likely fail and return NFS4ERR_STALE.
> 
>    If a server supports the inter-server copy feature, a PUTFH followed
>    by a SAVEFH MUST NOT return NFS4ERR_STALE for either operation.
>    These restrictions do not pose substantial difficulties for servers.
>    CURRENT_FH and SAVED_FH may be validated in the context of the
>    operation referencing them and an NFS4ERR_STALE error returned for an
>    invalid filehandle at that point.
> 
> [The RFC neglects the possibility of NFS4ERR_BADHANDLE]
> 
> Linux nfsd currently takes a different approach.  Rather than just
> checking for "a PUTFH followed by a SAVEFH", it goes further to consider
> only that case when this filehandle is then used for COPY, and allows
> that it might have been subject of RESTOREFH and SAVEFH in between.
> 
> This is not a problem in itself except for the extra code with little
> benefit.  This analysis of the COMPOUND to detect PUTFH ops which need
> care is performed on every COMPOUND request, which is not necessary.
> 
> It is sufficient to check if the relevant conditions are met only when a
> PUTFH op actually receives an error from fh_verify().
> 
> This patch removes the checking code from common paths and places it in
> nfsd4_putfh() only when fh_verify() returns a relevant error.
> 
> Rather than scanning ahead for a COPY, this patch notes (in
> nfsd4_compoundargs) when an inter-server COPY is decoded, and in that
> case only checks the next op to see if it is SAVEFH as this is what the
> RFC requires.
> 
> A test on "inter_copy_offload_enable" is also added to be completely
> consistent with the "If a server supports the inter-server copy feature"
> precondition.
> 
> As we do this test in nfsd4_putfh() there is no now need to mark the
> putfh op as "no_verify".
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/nfsd/nfs4proc.c | 64 ++++++++++++++++------------------------------
>  fs/nfsd/nfs4xdr.c  |  2 +-
>  fs/nfsd/xdr4.h     |  2 +-
>  3 files changed, 24 insertions(+), 44 deletions(-)
> 

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