Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3

On Thu, 2026-08-06 at 13:13 +0000, Michael Nemanov wrote:
> When open(2) is called with O_CREAT on a path that already exists as
> a
> symlink, over an NFSv3 mount with a cold dcache, the kernel returns
> ENXIO instead of following the symlink to its target.
> 
> Reproducer script (MNT is an NFSv3 mount, kernel is 7.1-rc6):
> 
> MNT=/mnt/export
> ln -sf /tmp/target $MNT/mylink
> echo 3 | sudo tee /proc/sys/vm/drop_caches   # cold dcache
> 
> python3 - <<'EOF'
> import os
> fd = os.open('/mnt/export/mylink', os.O_WRONLY | os.O_CREAT |
> os.O_APPEND, 0o666)
> os.close(fd)
> EOF
> 
> Expected: success (follow symlink, open target)
> Actual:   OSError: [Errno 6] No such device or address
> 
> The bug does not trigger when the dcache is warm (e.g. after a prior
> stat(2)), because lookup_open() then finds a positive dentry and
> skips
> atomic_open entirely, leaving symlink resolution to the VFS.
> 
> Root cause:
> nfs_atomic_open_v23(), registered as inode->i_op->atomic_open for
> NFSv3, handles O_CREAT by sending a CREATE UNCHECKED RPC. As
> implemented in nfsd3_create_file() (fs/nfsd/nfs3proc.c) and as
> required
> by RFC 1813 (3.3.8), when the name already exists as a non-regular
> file
> the server returns NFS3_OK with the existing object's file handle
> rather
> than NFS3ERR_EXIST causing nfs_do_create() to return 0 with the
> dentry now pointing to a symlink.
> The code then unconditionally calls finish_open(), which dispatches
> through inode->i_fop->open(). Symlink inodes never have i_fop set —
> the
> VFS initialises it to &no_open_fops because POSIX requires open(2) to
> follow symlinks, never open them directly. no_open() returns -ENXIO.
> 
> Fix:
> After nfs_do_create() succeeds, verify the returned inode is a
> regular
> file before calling finish_open(). If the object is not regular,
> return
> finish_no_open(file, NULL) so the VFS follows the symlink through the
> normal open path. NULL is passed because nfs_do_create() instantiates
> the inode on the dentry already owned by the caller; passing dentry
> back
> would cause atomic_open() to dput() it a second time.
> !S_ISREG() is used rather than S_ISLNK() to cover any other non-
> regular
> types a server might return.
> 
> Changes in v2:
> - Pass NULL to finish_no_open() per Trond's feedback.
> 
> Fixes: 7c6c5249f061 ("NFS: add atomic_open for NFSv3 to handle
> O_TRUNC correctly.")
> Link:
> https://lore.kernel.org/linux-nfs/[email protected]/
>  (v1)
> Signed-off-by: Michael Nemanov <michael.nemanov-8Du6NiZp2BlWk0Htik3J/[email protected]>
> Tested-by: Michael Nemanov <michael.nemanov-8Du6NiZp2BlWk0Htik3J/[email protected]>
> ---
>  fs/nfs/dir.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index e9ce1883288c5..c3481d9c74334 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -2317,6 +2317,13 @@ int nfs_atomic_open_v23(struct inode *dir,
> struct dentry *dentry,
>  	if (open_flags & O_CREAT) {
>  		error = nfs_do_create(dir, dentry, mode,
> open_flags);
>  		if (!error) {
> +			/* With UNCHECKED mode, a server may return
> NFS3_OK for
> +			 * a pre-existing non-regular file (e.g. a
> symlink).
> +			 * Let the VFS handle it; calling
> finish_open() would
> +			 * hit no_open() and return -ENXIO.
> +			 */
> +			if (d_inode(dentry) &&
> !S_ISREG(d_inode(dentry)->i_mode))

Hmm... I'm changing the above to "if (!d_inode(dentry) ||
!S_ISREG(....))" so that we don't inadvertently try to open a negative
dentry. That can happen with NFSv2 if we hit an existing directory that
has been renamed on the server.

> +				return finish_no_open(file, NULL);
>  			file->f_mode |= FMODE_CREATED;
>  			return finish_open(file, dentry, NULL);
>  		} else if (error != -EEXIST || open_flags & O_EXCL)

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected], trond.myklebust-F/[email protected]