Bug 260155 - inadequate LAYOUTTYPE sanity check in nfsv4_loadattr()
Summary: inadequate LAYOUTTYPE sanity check in nfsv4_loadattr()
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-01 15:14 UTC by Robert Morris
Modified: 2021-12-18 01:53 UTC (History)
2 users (show)

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


Attachments
Crash an NFS server with a broken LAYOUTTYPE attribute. (30.78 KB, text/plain)
2021-12-01 15:14 UTC, Robert Morris
no flags Details
sanity check the count of layouttypes in attributes (635 bytes, patch)
2021-12-04 01:25 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-01 15:14:36 UTC
Created attachment 229829 [details]
Crash an NFS server with a broken LAYOUTTYPE attribute.

In this nfsv4_loadattr() code:

                case NFSATTRBIT_FSLAYOUTTYPE:
                case NFSATTRBIT_LAYOUTTYPE:
                        NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED);
                        attrsum += NFSX_UNSIGNED;
                        i = fxdr_unsigned(int, *tl);
                        if (i > 0) {
                                NFSM_DISSECT(tl, u_int32_t *, i *
                                    NFSX_UNSIGNED);

If the RPC contains an i that is positive and big enough that
i*NFSX_UNSIGNED is negative, the next attribute may experience a crash
due to nd_dpos being wild. nfsm_dissect() and nfsm_dissct() are only
able to prevent this if the siz is positive, not negative.

I've attached a demo:

# uname -a
FreeBSD  14.0-CURRENT FreeBSD 14.0-CURRENT #120 main-n250906-d95bc6b0bf4c-dirty: Wed Dec  1 06:52:50 EST 2021     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM  riscv
# cc fnfsd_8.c
# ./a.out
...
panic: Fatal page fault at 0xffffffc000209adc: 0xffffffcf818ecbc0
--- exception 13, tval = 0xffffffcf818ecbc0
nfsv4_loadattr() at nfsv4_loadattr+0xef8
nfsrvd_verify() at nfsrvd_verify+0xb6
nfsrvd_dorpc() at nfsrvd_dorpc+0x147a
nfssvc_program() at nfssvc_program+0x5a8
svc_run_internal() at svc_run_internal+0x810
svc_run() at svc_run+0x1a2
nfsrvd_nfsd() at nfsrvd_nfsd+0x30c
nfssvc_nfsd() at nfssvc_nfsd+0x3ac
sys_nfssvc() at sys_nfssvc+0xd0
do_trap_user() at do_trap_user+0x220
cpu_exception_handler_user() at cpu_exception_handler_user+0x72
--- exception 8, tval = 0x1c5816ef20
Comment 1 Rick Macklem freebsd_committer 2021-12-04 01:25:28 UTC
Created attachment 229868 [details]
sanity check the count of layouttypes in attributes

This patch should fix the crash.

Maybe the reporter can confirm it fixes the problem for him?
Comment 2 Robert Morris 2021-12-04 11:18:13 UTC
(In reply to Rick Macklem from comment #1)
The patch does fix the problem for me.
Comment 3 commit-hook freebsd_committer 2021-12-04 22:26:10 UTC
A commit in branch main references this bug:

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

commit 480be96e1e24617f0f152256d7453f60774fd1f3
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-04 22:18:48 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-04 22:22:19 +0000

    nfsd: Sanity check the Layouttype count

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

 sys/fs/nfs/nfs_commonsubs.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 4 Rick Macklem freebsd_committer 2021-12-04 22:47:58 UTC
The patch has been committed and will be MFC'd.
Comment 5 commit-hook freebsd_committer 2021-12-18 01:36:37 UTC
A commit in branch stable/13 references this bug:

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

commit fac8422a41e61847acd06f7dab9fab9cfc2f4a74
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-04 22:18:48 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-18 01:32:47 +0000

    nfsd: Sanity check the Layouttype count

    PR:     260155

    (cherry picked from commit 480be96e1e24617f0f152256d7453f60774fd1f3)

 sys/fs/nfs/nfs_commonsubs.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 6 commit-hook freebsd_committer 2021-12-18 01:45:40 UTC
A commit in branch stable/12 references this bug:

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

commit 5ad7804beb38fdc23f0f161484f5469e38c8fade
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-04 22:18:48 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-18 01:41:43 +0000

    nfsd: Sanity check the Layouttype count

    PR:     260155

    (cherry picked from commit 480be96e1e24617f0f152256d7453f60774fd1f3)

 sys/fs/nfs/nfs_commonsubs.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 7 Rick Macklem freebsd_committer 2021-12-18 01:53:23 UTC
Patch has been committed and MFC'd.