Created attachment 229846 [details] Crash an NFS v4 server with a VERIFY that mentions FILESAVAIL nfsrvd_verify() passes NULL for sfp to nfsv4_loadattr(), but if the client includes an attribute like FILESAVAIL that uses sfp, nfsv4_loadattr() will crash. I've attached a demo: # uname -a FreeBSD 14.0-CURRENT FreeBSD 14.0-CURRENT #122 main-n250906-d95bc6b0bf4c-dirty: Thu Dec 2 05:26:06 EST 2021 rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # cc fnfsd_9.c # ./a.out ... panic: Fatal page fault at 0xffffffc00020a678: 0x00000000000028 --- exception 13, tval = 0x28 nfsv4_loadattr() at nfsv4_loadattr+0x1a94 nfsrvd_verify() at nfsrvd_verify+0xb6 nfsrvd_dorpc() at nfsrvd_dorpc+0x147a nfssvc_program() at nfssvc_program+0x5a8 svc_run_internal() at svc_run_internal+0x810 svc_run() at svc_run+0x1a2 nfsrvd_nfsd() at nfsrvd_nfsd+0x30c nfssvc_nfsd() at nfssvc_nfsd+0x3ac sys_nfssvc() at sys_nfssvc+0xd0 do_trap_user() at do_trap_user+0x220 cpu_exception_handler_user() at cpu_exception_handler_user+0x72
Created attachment 229867 [details] Just reply not same for verify of file system stats attributes To correctly verify file system stats like FILESFREE is somewhat involved and, since no crashes are being reported, not being done by extant NFSv4 clients. Also, I do not see how file system stats that change constantly could be effectively used by VERIFY/NVERIFY. As such, this patch changes the code to just return NFSERR_NOTSAME for them. If a case where the VERIFY needs to work for any of these attributes, a patch for that could be created. This should avoid the crashes. Maybe the reporter can confirm that it fixes the problem for him?
(In reply to Rick Macklem from comment #1) The patch does fix the problem for me.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=2d90ef47141d3ea0f88b43a1b6daf08d68ba8aba commit 2d90ef47141d3ea0f88b43a1b6daf08d68ba8aba Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-04 22:38:55 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-04 22:38:55 +0000 nfsd: Fix Verify for attributes like FilesAvail When the Verify operation calls nfsv4_loadattr(), it provides the "struct statfs" information that can be used for doing a compare for FilesAvail, FilesFree, FilesTotal, SpaceAvail, SpaceFree and SpaceTotal. However, the code erroneously used the "struct nfsstatfs *" argument that is NULL. This patch fixes these cases to use the correct argument structure. For the case of FilesAvail, the code in nfsv4_fillattr() was factored out into a separate function called nfsv4_filesavail(), so that it can be called from nfsv4_loadattr() as well as nfsv4_fillattr(). In fact, most of the code in nfsv4_filesavail() is old OpenBSD code that does not build/run on FreeBSD, but I left it in place, in case it is of some use someday. I am not aware of any extant NFSv4 client that does Verify on these attributes. Reported by: rtm@lcs.mit.edu Tested by: rtm@lcs.mit.edu PR: 260176 MFC after: 2 weeks sys/fs/nfs/nfs_commonsubs.c | 91 ++++++++++++++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 30 deletions(-)
An alternate patch (which does do Verify for the cases) has been committed and will be MFC'd.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f15f29c975f1374d67ae722df9f9bc7b55d4c42a commit f15f29c975f1374d67ae722df9f9bc7b55d4c42a Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-04 22:38:55 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-18 01:36:41 +0000 nfsd: Fix Verify for attributes like FilesAvail When the Verify operation calls nfsv4_loadattr(), it provides the "struct statfs" information that can be used for doing a compare for FilesAvail, FilesFree, FilesTotal, SpaceAvail, SpaceFree and SpaceTotal. However, the code erroneously used the "struct nfsstatfs *" argument that is NULL. This patch fixes these cases to use the correct argument structure. For the case of FilesAvail, the code in nfsv4_fillattr() was factored out into a separate function called nfsv4_filesavail(), so that it can be called from nfsv4_loadattr() as well as nfsv4_fillattr(). In fact, most of the code in nfsv4_filesavail() is old OpenBSD code that does not build/run on FreeBSD, but I left it in place, in case it is of some use someday. I am not aware of any extant NFSv4 client that does Verify on these attributes. PR: 260176 (cherry picked from commit 2d90ef47141d3ea0f88b43a1b6daf08d68ba8aba) sys/fs/nfs/nfs_commonsubs.c | 91 ++++++++++++++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 30 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=b386392ea909bff2d4b3c46d43ba9c1ee3217e42 commit b386392ea909bff2d4b3c46d43ba9c1ee3217e42 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-04 22:38:55 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-18 01:43:58 +0000 nfsd: Fix Verify for attributes like FilesAvail When the Verify operation calls nfsv4_loadattr(), it provides the "struct statfs" information that can be used for doing a compare for FilesAvail, FilesFree, FilesTotal, SpaceAvail, SpaceFree and SpaceTotal. However, the code erroneously used the "struct nfsstatfs *" argument that is NULL. This patch fixes these cases to use the correct argument structure. For the case of FilesAvail, the code in nfsv4_fillattr() was factored out into a separate function called nfsv4_filesavail(), so that it can be called from nfsv4_loadattr() as well as nfsv4_fillattr(). In fact, most of the code in nfsv4_filesavail() is old OpenBSD code that does not build/run on FreeBSD, but I left it in place, in case it is of some use someday. I am not aware of any extant NFSv4 client that does Verify on these attributes. PR: 260176 (cherry picked from commit 2d90ef47141d3ea0f88b43a1b6daf08d68ba8aba) sys/fs/nfs/nfs_commonsubs.c | 91 ++++++++++++++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 30 deletions(-)
Patch has been committed and MFC'd.