All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@kernel.org>
To: Michael Nemanov <michael.nemanov@vastdata.com>,
	anna@kernel.org,  neil@brown.name
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
Date: Sat, 08 Aug 2026 12:34:57 -0400	[thread overview]
Message-ID: <c887ed23d456542f97168ffeda4abb5d55ba0fa6.camel@kernel.org> (raw)
In-Reply-To: <20260806131358.398096-1-michael.nemanov@vastdata.com>

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/20260614122911.3485467-1-michael.nemanov@vastdata.com/
>  (v1)
> Signed-off-by: Michael Nemanov <michael.nemanov@vastdata.com>
> Tested-by: Michael Nemanov <michael.nemanov@vastdata.com>
> ---
>  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
trondmy@kernel.org, trond.myklebust@hammerspace.com

  reply	other threads:[~2026-08-08 16:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 13:13 [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3 Michael Nemanov
2026-08-08 16:34 ` Trond Myklebust [this message]
2026-08-08 16:53   ` Trond Myklebust
2026-08-10  7:30     ` Michael Nemanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c887ed23d456542f97168ffeda4abb5d55ba0fa6.camel@kernel.org \
    --to=trondmy@kernel.org \
    --cc=anna@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=michael.nemanov@vastdata.com \
    --cc=neil@brown.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.