On Thu, 2021-11-04 at 16:10 -0400, [email protected] wrote:
> [You don't often get email from [email protected]. Learn why this is
> important at http://aka.ms/LearnAboutSenderIdentification.]
>
> decode_compound_hdr() in fs/nfs/nfs4xdr.c adds the taglen supplied by
> the server to a pointer and then dereferences it, but does not first
> check taglen for sanity:
>
> p = xdr_inline_decode(xdr, 8);
> ...;
> hdr->taglen = be32_to_cpup(p);
> ...;
> p = xdr_inline_decode(xdr, hdr->taglen + 4);
> ...;
> p += XDR_QUADLEN(hdr->taglen);
Thanks! Yes, this follows legacy coding practices and really should be
converted to use xdr_stream_decode_opaque_inline().
I'm a little surprised that xdr_inline_decode() didn't barf, though.
Isn't size_t a 64-bit type on riscv64?
> hdr->nops = be32_to_cpup(p);
>
> The second xdr_inline_decode() limits the opportunities for an
> attacker-controlled pointer dereference, but a taglen of 0xfffffffc
> will cause a kernel page fault.
>
> I've attached a program that tickles the bug on my kernel 5.15
> machine:
>
> # uname -a
> Linux (none) 5.15.0-rc7-dirty #15 SMP Thu Nov 4 19:20:36 UTC 2021
> riscv64 riscv64 riscv64 GNU/Linux
> # cc nfs_1.c
> # ./a.out
> mount:mount.nfs: timeout set for Thu Jan 1 00:02:12 1970
> mount.nfs: trying text-based options
> 'vers=4.2,addr=127.0.0.1,clientaddr=127.0.0.1'
> accept returned 4
> proc 0
> proc 1
> exception pc=0xffffffff8022dcd8 cause=d symbolic
> tval=0xffffffe102c5aad8
> [ 16.101267] Unable to handle kernel paging request at virtual
> address ffffffe102c5aad8
> [ 16.112762] Oops [#1]
> [ 16.116973] Modules linked in:
> [ 16.122429] CPU: 0 PID: 60 Comm: mount.nfs Not tainted 5.15.0-rc7-
> dirty #13
> [ 16.131634] Hardware name: ucbbar,riscvemu-bare (DT)
> [ 16.138694] epc : decode_compound_hdr+0x96/0x12e
> [ 16.146706] ra : decode_compound_hdr+0x82/0x12e
> [ 16.154151] epc : ffffffff8022dcd8 ra : ffffffff8022dcc4 sp :
> ffffffd00057b6e0
> ...
> [ 16.272291] status: 0000000200000121 badaddr: ffffffe102c5aad8
> cause: 000000000000000d
> [ 16.282369] [<ffffffff8022dcd8>] decode_compound_hdr+0x96/0x12e
> [ 16.290699] [<ffffffff80239c2a>]
> nfs4_xdr_dec_exchange_id+0x32/0x57e
> [ 16.299265] [<ffffffff8071af5c>]
> rpcauth_unwrap_resp_decode+0x12/0x1a
> [ 16.307926] [<ffffffff8071bc18>] rpcauth_unwrap_resp+0x12/0x1a
> [ 16.316196] [<ffffffff80711fb4>] call_decode+0x112/0x176
> [ 16.323488] [<ffffffff8071a498>] __rpc_execute+0x76/0x216
> [ 16.330751] [<ffffffff8071aab6>] rpc_execute+0x58/0x7e
> [ 16.337966] [<ffffffff80713340>] rpc_run_task+0x12c/0x16c
> [ 16.345113] [<ffffffff80224eba>] nfs4_run_exchange_id+0x1d8/0x262
> [ 16.353364] [<ffffffff80224f68>] _nfs4_proc_exchange_id+0x24/0x2ba
> [ 16.361556] [<ffffffff8022cfc4>] nfs4_proc_exchange_id+0x30/0x50
> [ 16.369829] [<ffffffff8023ea28>]
> nfs41_discover_server_trunking+0x1c/0xa8
> [ 16.378421] [<ffffffff80240d4e>]
> nfs4_discover_server_trunking+0x7c/0x1e8
> [ 16.386958] [<ffffffff802490fe>] nfs4_init_client+0x92/0xf6
> [ 16.394014] [<ffffffff80205412>] nfs_get_client+0x36a/0x394
> [ 16.401147] [<ffffffff8024882e>] nfs4_set_client+0xd6/0x13e
> [ 16.410346] [<ffffffff8024981a>] nfs4_create_server+0xb8/0x208
> [ 16.421493] [<ffffffff80241606>] nfs4_try_get_tree+0x16/0x4c
> [ 16.432759] [<ffffffff80218bac>] nfs_get_tree+0x34a/0x3ac
> [ 16.442283] [<ffffffff8012bce4>] vfs_get_tree+0x18/0x88
> [ 16.451889] [<ffffffff8014a28e>] path_mount+0x4f4/0x77a
> [ 16.461619] [<ffffffff8014a560>] do_mount+0x4c/0x7e
> [ 16.470833] [<ffffffff8014a912>] sys_mount+0xca/0x14e
> [ 16.480401] [<ffffffff80003046>] ret_from_syscall+0x0/0x2
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
> I'm a little surprised that xdr_inline_decode() didn't barf, though.
> Isn't size_t a 64-bit type on riscv64?
I believe hdr->taglen is a u32.