Bug 260046
| Summary: | An NFS v4 client can crash the server with a LISTXATTRS RPC | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Robert Morris <rtm> | ||||||
| Component: | kern | Assignee: | Rick Macklem <rmacklem> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | emaste, rmacklem | ||||||
| Priority: | --- | Flags: | rmacklem:
mfc-stable13+
|
||||||
| Version: | CURRENT | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Created attachment 229735 [details]
Handle the case of a small maxcount < minimum reply
I think this patch will fix the crash
(although the demo program used a maxcount == 16K),
which I don't think would cause the crash.
Anyhow, it tests ok with 16K and with 5, so I
think it deals with the problem.
Maybe the reporter can test this patch?
(In reply to Rick Macklem from comment #1) The patch fixes the crash for me -- thank you. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=5b430a132330bd4a4ea37780807947f3800d009e commit 5b430a132330bd4a4ea37780807947f3800d009e Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-11-26 23:56:29 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-11-26 23:56:29 +0000 nfsd: Sanity check the len argument for ListXattr The check for the original len being >= retlen needs to be done before the "if (nd->nd_repstat == 0)" code, so that it can be reported as too small. Reported by: rtm@lcs.mit.edu Tested by: rtm@lcs.mit.edu PR: 260046 MFC after: 2 weeks sys/fs/nfsserver/nfs_nfsdserv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) The patch has been committed to main and will be MFC'd. A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=5b76a6b862ad5ea35c3f9a4572b6388acdfd3de7 commit 5b76a6b862ad5ea35c3f9a4572b6388acdfd3de7 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-11-26 23:56:29 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-12-11 02:26:22 +0000 nfsd: Sanity check the len argument for ListXattr The check for the original len being >= retlen needs to be done before the "if (nd->nd_repstat == 0)" code, so that it can be reported as too small. PR: 260046 (cherry picked from commit 5b430a132330bd4a4ea37780807947f3800d009e) sys/fs/nfsserver/nfs_nfsdserv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) Patch has been MFC'd. |
Created attachment 229724 [details] Demo of an NFS client crashing a server with LISTXATTRS. nfsrvd_listxattr() calls nfsvno_listxattr(). The latter can return zero at the same time as leaving buf == NULL. But if the return value is zero, nfsrvd_listxattr() dereferences the NULL buf, causing a crash. I've attached a demo: # uname -a FreeBSD 14.0-CURRENT FreeBSD 14.0-CURRENT #99 main-n250901-77e3db078984-dirty: Wed Nov 24 05:42:01 EST 2021 rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # cc fnfsd_3.c # ./a.out ... panic: Fatal page fault at 0xffffffc00026f256: 0000000000000000 cpuid = 0 time = 1637841164 KDB: stack backtrace: db_trace_self() at db_trace_self db_trace_self_wrapper() at db_trace_self_wrapper+0x38 kdb_backtrace() at kdb_backtrace+0x2c vpanic() at vpanic+0x154 panic() at panic+0x2a page_fault_handler() at page_fault_handler+0x1ee do_trap_supervisor() at do_trap_supervisor+0x76 cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x70 --- exception 13, tval = 0 nfsrvd_listxattr() at nfsrvd_listxattr+0x306 nfsrvd_dorpc() at nfsrvd_dorpc+0x154c nfssvc_program() at nfssvc_program+0x5a4 svc_run_internal() at svc_run_internal+0x808 svc_run() at svc_run+0x1a8 nfsrvd_nfsd() at nfsrvd_nfsd+0x30e nfssvc_nfsd() at nfssvc_nfsd+0x386 sys_nfssvc() at sys_nfssvc+0xd0 do_trap_user() at do_trap_user+0x206 cpu_exception_handler_user() at cpu_exception_handler_user+0x72 --- exception 8, tval = 0