Re: [PATCH v3 0/4] Avoid returning NFS4ERR_FILE_OPEN when not appropriate

[Jeff Layton <jlayton@kernel.org>]

On Sun, 2025-01-26 at 16:50 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> This short series aims to prevent NFSD from returning
> NFS4ERR_FILE_OPEN when an NFSv4 LINK, RENAME, or REMOVE operation
> targets a directory.  The only time the protocol spec permits a
> server to return FILE_OPEN is when the target of the operation is an
> object that is open and cannot be closed immediately to satisfy the
> request.
> 
> I would have preferred these fixes go into NFSv4-specific sections
> of NFSD, but the current structure of the code prevents doing that
> while maintaining operational efficiency. Plus, these small patches
> should be able to apply cleanly to LTS kernels.
> 
> We can defer deeper restructuring for later. For example,
> fh_verify() could be made to return an errno instead of a generic
> NFS status code; then the VFS utility functions in fs/nfsd/vfs.c
> could be made to do the same, making their callers responsible for
> the proper NFS version-specific translation of the errno into a
> status code.
>
> This series has passed git regression and xfstests. I'm interested
> in review and comment about this approach, but please do test these
> if you have the ability to trigger -EBUSY easily.
> 
> NFSv4 OPEN is also affected, but because of its complexity will
> require careful audit (ie, a separate patch set). Please send a copy
> of the output of WARN_ONCE so we can see where to start digging in
> that area.
> 
> Changes since v2:
> - Fix crash when renaming to a non-existent object
> 
> Changes since RFC:
> - Address a minor code odor
> - Clarify some code comments
> 
> Chuck Lever (4):
>   NFSD: nfsd_unlink() clobbers non-zero status returned from
>     fh_fill_pre_attrs()
>   NFSD: Never return NFS4ERR_FILE_OPEN when removing a directory
>   NFSD: Return NFS4ERR_FILE_OPEN only when renaming over an open file
>   NFSD: Return NFS4ERR_FILE_OPEN only when linking an open file
> 
>  fs/nfsd/vfs.c | 105 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 79 insertions(+), 26 deletions(-)
> 

This all looks pretty reasonable to me.

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