[PATCH AUTOSEL 6.1 3/9] NFSv4: Always set NLINK even if the server doesn't support it

[PATCH AUTOSEL 6.1 3/9] NFSv4: Always set NLINK even if the server doesn't support it

[Sasha Levin]

From: Han Young <hanyang.tony@bytedance.com>

[ Upstream commit 3a3065352f73381d3a1aa0ccab44aec3a5a9b365 ]

fattr4_numlinks is a recommended attribute, so the client should emulate
it even if the server doesn't support it. In decode_attr_nlink function
in nfs4xdr.c, nlink is initialized to 1. However, this default value
isn't set to the inode due to the check in nfs_fhget.

So if the server doesn't support numlinks, inode's nlink will be zero,
the mount will fail with error "Stale file handle". Set the nlink to 1
if the server doesn't support it.

Signed-off-by: Han Young <hanyang.tony@bytedance.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis of the commit and the kernel source code, here is
my determination: **YES** This commit should be backported to stable
kernel trees. Here's my extensive analysis: ## Critical Bug Fix Analysis
### 1. **Root Cause Understanding** The commit addresses a critical
issue where NFSv4 mounts fail with "Stale file handle" errors when the
server doesn't support the `fattr4_numlinks` (NLINK) attribute. Looking
at the code: - In `/home/sasha/linux/fs/nfs/nfs4xdr.c:3969`, the
`decode_attr_nlink` function initializes `*nlink = 1` as a default -
However, in `/home/sasha/linux/fs/nfs/inode.c:556-559`, the current
logic only sets the inode's nlink if `fattr->valid &
NFS_ATTR_FATTR_NLINK` is true - When the server doesn't support
numlinks, the `fattr->valid` flag isn't set, so the inode's nlink
remains 0 (from initial inode allocation) ### 2. **Impact of Zero
nlink** From `/home/sasha/linux/fs/nfs/dir.c:1578-1582`, I can see the
critical check: ```c if (inode->i_nlink > 0 || (inode->i_nlink == 0 &&
test_bit(NFS_INO_PRESERVE_UNLINKED, &NFS_I(inode)->flags))) return 0;
else return -ESTALE; ``` When `i_nlink == 0` and the
`NFS_INO_PRESERVE_UNLINKED` flag isn't set, NFS returns `-ESTALE`,
causing "Stale file handle" errors and mount failures. ### 3. **Backport
Suitability Criteria** **✅ Fixes important user-affecting bug**: Mount
failures prevent basic filesystem functionality **✅ Small and contained
fix**: The change adds only 2 lines: ```c else if (fattr_supported &
NFS_ATTR_FATTR_NLINK) nfs_set_cache_invalid(inode,
NFS_INO_INVALID_NLINK); +else + set_nlink(inode, 1); ``` **✅ Minimal
regression risk**: Setting nlink=1 as fallback is safe and follows the
NFSv4 spec (fattr4_numlinks is a "recommended" attribute, so emulation
is appropriate) **✅ No architectural changes**: This is a targeted fix
within existing error handling logic **✅ Follows stable tree rules**:
It's an important bugfix with minimal risk ### 4. **Comparison with
Similar Commits** Unlike the provided similar commits that were marked
"NO" (which involved more complex nlink management, cache invalidation
changes, or feature additions), this commit: - Fixes a fundamental
compatibility issue - Prevents complete mount failures - Is minimal and
defensive - Addresses server interoperability problems ### 5. **Server
Compatibility** This fix ensures NFSv4 clients remain functional with
servers that don't fully implement all recommended attributes, which is
critical for real-world deployments where server implementations vary.
The commit directly prevents mount failures that would otherwise require
server-side fixes or workarounds, making it essential for stable kernel
trees where users expect reliable NFS functionality.

 fs/nfs/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index f2e66b946f4b4..e774cfc85eeed 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -555,6 +555,8 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 			set_nlink(inode, fattr->nlink);
 		else if (fattr_supported & NFS_ATTR_FATTR_NLINK)
 			nfs_set_cache_invalid(inode, NFS_INO_INVALID_NLINK);
+		else
+			set_nlink(inode, 1);
 		if (fattr->valid & NFS_ATTR_FATTR_OWNER)
 			inode->i_uid = fattr->uid;
 		else if (fattr_supported & NFS_ATTR_FATTR_OWNER)
-- 
2.39.5

Re: [PATCH AUTOSEL 6.1 3/9] NFSv4: Always set NLINK even if the server doesn't support it

[Lionel Cons]

On Fri, 6 Jun 2025 at 17:48, Sasha Levin <sashal@kernel.org> wrote:
>
> From: Han Young <hanyang.tony@bytedance.com>
>
> [ Upstream commit 3a3065352f73381d3a1aa0ccab44aec3a5a9b365 ]
>
> fattr4_numlinks is a recommended attribute, so the client should emulate
> it even if the server doesn't support it. In decode_attr_nlink function
> in nfs4xdr.c, nlink is initialized to 1. However, this default value
> isn't set to the inode due to the check in nfs_fhget.
>
> So if the server doesn't support numlinks, inode's nlink will be zero,
> the mount will fail with error "Stale file handle". Set the nlink to 1
> if the server doesn't support it.
>
> Signed-off-by: Han Young <hanyang.tony@bytedance.com>
> Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> Based on my analysis of the commit and the kernel source code, here is
> my determination: **YES** This commit should be backported to stable
> kernel trees. Here's my extensive analysis: ## Critical Bug Fix Analysis
> ### 1. **Root Cause Understanding** The commit addresses a critical
> issue where NFSv4 mounts fail with "Stale file handle" errors when the
> server doesn't support the `fattr4_numlinks` (NLINK) attribute. Looking
> at the code: - In `/home/sasha/linux/fs/nfs/nfs4xdr.c:3969`, the
> `decode_attr_nlink` function initializes `*nlink = 1` as a default -
> However, in `/home/sasha/linux/fs/nfs/inode.c:556-559`, the current
> logic only sets the inode's nlink if `fattr->valid &
> NFS_ATTR_FATTR_NLINK` is true - When the server doesn't support
> numlinks, the `fattr->valid` flag isn't set, so the inode's nlink
> remains 0 (from initial inode allocation) ### 2. **Impact of Zero
> nlink** From `/home/sasha/linux/fs/nfs/dir.c:1578-1582`, I can see the
> critical check: ```c if (inode->i_nlink > 0 || (inode->i_nlink == 0 &&
> test_bit(NFS_INO_PRESERVE_UNLINKED, &NFS_I(inode)->flags))) return 0;
> else return -ESTALE; ``` When `i_nlink == 0` and the
> `NFS_INO_PRESERVE_UNLINKED` flag isn't set, NFS returns `-ESTALE`,
> causing "Stale file handle" errors and mount failures. ### 3. **Backport
> Suitability Criteria** **✅ Fixes important user-affecting bug**: Mount
> failures prevent basic filesystem functionality **✅ Small and contained
> fix**: The change adds only 2 lines: ```c else if (fattr_supported &
> NFS_ATTR_FATTR_NLINK) nfs_set_cache_invalid(inode,
> NFS_INO_INVALID_NLINK); +else + set_nlink(inode, 1); ``` **✅ Minimal
> regression risk**: Setting nlink=1 as fallback is safe and follows the
> NFSv4 spec (fattr4_numlinks is a "recommended" attribute, so emulation
> is appropriate) **✅ No architectural changes**: This is a targeted fix
> within existing error handling logic **✅ Follows stable tree rules**:
> It's an important bugfix with minimal risk ### 4. **Comparison with
> Similar Commits** Unlike the provided similar commits that were marked
> "NO" (which involved more complex nlink management, cache invalidation
> changes, or feature additions), this commit: - Fixes a fundamental
> compatibility issue - Prevents complete mount failures - Is minimal and
> defensive - Addresses server interoperability problems ### 5. **Server
> Compatibility** This fix ensures NFSv4 clients remain functional with
> servers that don't fully implement all recommended attributes, which is
> critical for real-world deployments where server implementations vary.
> The commit directly prevents mount failures that would otherwise require
> server-side fixes or workarounds, making it essential for stable kernel
> trees where users expect reliable NFS functionality.
>
>  fs/nfs/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index f2e66b946f4b4..e774cfc85eeed 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -555,6 +555,8 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>                         set_nlink(inode, fattr->nlink);
>                 else if (fattr_supported & NFS_ATTR_FATTR_NLINK)
>                         nfs_set_cache_invalid(inode, NFS_INO_INVALID_NLINK);
> +               else
> +                       set_nlink(inode, 1);
>                 if (fattr->valid & NFS_ATTR_FATTR_OWNER)
>                         inode->i_uid = fattr->uid;
>                 else if (fattr_supported & NFS_ATTR_FATTR_OWNER)

How can an application then test whether a filesystem supports hard
links, or not?

Lionel

Re: [PATCH AUTOSEL 6.1 3/9] NFSv4: Always set NLINK even if the server doesn't support it

[Han Young]

On Mon, 9 Jun 2025 at 09:12, Lionel Cons <lionelcons1972@gmail.com> wrote:
> How can an application then test whether a filesystem supports hard
> links, or not?

I think link will fail with EPERM on those systems. However, this patch
doesn't break existing things though, since filesystems don't support
hard links cannot be mounted by NFS (before).

Thanks.

[PATCH AUTOSEL 6.1 4/9] NFSv4.2: fix listxattr to return selinux security label

[Sasha Levin]

From: Olga Kornievskaia <okorniev@redhat.com>

[ Upstream commit 243fea134633ba3d64aceb4c16129c59541ea2c6 ]

Currently, when NFS is queried for all the labels present on the
file via a command example "getfattr -d -m . /mnt/testfile", it
does not return the security label. Yet when asked specifically for
the label (getfattr -n security.selinux) it will be returned.
Include the security label when all attributes are queried.

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

**YES** This commit should be backported to stable kernel trees. Here's
my extensive analysis: ## Bug Analysis The commit fixes a clear
functional bug in NFSv4.2's `listxattr` implementation in
`fs/nfs/nfs4proc.c:10853`. When users run `getfattr -d` to list all
extended attributes, the security label (like SELinux context) is
missing from the output, even though it's correctly returned when
specifically requested with `getfattr -n security.selinux`. ## Code
Change Analysis The fix is minimal and well-contained: 1. **Variable
addition**: Adds `error4` to track the security label listing result 2.
**Function call**: Adds `error4 =
security_inode_listsecurity(d_inode(dentry), list, left)` 3. **Pointer
management**: Updates `list` and `left` after `error3` processing
(missing in original) 4. **Total calculation**: Changes `error += error2
+ error3` to `error += error2 + error3 + error4` ## Why This Qualifies
for Backport **1. Clear User-Facing Bug**: The inconsistency between
`getfattr -d` and `getfattr -n security.selinux` affects real-world
usage and user expectations. **2. Minimal Risk**: The change is
architecturally simple - it just adds another xattr source to the
listing function, following the exact same pattern as existing `error2`
and `error3` handling. **3. Follows Existing Patterns**: The commit uses
the same error handling, pointer arithmetic, and function call pattern
established by `nfs4_listxattr_nfs4_label()` and
`nfs4_listxattr_nfs4_user()`. **4. No Feature Addition**: This fixes
existing functionality rather than adding new features. **5. Critical
Subsystem**: Extended attributes and security labels are fundamental for
SELinux environments, making this fix important for security-conscious
deployments. **6. Similar Historical Precedent**: All 5 provided
reference commits with "Backport Status: YES" are NFSv4 security label
fixes with similar characteristics - small, contained bugfixes in the
same subsystem. **7. Contained Scope**: The change is isolated to one
function (`nfs4_listxattr`) in one file, with no cross-subsystem
implications. The fix correctly implements the missing piece: while
`nfs4_listxattr_nfs4_label()` calls `security_inode_listsecurity()` when
`CONFIG_NFS_V4_SECURITY_LABEL` is enabled, the main `nfs4_listxattr()`
function wasn't calling it directly for the general xattr listing case,
creating the inconsistent behavior reported.

 fs/nfs/nfs4proc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0f28607c57473..2d94d1d7b0c62 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10630,7 +10630,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
 
 static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
 {
-	ssize_t error, error2, error3;
+	ssize_t error, error2, error3, error4;
 	size_t left = size;
 
 	error = generic_listxattr(dentry, list, left);
@@ -10653,8 +10653,16 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
 	error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, left);
 	if (error3 < 0)
 		return error3;
+	if (list) {
+		list += error3;
+		left -= error3;
+	}
+
+	error4 = security_inode_listsecurity(d_inode(dentry), list, left);
+	if (error4 < 0)
+		return error4;
 
-	error += error2 + error3;
+	error += error2 + error3 + error4;
 	if (size && error > size)
 		return -ERANGE;
 	return error;
-- 
2.39.5

[PATCH AUTOSEL 6.1 8/9] NFSv4: xattr handlers should check for absent nfs filehandles

[Sasha Levin]

From: Scott Mayhew <smayhew@redhat.com>

[ Upstream commit 6e9a2f8dbe93c8004c2af2c0158888628b7ca034 ]

The nfs inodes for referral anchors that have not yet been followed have
their filehandles zeroed out.

Attempting to call getxattr() on one of these will cause the nfs client
to send a GETATTR to the nfs server with the preceding PUTFH sans
filehandle.  The server will reply NFS4ERR_NOFILEHANDLE, leading to -EIO
being returned to the application.

For example:

$ strace -e trace=getxattr getfattr -n system.nfs4_acl /mnt/t/ref
getxattr("/mnt/t/ref", "system.nfs4_acl", NULL, 0) = -1 EIO (Input/output error)
/mnt/t/ref: system.nfs4_acl: Input/output error
+++ exited with 1 +++

Have the xattr handlers return -ENODATA instead.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis of the commit, I can now provide my assessment.

**YES**

This commit should be backported to stable kernel trees for the
following reasons:

1. **It fixes a real user-visible bug**: The commit addresses an issue
   where attempting to call `getxattr()` on NFS referral anchors that
   haven't been followed yet causes an I/O error (-EIO) to be returned
   to applications. The example in the commit message clearly
   demonstrates this:
  ```
  $ strace -e trace=getxattr getfattr -n system.nfs4_acl /mnt/t/ref
  getxattr("/mnt/t/ref", "system.nfs4_acl", NULL, 0) = -1 EIO
  (Input/output error)
  ```

2. **The fix is minimal and contained**: The changes are very small -
   just adding two simple checks:
  ```c
  if (unlikely(NFS_FH(inode)->size == 0))
  return -ENODATA;
  ```
  These checks are added to both `nfs4_proc_get_acl()` and
  `nfs4_proc_set_acl()` functions.

3. **Low risk of regression**: The fix is straightforward and only
   affects the specific error path when filehandles are zero-sized
   (which occurs for referral anchors that haven't been followed). It
   doesn't change any core logic or introduce new features.

4. **Improves application compatibility**: Returning -ENODATA instead of
   -EIO is more semantically correct. Applications expect -ENODATA when
   extended attributes are not available, while -EIO suggests a more
   serious I/O problem.

5. **Follows stable kernel rules**: This fix:
   - Fixes a real bug that affects users
   - Is small and self-contained
   - Has minimal risk of introducing new issues
   - Doesn't add new features or make architectural changes

The commit addresses a specific edge case in NFSv4 where referral
anchors (mount points that redirect to other servers) have zero-length
filehandles until they are actually accessed. When applications try to
read extended attributes on these special inodes, the kernel would
previously send invalid GETATTR requests to the server, resulting in
confusing I/O errors. This fix makes the behavior more predictable and
correct by returning -ENODATA, which indicates that the requested
attribute doesn't exist.

 fs/nfs/nfs4proc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 2d94d1d7b0c62..29f8a2df2c11a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6065,6 +6065,8 @@ static ssize_t nfs4_proc_get_acl(struct inode *inode, void *buf, size_t buflen,
 	struct nfs_server *server = NFS_SERVER(inode);
 	int ret;
 
+	if (unlikely(NFS_FH(inode)->size == 0))
+		return -ENODATA;
 	if (!nfs4_server_supports_acls(server, type))
 		return -EOPNOTSUPP;
 	ret = nfs_revalidate_inode(inode, NFS_INO_INVALID_CHANGE);
@@ -6139,6 +6141,9 @@ static int nfs4_proc_set_acl(struct inode *inode, const void *buf,
 {
 	struct nfs4_exception exception = { };
 	int err;
+
+	if (unlikely(NFS_FH(inode)->size == 0))
+		return -ENODATA;
 	do {
 		err = __nfs4_proc_set_acl(inode, buf, buflen, type);
 		trace_nfs4_set_acl(inode, err);
-- 
2.39.5