Bug 260071 - bad slotid in SEQUENCE reply can crash NFS v4 client
Summary: bad slotid in SEQUENCE 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-11-26 20:26 UTC by Robert Morris
Modified: 2021-12-11 02:46 UTC (History)
2 users (show)

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


Attachments
A demo of an nfs v4 server crashing the client with a negative slotid. (19.33 KB, text/plain)
2021-11-26 20:26 UTC, Robert Morris
no flags Details
Check that slot in Sequence reply is the same as in the request (763 bytes, patch)
2021-11-27 02:09 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-11-26 20:26:59 UTC
Created attachment 229750 [details]
A demo of an nfs v4 server crashing the client with a negative slotid.

This code in newnfs_request() uses a slot number sent by the server
to index into an array without bounds-checking:

                                        slot = fxdr_unsigned(int, *tl++);
                                        freeslot = slot;
                                        if (retseq != sep->nfsess_slotseq[slot])
                                                printf("retseq diff 0x%x\n",
                                                    retseq);

I've attached a demo:

# uname -a
FreeBSD  14.0-CURRENT FreeBSD 14.0-CURRENT #103 main-n250903-2a29c1558e82-dirty: Fri Nov 26 13:54:28 EST 2021     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM  riscv
# cc fnfs_3.c
# ./a.out
...
panic: Fatal page fault at 0xffffffc000207f4c: 0xffffffce02bf4cc0
--- exception 13, tval = 0xffffffce02bf4cc0
newnfs_request() at newnfs_request+0x1002
nfsrpc_reclaimcomplete() at nfsrpc_reclaimcomplete+0x92
nfsrpc_setclient() at nfsrpc_setclient+0x630
nfscl_getcl() at nfscl_getcl+0x53e
mountnfs() at mountnfs+0x722
nfs_mount() at nfs_mount+0x161e
vfs_mount_sigdefer() at vfs_mount_sigdefer+0x20
vfs_domount_first() at vfs_domount_first+0x1d4
vfs_domount() at vfs_domount+0x21c
vfs_donmount() at vfs_donmount+0x79c
sys_nmount() at sys_nmount+0x66
do_trap_user() at do_trap_user+0x206
cpu_exception_handler_user() at cpu_exception_handler_user+0x72
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2021-11-27 02:09:07 UTC
Created attachment 229752 [details]
Check that slot in Sequence reply is the same as in the request

This patch should fix the crash.
It checks that the slotid in the Sequence reply
is the same as in the request and corrects it
if it is not the same.

Maybe the reporter can test it and confirm that it
fixes the problem for them?
Comment 2 Robert Morris 2021-11-27 10:10:29 UTC
(In reply to Rick Macklem from comment #1)
Yes -- the patch fixes the problem for me.
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-11-27 23:06:25 UTC
A commit in branch main references this bug:

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

commit 1c15c8c0e935f3f7def7b03e9546f7d6790f465e
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-11-27 23:02:04 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-11-27 23:02:04 +0000

    nfscl: Sanity check the Sequence slotid in reply

    The slotid in the Sequence reply must be the same as
    in the request.  Check that it is the same and log
    a console message if it is not, plus set it to the
    correct value.

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

 sys/fs/nfs/nfs_commonkrpc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Comment 4 Rick Macklem freebsd_committer freebsd_triage 2021-11-28 01:04:13 UTC
Patch has been committed and will be MFC'd.
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-12-11 02:32:09 UTC
A commit in branch stable/13 references this bug:

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

commit 3e8081ac8869e2b5b92d4a84c217a05cdd10487a
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-11-27 23:02:04 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-11 02:28:14 +0000

    nfscl: Sanity check the Sequence slotid in reply

    The slotid in the Sequence reply must be the same as
    in the request.  Check that it is the same and log
    a console message if it is not, plus set it to the
    correct value.

    PR:     260071

    (cherry picked from commit 1c15c8c0e935f3f7def7b03e9546f7d6790f465e)

 sys/fs/nfs/nfs_commonkrpc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Comment 6 commit-hook freebsd_committer freebsd_triage 2021-12-11 02:43:11 UTC
A commit in branch stable/12 references this bug:

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

commit 9cd92bc5abdefb1897c3f948f50e66a3c21dff8e
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-11-27 23:02:04 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-11 02:38:11 +0000

    nfscl: Sanity check the Sequence slotid in reply

    The slotid in the Sequence reply must be the same as
    in the request.  Check that it is the same and log
    a console message if it is not, plus set it to the
    correct value.

    PR:     260071

    (cherry picked from commit 1c15c8c0e935f3f7def7b03e9546f7d6790f465e)

 sys/fs/nfs/nfs_commonkrpc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Comment 7 Rick Macklem freebsd_committer freebsd_triage 2021-12-11 02:46:19 UTC
Patch has been MFC'd.