Created attachment 229757 [details] An NFS client that crashes the server with a bad slotid. nfsrvd_sequence() accepts the client's slotid without checking against 0..64: nd->nd_slotid = fxdr_unsigned(uint32_t, *tl++); This can cause a crash when nd_slotid is later used to index into sess_slots. I've attached a demo: # uname -a FreeBSD 14.0-CURRENT FreeBSD 14.0-CURRENT #107 main-n250904-c4c468281fb6-dirty: Sat Nov 27 06:21:53 EST 2021 rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # cc fnfsd_6.c # ./a.out ... panic: Fatal page fault at 0xffffffc000317a82: 0x4000000000000000 --- exception 13, tval = 0x4000000000000000 m_free() at m_free+0x10 m_freem() at m_freem+0x22 nfsv4_seqsess_cacherep() at nfsv4_seqsess_cacherep+0x56 nfsrv_cache_session() at nfsrv_cache_session+0x114 nfssvc_program() at nfssvc_program+0x624 svc_run_internal() at svc_run_internal+0x808 svc_thread_start() at svc_thread_start+0xe fork_exit() at fork_exit+0x68 fork_trampoline() at fork_trampoline+0xa
Created attachment 229772 [details] Don't set ND_HASSEQUNCE for NFSERR_BADSLOT For the NFSERR_BADSLOT failure, do not set ND_HASSEQUENCE because the nd_slotid is not valid. This should avoid the crash. Maybe the reporter can test the patch?
(In reply to Robert Morris from comment #0)
(In reply to Tim Foster from comment #2) Apologies - was browsing bugs here and seem to have inadvertently added a comment to this one. Please ignore it.
(In reply to Rick Macklem from comment #1) Yes -- the patch makes the crash go away for me.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=33d0be8a923a840ec0724d50815890c5ffe0e884 commit 33d0be8a923a840ec0724d50815890c5ffe0e884 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-01 21:46:41 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-01 21:46:41 +0000 nfsd: Do not try to cache a reply for NFSERR_BADSLOT When nfsrv_checksequence() replies NFSERR_BADSLOT, the value of nd_slotid is not valid. As such, the reply cannot be cached in the session. Do not set ND_HASSEQUENCE for this case. Reported by: rtm@lcs.mit.edu Tested by: rtm@lcs.mit.edu PR: 260076 MFC after: 2 weeks sys/fs/nfsserver/nfs_nfsdserv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
The patch has now been comitted and will be MFC'd.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6a6b08d4646e109222a995c8432c025266b6101a commit 6a6b08d4646e109222a995c8432c025266b6101a Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-01 21:46:41 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-15 01:34:05 +0000 nfsd: Do not try to cache a reply for NFSERR_BADSLOT When nfsrv_checksequence() replies NFSERR_BADSLOT, the value of nd_slotid is not valid. As such, the reply cannot be cached in the session. Do not set ND_HASSEQUENCE for this case. PR: 260076 (cherry picked from commit 33d0be8a923a840ec0724d50815890c5ffe0e884) sys/fs/nfsserver/nfs_nfsdserv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3a2be1fb9906a78841ab5266b9d75128569a2ca5 commit 3a2be1fb9906a78841ab5266b9d75128569a2ca5 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-01 21:46:41 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-15 01:40:12 +0000 nfsd: Do not try to cache a reply for NFSERR_BADSLOT When nfsrv_checksequence() replies NFSERR_BADSLOT, the value of nd_slotid is not valid. As such, the reply cannot be cached in the session. Do not set ND_HASSEQUENCE for this case. PR: 260076 (cherry picked from commit 33d0be8a923a840ec0724d50815890c5ffe0e884) sys/fs/nfsserver/nfs_nfsdserv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Patch has been committed and MFC'd.