Created attachment 229961 [details] Crash an NFS client with a callback with a bad taglen. nfscl_docb() has a few "goto nfsmout" before it initializes retopsp, but the code after nfsmout dereferences retopsp if taglen != -1. I've attached a demo: # uname -a FreeBSD 14.0-CURRENT FreeBSD 14.0-CURRENT #130 main-n250909-08e6880c1a5c-dirty: Tue Dec 7 13:30:08 EST 2021 rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # cc fnfs_6.c # ./a.out ... panic: Fatal page fault at 0xffffffc00021c200: 0000000000000000 --- exception 15, tval = 0 nfscl_docb() at nfscl_docb+0x1ac 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 229981 [details] properly sanity check the callback taglen The sanity check for taglen in the callback request was incoorect in two ways. 1 - It did not check for an excessively large value. 2 - It did not set the taglen to -1 when the sanity check failed. This patch fixes the above and should stop the crashes. Maybe the reporter can confirm that the patch stops the crashes for them?
(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=d9931c25617d6625e280fda19bd9c2878e49c091 commit d9931c25617d6625e280fda19bd9c2878e49c091 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-09 22:15:48 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-09 22:15:48 +0000 nfscl: Sanity check the callback tag length The sanity check for tag length in a callback request was broken in two ways: It checked for a negative value, but not a large positive value. It did not set taglen to -1, to indicate to the code that it should not be used. This patch fixes both of these issues. Reported by: rtm@lcs.mit.edu Tested by: rtm@lcs.mit.edu PR: 260266 MFC after: 2 weeks sys/fs/nfsclient/nfs_clstate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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=a41216dbe060032bb439d55220bd4a8e135ec80f commit a41216dbe060032bb439d55220bd4a8e135ec80f Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-09 22:15:48 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-23 01:36:18 +0000 nfscl: Sanity check the callback tag length The sanity check for tag length in a callback request was broken in two ways: It checked for a negative value, but not a large positive value. It did not set taglen to -1, to indicate to the code that it should not be used. This patch fixes both of these issues. PR: 260266 (cherry picked from commit d9931c25617d6625e280fda19bd9c2878e49c091) sys/fs/nfsclient/nfs_clstate.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=055b230b85cde066eeedd1417e336d2acf6281b9 commit 055b230b85cde066eeedd1417e336d2acf6281b9 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-12-09 22:15:48 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-23 01:42:24 +0000 nfscl: Sanity check the callback tag length The sanity check for tag length in a callback request was broken in two ways: It checked for a negative value, but not a large positive value. It did not set taglen to -1, to indicate to the code that it should not be used. This patch fixes both of these issues. PR: 260266 (cherry picked from commit d9931c25617d6625e280fda19bd9c2878e49c091) sys/fs/nfsclient/nfs_clstate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Patch has been committed and MFC'd.