Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
Trond Myklebust <[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
On Sat, 2026-08-08 at 12:34 -0400, Trond Myklebust wrote:
> 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-8Du6NiZp2BlWk0Htik3J/[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.
Actually, "if (!d_is_reg(dentry))" is even better.
>
> > + 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]