Bug 271174 - NFS readdir truncates dirent fileid to 32 bits
Summary: NFS readdir truncates dirent fileid to 32 bits
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.4-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Rick Macklem
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-01 16:19 UTC by Brian Mueller
Modified: 2023-05-22 19:11 UTC (History)
3 users (show)

See Also:
rmacklem: mfc-stable13+
rmacklem: mfc-stable12+


Attachments
Fix the NFSv3 server so that it returns all 64bits of fileno for Readdir entries (1023 bytes, patch)
2023-05-03 02:03 UTC, Rick Macklem
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Mueller 2023-05-01 16:19:33 UTC
The return result of an NFS readdir rpc include directory entries (dirents).  According to NFSv3 RFC, the struct entry3 (which represent the dirents) contains type fileid3 for the fileid.  fileid3 is a 64 bit number.

However, we can see that in the FreeBSD kernel NFS server, when assembling the dirent, the server only marshalls 32 bits of the d_fileno: https://github.com/freebsd/freebsd-src/blob/main/sys/fs/nfsserver/nfs_nfsdport.c#L2259.  This results in the lower 32 bits of the fileno being transmitted.

This manifests as a problem with the libc getcwd call because under some circumstances, getcwd wants to manually determine the cwd by doing a series of readdirs, comparing the inode number (fileid) from the direntries with a stat of the current directory.  Stat (NFS getattr) will return a 64 bit fileid, but the readdir will be 32 bits, so the comparison never matches and getcwd fails.

The NFS server can be fixed by doing 64 bit fileid marshalling for NFSv3/4, while leaving it as 32 bits for NFSv2 where fileid.
Comment 1 Brian Mueller 2023-05-02 18:10:17 UTC
The last sentence in comment #0 should read:

The NFS server can be fixed by doing 64 bit fileid marshalling for NFSv3/4, while leaving it as 32 bits for NFSv2 where fileid is 32 bits.
Comment 2 Rick Macklem freebsd_committer freebsd_triage 2023-05-02 22:37:26 UTC
This was fixed by asomers@ for main/FreeBSD14 on
Dec. 15, 2021 (commit 32fbc5d) and MFC'd to stable/13
on Jan. 2, 2022 (commit 8ec62f0).

If you really need it for FreeBSD12, you could try and
back-port the patch.
Comment 3 Ed Maste freebsd_committer freebsd_triage 2023-05-02 22:42:43 UTC
(In reply to Rick Macklem from comment #2)
Aha, in fact it was also merged to stable/12 in 374e8226d225.
Comment 4 Rick Macklem freebsd_committer freebsd_triage 2023-05-03 00:58:45 UTC
Oops, my mistake. I confused fileno with
directory offset cookie.
I'm almost certain I had a patch waiting
for the 64bit ino_t conversion, but it never
got committed.

I'll come up with a patch.

As such, I've marked the PR Open.
(Sorry about the previous noise.)
Comment 5 Rick Macklem freebsd_committer freebsd_triage 2023-05-03 02:03:43 UTC
Created attachment 241943 [details]
Fix the NFSv3 server so that it returns all 64bits of fileno for Readdir entries

This patch should fix the problem for any version
of FreeBSD that defines ino_t as 64bits.
The problem is NFSv3 specific (not NFSv4) and only
causes problems when a file system on the NFSv3
server generates inode#s that do not fit in 32bits.
This can happen for large ZFS stores.

Maybe the reporter can test it and report whether
or not it fixes the problem for them?
Comment 6 Brian Mueller 2023-05-04 20:45:22 UTC
To test, I do the following.  Note that pwd fails.

root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # sysctl -w debug.disablecwd=1
debug.disablecwd: 0 -> 1
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # pwd
pwd: .: No such file or directory
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # sysctl -w debug.disablecwd=0
debug.disablecwd: 1 -> 0
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # pwd
/mnt/pgh-realm-65-122/home

With the suggested fix, the test succeeds.

root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # sysctl -w debug.disablecwd=1
debug.disablecwd: 0 -> 1
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # pwd
/mnt/pgh-realm-65-122/home
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # sysctl -w debug.disablecwd=0
debug.disablecwd: 1 -> 0
root@pgh-quad-19-c:/mnt/pgh-realm-65-122/home # pwd
/mnt/pgh-realm-65-122/home
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-05-05 22:46:22 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=648a208ef3a171585f3446464646832f0e0ed3dc

commit 648a208ef3a171585f3446464646832f0e0ed3dc
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-05-05 22:43:55 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-05-05 22:43:55 +0000

    nfsd: Fix NFSv3 Readdir/ReaddirPlus reply for large i-node numbers

    If the i-node number (d_fileno) for a file on the server did
    not fit in 32bits, it would be truncated to the low order 32bits
    for the NFSv3 Readdir and ReaddirPlus RPC replies.
    This is no longer correct, given that ino_t is now 64bits.

    This patch fixes this by sending the full 64bits of d_fileno
    on the wire in the NFSv3 Readdir/ReaddirPlus RPC reply.

    PR:     271174
    Reported by:    bmueller@panasas.com
    Tested by:      bmueller@panasas.com
    MFC after:      2 weeks

 sys/fs/nfsserver/nfs_nfsdport.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Comment 8 Rick Macklem freebsd_committer freebsd_triage 2023-05-05 22:48:29 UTC
The patch has been committed to main and will
be MFC'd.
Comment 9 commit-hook freebsd_committer freebsd_triage 2023-05-22 19:02:12 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c7f6408f482059d4fb7e5344b49a8d879da73743

commit c7f6408f482059d4fb7e5344b49a8d879da73743
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-05-05 22:43:55 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-05-22 18:56:03 +0000

    nfsd: Fix NFSv3 Readdir/ReaddirPlus reply for large i-node numbers

    If the i-node number (d_fileno) for a file on the server did
    not fit in 32bits, it would be truncated to the low order 32bits
    for the NFSv3 Readdir and ReaddirPlus RPC replies.
    This is no longer correct, given that ino_t is now 64bits.

    This patch fixes this by sending the full 64bits of d_fileno
    on the wire in the NFSv3 Readdir/ReaddirPlus RPC reply.

    PR:     271174

    (cherry picked from commit 648a208ef3a171585f3446464646832f0e0ed3dc)

 sys/fs/nfsserver/nfs_nfsdport.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2023-05-22 19:10:15 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=886d82afb0359c2bb8321eb1f5c6675787840468

commit 886d82afb0359c2bb8321eb1f5c6675787840468
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-05-05 22:43:55 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-05-22 19:08:30 +0000

    nfsd: Fix NFSv3 Readdir/ReaddirPlus reply for large i-node numbers

    If the i-node number (d_fileno) for a file on the server did
    not fit in 32bits, it would be truncated to the low order 32bits
    for the NFSv3 Readdir and ReaddirPlus RPC replies.
    This is no longer correct, given that ino_t is now 64bits.

    This patch fixes this by sending the full 64bits of d_fileno
    on the wire in the NFSv3 Readdir/ReaddirPlus RPC reply.

    PR:     271174

    (cherry picked from commit 648a208ef3a171585f3446464646832f0e0ed3dc)

 sys/fs/nfsserver/nfs_nfsdport.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Comment 11 Rick Macklem freebsd_committer freebsd_triage 2023-05-22 19:11:04 UTC
Patch has been committed and MFC'd.