All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
@ 2026-08-06 13:13 Michael Nemanov
  2026-08-08 16:34 ` Trond Myklebust
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Nemanov @ 2026-08-06 13:13 UTC (permalink / raw)
  To: trondmy, anna, neil; +Cc: linux-nfs, Michael Nemanov

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))
+				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)
-- 
2.43.7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
  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
  2026-08-08 16:53   ` Trond Myklebust
  0 siblings, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2026-08-08 16:34 UTC (permalink / raw)
  To: Michael Nemanov, anna, neil; +Cc: linux-nfs

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
  2026-08-08 16:34 ` Trond Myklebust
@ 2026-08-08 16:53   ` Trond Myklebust
  2026-08-10  7:30     ` Michael Nemanov
  0 siblings, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2026-08-08 16:53 UTC (permalink / raw)
  To: Michael Nemanov, anna, neil; +Cc: linux-nfs

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@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.

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
trondmy@kernel.org, trond.myklebust@hammerspace.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs: fix ENXIO on O_CREAT open of existing symlink over NFSv3
  2026-08-08 16:53   ` Trond Myklebust
@ 2026-08-10  7:30     ` Michael Nemanov
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Nemanov @ 2026-08-10  7:30 UTC (permalink / raw)
  To: Trond Myklebust, anna, neil; +Cc: linux-nfs



On 08/08/2026 19:53, Trond Myklebust wrote:
> On Sat, 2026-08-08 at 12:34 -0400, Trond Myklebust wrote:
>> On Thu, 2026-08-06 at 13:13 +0000, Michael Nemanov wrote:
...
>>> 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.
> 

Agreed. Would you like a v3 with this fix?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-10  7:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-08-08 16:53   ` Trond Myklebust
2026-08-10  7:30     ` Michael Nemanov

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.