* [PATCH AUTOSEL 6.6 04/13] NFSv4: Always set NLINK even if the server doesn't support it
[not found] <20250606154327.547792-1-sashal@kernel.org>
@ 2025-06-06 15:43 ` Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 05/13] NFSv4.2: fix listxattr to return selinux security label Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2025-06-06 15:43 UTC (permalink / raw)
To: patches, stable
Cc: Han Young, Anna Schumaker, Sasha Levin, trondmy, anna, linux-nfs
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 06230baaa554e..419d98cf9e29f 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.6 05/13] NFSv4.2: fix listxattr to return selinux security label
[not found] <20250606154327.547792-1-sashal@kernel.org>
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 04/13] NFSv4: Always set NLINK even if the server doesn't support it Sasha Levin
@ 2025-06-06 15:43 ` Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 08/13] sunrpc: don't immediately retransmit on seqno miss Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 12/13] NFSv4: xattr handlers should check for absent nfs filehandles Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2025-06-06 15:43 UTC (permalink / raw)
To: patches, stable
Cc: Olga Kornievskaia, Anna Schumaker, Sasha Levin, trondmy, anna,
linux-nfs
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 1b94a55215e7d..97a009e007f95 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10625,7 +10625,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);
@@ -10648,8 +10648,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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.6 08/13] sunrpc: don't immediately retransmit on seqno miss
[not found] <20250606154327.547792-1-sashal@kernel.org>
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 04/13] NFSv4: Always set NLINK even if the server doesn't support it Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 05/13] NFSv4.2: fix listxattr to return selinux security label Sasha Levin
@ 2025-06-06 15:43 ` Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 12/13] NFSv4: xattr handlers should check for absent nfs filehandles Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2025-06-06 15:43 UTC (permalink / raw)
To: patches, stable
Cc: Nikhil Jha, Chuck Lever, Anna Schumaker, Sasha Levin, trondmy,
anna, jlayton, linux-nfs
From: Nikhil Jha <njha@janestreet.com>
[ Upstream commit fadc0f3bb2de8c570ced6d9c1f97222213d93140 ]
RFC2203 requires that retransmitted messages use a new gss sequence
number, but the same XID. This means that if the server is just slow
(e.x. overloaded), the client might receive a response using an older
seqno than the one it has recorded.
Currently, Linux's client immediately retransmits in this case. However,
this leads to a lot of wasted retransmits until the server eventually
responds faster than the client can resend.
Client -> SEQ 1 -> Server
Client -> SEQ 2 -> Server
Client <- SEQ 1 <- Server (misses, expecting seqno = 2)
Client -> SEQ 3 -> Server (immediate retransmission on miss)
Client <- SEQ 2 <- Server (misses, expecting seqno = 3)
Client -> SEQ 4 -> Server (immediate retransmission on miss)
... and so on ...
This commit makes it so that we ignore messages with bad checksums
due to seqnum mismatch, and rely on the usual timeout behavior for
retransmission instead of doing so immediately.
Signed-off-by: Nikhil Jha <njha@janestreet.com>
Acked-by: Chuck Lever <chuck.lever@oracle.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 similar backported commits as
reference, here is my assessment:
**YES**
This commit should be backported to stable kernel trees. Here's my
detailed analysis:
## Key Analysis Points:
### 1. **Bug Fix Nature**:
This commit addresses a clear performance bug in SUNRPC's RPCSEC_GSS
sequence number handling. The current code immediately retransmits when
it receives a response with an older sequence number than expected,
leading to excessive and wasteful retransmissions when the server is
simply slow.
### 2. **Alignment with RFC2203**:
The commit correctly implements RFC2203 requirements, which state that
retransmitted messages should use new GSS sequence numbers but keep the
same XID. The current behavior creates a retransmission storm that
violates the spirit of this requirement.
### 3. **Code Changes Analysis**:
The modification in `rpc_decode_header()` at the `out_verifier` label is
minimal and targeted:
**Before** (lines 2773-2776):
```c
case -EACCES:
/* Re-encode with a fresh cred */
fallthrough;
default:
goto out_garbage;
```
**After**:
```c
case -EACCES:
/* possible RPCSEC_GSS out-of-sequence event (RFC2203),
- reset recv state and keep waiting, don't retransmit
*/
task->tk_rqstp->rq_reply_bytes_recvd = 0;
task->tk_status = xprt_request_enqueue_receive(task);
task->tk_action = call_transmit_status;
return -EBADMSG;
default:
goto out_garbage;
```
### 4. **Risk Assessment - LOW**:
- **Scope**: Changes are confined to a specific error handling path in
SUNRPC client code
- **Behavioral change**: Instead of immediate retransmission, it now
waits for normal timeout-based retransmission
- **Fallback**: The normal timeout mechanism still provides
retransmission if needed
- **Error handling**: Uses existing infrastructure
(`xprt_request_enqueue_receive`, `call_transmit_status`)
### 5. **Comparison with Similar Backported Commits**:
This commit shares characteristics with successful backports:
- **Similar Commit #1**: Also fixed SUNRPC verifier handling by changing
`goto out_err` to `goto out_garbage`
- **Similar Commit #2**: Fixed incomplete RPC message handling with
proper state management
- **Similar Commit #5**: Fixed RPCSEC_GSS sequence number limit handling
All these were backported because they fixed protocol compliance issues
with minimal risk.
### 6. **User Impact**:
- **Performance improvement**: Significantly reduces unnecessary
retransmissions under server load
- **Network efficiency**: Reduces wasted bandwidth and server load
- **No breaking changes**: Maintains compatibility while improving
behavior
### 7. **Stability Criteria Compliance**:
✅ **Important bugfix**: Fixes performance degradation and protocol
compliance
✅ **Minimal risk**: Small, localized change to error handling path
✅ **No new features**: Pure bugfix, no architectural changes
✅ **Subsystem-contained**: Changes only affect SUNRPC client verifier
handling
✅ **Clear benefit**: Reduces retransmission storms in production
environments
The commit follows the stable tree rules perfectly: it's a targeted fix
for a real-world performance problem with minimal risk of regression,
affecting only the specific error case of RPCSEC_GSS sequence number
mismatches.
net/sunrpc/clnt.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 4ffb2bcaf3648..63756607f6327 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2733,8 +2733,13 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr)
case -EPROTONOSUPPORT:
goto out_err;
case -EACCES:
- /* Re-encode with a fresh cred */
- fallthrough;
+ /* possible RPCSEC_GSS out-of-sequence event (RFC2203),
+ * reset recv state and keep waiting, don't retransmit
+ */
+ task->tk_rqstp->rq_reply_bytes_recvd = 0;
+ task->tk_status = xprt_request_enqueue_receive(task);
+ task->tk_action = call_transmit_status;
+ return -EBADMSG;
default:
goto out_garbage;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.6 12/13] NFSv4: xattr handlers should check for absent nfs filehandles
[not found] <20250606154327.547792-1-sashal@kernel.org>
` (2 preceding siblings ...)
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 08/13] sunrpc: don't immediately retransmit on seqno miss Sasha Levin
@ 2025-06-06 15:43 ` Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2025-06-06 15:43 UTC (permalink / raw)
To: patches, stable
Cc: Scott Mayhew, Anna Schumaker, Sasha Levin, trondmy, anna,
linux-nfs
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 97a009e007f95..3085a2faab2d3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6059,6 +6059,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);
@@ -6133,6 +6135,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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-06 15:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250606154327.547792-1-sashal@kernel.org>
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 04/13] NFSv4: Always set NLINK even if the server doesn't support it Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 05/13] NFSv4.2: fix listxattr to return selinux security label Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 08/13] sunrpc: don't immediately retransmit on seqno miss Sasha Levin
2025-06-06 15:43 ` [PATCH AUTOSEL 6.6 12/13] NFSv4: xattr handlers should check for absent nfs filehandles Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox