Created attachment 230107 [details] Crash an NFS v4 client with two CB_SEQUENCEs and a wild slotid If a callback message contains two CB_SEQUENCE operators, and the first one is valid, but the second contains a wild slotid, then at the end of nfscl_docb() gotseq_ok will be non-zero, and the wild slotid will be passed to nfsv4_seqsess_cacherep(). The latter indexes an array with slotid. I've attached a demo: # uname -a FreeBSD junk.doesnotexist.org 14.0-CURRENT FreeBSD 14.0-CURRENT #147 main-n250911-3ff4b4101008-dirty: Tue Dec 14 05:47:56 EST 2021 rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # cc fnfs_11.c # ./a.out ... panic: Fatal page fault at 0xffffffc0002104d0: 0xffffffe001428c28 --- exception 13, tval = 0xffffffe001428c28 nfsv4_seqsess_cacherep() at nfsv4_seqsess_cacherep+0x18 nfscl_docb() at nfscl_docb+0x3aa nfscb_program() at nfscb_program+0xee svc_run_internal() at svc_run_internal+0x810 svc_run() at svc_run+0x1a2 nfscbd_nfsd() at nfscbd_nfsd+0xce nfssvc_nfscl() at nfssvc_nfscl+0x204 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 230127 [details] check for cbsequence not first op at the beginning of processing This patch should stop the crashes. It moves the check for "not first op" to the beginning of CB_Sequence processing. It also fixes a couple of other things: - Adds a sanity check for a large taglen. - Moves the check for "no cbsequence" to the beginning of op processing, since the check was in some CB ops, but not all of them. Maybe the reporter can confirm it fixes the problem for them?
(In reply to Rick Macklem from comment #1) Yes, the patch eliminates the crash for me.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e0861304a7b6b9c410db69be6148a5510c6b2d23 commit e0861304a7b6b9c410db69be6148a5510c6b2d23 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-16 00:36:40 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-16 00:36:40 +0000 nfscl: Handle CB_SEQUENCE not first op correctly The check for "not first operation" in CB_SEQUENCE was done after the slot, etc. was updated. This patch moves the check to the beginning of CB_SEQUENCE processing. While here, also fix the check for "no CB_SEQUENCE operation first" by moving the check to the beginning of callback operation parsing, since the check was in a couple of the other operations, but not all of them. Reported by: rtm@lcs.mit.edu Tested by: rtm@lcs.mit.edu PR: 260412 MFC after: 2 weeks sys/fs/nfsclient/nfs_clstate.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Created attachment 230161 [details] check for cbsequence not first op at the beginning of processing The previous patch had an incorrect partial patch accidentally included in it. This patch has that partial patch removed from it. The correct version of this other patch can be found in PR#260266.
The patch 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=efc08dab492c38a0a1830d8227c9c62e70ccf5df commit efc08dab492c38a0a1830d8227c9c62e70ccf5df Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-16 00:36:40 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-30 01:24:55 +0000 nfscl: Handle CB_SEQUENCE not first op correctly The check for "not first operation" in CB_SEQUENCE was done after the slot, etc. was updated. This patch moves the check to the beginning of CB_SEQUENCE processing. While here, also fix the check for "no CB_SEQUENCE operation first" by moving the check to the beginning of callback operation parsing, since the check was in a couple of the other operations, but not all of them. PR: 260412 (cherry picked from commit e0861304a7b6b9c410db69be6148a5510c6b2d23) sys/fs/nfsclient/nfs_clstate.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6d16489da847852adb998c5f44b238d0b9c39aaf commit 6d16489da847852adb998c5f44b238d0b9c39aaf Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-16 00:36:40 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-30 01:32:33 +0000 nfscl: Handle CB_SEQUENCE not first op correctly The check for "not first operation" in CB_SEQUENCE was done after the slot, etc. was updated. This patch moves the check to the beginning of CB_SEQUENCE processing. While here, also fix the check for "no CB_SEQUENCE operation first" by moving the check to the beginning of callback operation parsing, since the check was in a couple of the other operations, but not all of them. PR: 260412 (cherry picked from commit e0861304a7b6b9c410db69be6148a5510c6b2d23) sys/fs/nfsclient/nfs_clstate.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Patch has been committed and MFC'd.