Bug 260272 - short OPEN reply can crash NFS v4 client
Summary: short OPEN reply can crash NFS v4 client
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Rick Macklem
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-08 11:02 UTC by Robert Morris
Modified: 2021-12-23 01:48 UTC (History)
2 users (show)

See Also:
rmacklem: mfc-stable13+
rmacklem: mfc-stable12+


Attachments
Cause an NFS v4 client to crash due to a short OPEN reply. (24.45 KB, text/plain)
2021-12-08 11:02 UTC, Robert Morris
no flags Details
fix two places that need to check for an error return from nfsrv_getattrbits (1.01 KB, patch)
2021-12-08 22:02 UTC, Rick Macklem
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2021-12-08 11:02:10 UTC
Created attachment 229975 [details]
Cause an NFS v4 client to crash due to a short OPEN reply.

In these lines in nfsrpc_createv4():

                (void) nfsrv_getattrbits(nd, &attrbits, NULL, NULL);
                NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED);

If the RPC response being parsed is short, then nfsrv_getattrbits()'s
calls to NFSM_DISSECT can leave nd->nd_md == NULL, causing the
NFSM_DISSECT to crash.

I've attached a demo:

# uname -a
FreeBSD  14.0-CURRENT FreeBSD 14.0-CURRENT #133 main-n250909-08e6880c1a5c-dirty: Wed Dec  8 05:41:49 EST 2021     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM  riscv
# cc fnfs_7.c
# ./a.out
...
panic: Fatal page fault at 0xffffffc0001818e2: 0x00000000000010
--- exception 13, tval = 0x10
nfsm_dissect() at nfsm_dissect+0xa
nfsrpc_createv4() at nfsrpc_createv4+0x372
nfsrpc_create() at nfsrpc_create+0x1ae
nfs_create() at nfs_create+0x196
vop_sigdefer() at vop_sigdefer+0x26
nfs_vnodeops_bypass() at nfs_vnodeops_bypass+0x16
VOP_CREATE_APV() at VOP_CREATE_APV+0x3a
VOP_CREATE() at VOP_CREATE+0x2e
vn_open_cred() at vn_open_cred+0x20a
vn_open() at vn_open+0x32
kern_openat() at kern_openat+0x164
sys_openat() at sys_openat+0x32
syscallenter() at syscallenter+0xf4
ecall_handler() at ecall_handler+0x18
do_trap_user() at do_trap_user+0xea
cpu_exception_handler_user() at cpu_exception_handler_user+0x72
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2021-12-08 22:02:05 UTC
Created attachment 229982 [details]
fix two places that need to check for an error return from nfsrv_getattrbits

nfsrv_getattrbits() can return an error when the
received attribute bitmap does not parse correctly.
There were two places in the client where the code
did not check for such an error return.
This patch fixes these two cases and should fix the
crashes.

Maybe the reporter can confirm that the patch stops the
crashes for them?
Comment 2 Robert Morris 2021-12-09 09:57:08 UTC
(In reply to Rick Macklem from comment #1)
Yes -- the patch eliminates the crash for me.
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-12-09 22:36:19 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ab639f2398bf7efd4dfd38cd6527e22f6e781ae9

commit ab639f2398bf7efd4dfd38cd6527e22f6e781ae9
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-09 22:32:22 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-09 22:32:22 +0000

    nfscl: Check for an error return from nfsrv_getattrbits()

    There were two places where the client code did not check
    for a parse error return from nfsrv_getattrbits().

    This patch fixes both of these cases.

    Reported by:    rtm@lcs.mit.edu
    Tested by:      rtm@lcs.mit.edu
    PR:     260272
    MFC after:      2 weeks

 sys/fs/nfsclient/nfs_clrpcops.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Comment 4 Rick Macklem freebsd_committer freebsd_triage 2021-12-09 22:37:32 UTC
Patch has been committed and will be MFC'd.
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-12-23 01:39:04 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c4ad95fb8e4221ffcf435c4714923db7d791737b

commit c4ad95fb8e4221ffcf435c4714923db7d791737b
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-09 22:32:22 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-23 01:34:53 +0000

    nfscl: Check for an error return from nfsrv_getattrbits()

    There were two places where the client code did not check
    for a parse error return from nfsrv_getattrbits().

    This patch fixes both of these cases.

    PR:     260272

    (cherry picked from commit ab639f2398bf7efd4dfd38cd6527e22f6e781ae9)

 sys/fs/nfsclient/nfs_clrpcops.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2021-12-23 01:45:07 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=55a60738e7af6f388a1ac733bfcaec6d6b70d538

commit 55a60738e7af6f388a1ac733bfcaec6d6b70d538
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-09 22:32:22 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-23 01:41:00 +0000

    nfscl: Check for an error return from nfsrv_getattrbits()

    There were two places where the client code did not check
    for a parse error return from nfsrv_getattrbits().

    This patch fixes both of these cases.

    PR:     260272

    (cherry picked from commit ab639f2398bf7efd4dfd38cd6527e22f6e781ae9)

 sys/fs/nfsclient/nfs_clrpcops.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Comment 7 Rick Macklem freebsd_committer freebsd_triage 2021-12-23 01:48:43 UTC
Patch has been committed and MFC'd.