From: Scott Mayhew <smayhew@xxxxxxxxxx>
[ 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@xxxxxxxxxx>
Signed-off-by: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
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
