Date: Sat, 16 Dec 2023 11:36:09 -0700
From: David Ahern <dsahern@...nel.org>
To: Kui-Feng Lee <sinquersw@...il.com>, thinker.li@...il.com,
 netdev@...r.kernel.org, martin.lau@...ux.dev, kernel-team@...a.com,
 davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com
Cc: kuifeng@...a.com, syzbot+c15aa445274af8674f41@...kaller.appspotmail.com
Subject: Re: [PATCH net-next v3 1/2] net/ipv6: insert a f6i to a GC list only
 if the f6i is in a fib6_table tree.

On 12/15/23 11:12 AM, Kui-Feng Lee wrote:
> 
> I tried to reproduce the issue yesterday, according to the hypothesis
> behind the patch of this thread. The following is the instructions
> to reproduce the UAF issue. However, this instruction doesn't create
> a crash at the place since the memory is still available even it has
> been free. But, the log shows a UAF.
> 
> The patch at the end of this message is required to reproduce and
> show UAF. The most critical change in the patch is to insert
> a 'mdelay(3000)' to sleep 3s in rt6_route_rcv(). That gives us
> a chance to manipulate the kernel to reproduce the UAF.
> 
> Here is my conclusion. There is no protection between finding
> a route and changing the route in rt6_route_rcv(), including inserting
> the entry to the gc list. It is possible to insert an entry that will be
> free later to the gc list, causing a UAF. There is more explanations
> along with the following logs.

TL;DR: I think 3dec89b14d37 should be reverted for 6.6 and 6.7
(selftests should be valid with or without this change) and you try
again for 6.9 (6.7 dev cycle is about to close).

###

I was successful in triggering UAF twice yesterday, but a slightly
different code path that in Eric's trace:

This is the WARN_ON for !hlist_unhashed in fib6_info_release:

[   46.926339] ------------[ cut here ]------------
[   46.926368] WARNING: CPU: 3 PID: 0 at include/net/ip6_fib.h:332
fib6_info_release+0x2a/0x43
[   46.926384] Modules linked in:
[   46.926393] CPU: 3 PID: 0 Comm: swapper/3 Not tainted
6.7.0-rc4-debug+ #16
[   46.926399] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.15.0-1 04/01/2014
[   46.926404] RIP: 0010:fib6_info_release+0x2a/0x43
[   46.926409] Code: 48 85 ff 74 3d 55 48 89 e5 53 48 89 fb 48 8d 7f 2c
e8 3e ff ff ff 84 c0 74 25 48 8d 7b 40 e8 33 11 8b ff 48 83 7b 40 00 74
02 <0f> 0b 48 8d bb a0 00 00 00 48 c7 c6 93 ea ae 81 e8 3f 00 66 ff 5b
[   46.926416] RSP: 0018:ffffc900002a8390 EFLAGS: 00010282
[   46.926422] RAX: 1ffff11002048b00 RBX: ffff888010245c00 RCX:
ffffffff81af8c9e
[   46.926426] RDX: dffffc0000000000 RSI: 0000000000000004 RDI:
ffff888010245c40
[   46.926431] RBP: ffffc900002a8398 R08: ffffed1002048b86 R09:
0000000000000001
[   46.926435] R10: ffffffff81af8be4 R11: ffff888010245c2f R12:
ffff88800aeec000
[   46.926439] R13: 0000000000000000 R14: ffff88801ab7c200 R15:
0000000000000020
[   46.926444] FS:  0000000000000000(0000) GS:ffff88806c000000(0000)
knlGS:0000000000000000
[   46.926448] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   46.926452] CR2: 00007f897cc95fb0 CR3: 000000001d69d000 CR4:
0000000000750ef0
[   46.926458] PKRU: 55555554
[   46.926462] Call Trace:
[   46.926466]  <IRQ>
[   46.926470]  ? show_regs+0x5c/0x60
[   46.926478]  ? fib6_info_release+0x2a/0x43
[   46.926483]  ? __warn+0xcb/0x19c
[   46.926489]  ? fib6_info_release+0x2a/0x43
[   46.926495]  ? report_bug+0x114/0x186
[   46.926504]  ? handle_bug+0x40/0x6b
[   46.926510]  ? exc_invalid_op+0x19/0x41
[   46.926515]  ? asm_exc_invalid_op+0x1b/0x20
[   46.926524]  ? refcount_dec_and_test+0x15/0x43
[   46.926530]  ? fib6_info_release+0x23/0x43
[   46.926536]  ? fib6_info_release+0x2a/0x43
[   46.926542]  ndisc_router_discovery+0xf41/0xfa6


UAF here:

[   47.941928]
==================================================================
[   47.942548] BUG: KASAN: slab-use-after-free in
fib6_gc_all.constprop.0+0x19b/0x294
[   47.943178] Read of size 8 at addr ffff888010245c38 by task swapper/3/0

[   47.943866] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G        W
 6.7.0-rc4-debug+ #16
[   47.944548] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.15.0-1 04/01/2014
[   47.945204] Call Trace:
[   47.945416]  <IRQ>
[   47.945595]  dump_stack_lvl+0x5b/0x82
[   47.945912]  print_address_description.constprop.0+0x7a/0x2eb
[   47.946391]  print_report+0x106/0x1e0
[   47.946703]  ? virt_to_slab+0x9/0x1a
[   47.947007]  ? kmem_cache_debug_flags+0x13/0x1f
[   47.947392]  ? kasan_complete_mode_report_info+0x19e/0x1a1
[   47.947850]  ? fib6_gc_all.constprop.0+0x19b/0x294
[   47.948254]  kasan_report+0x99/0xc4
[   47.948554]  ? fib6_gc_all.constprop.0+0x19b/0x294
[   47.948962]  __asan_load8+0x77/0x79
[   47.949262]  fib6_gc_all.constprop.0+0x19b/0x294


for a route added here:

[   47.970092] Allocated by task 0:
[   47.970366]  stack_trace_save+0x8d/0xbb
[   47.970373]  kasan_save_stack+0x26/0x46
[   47.970379]  kasan_set_track+0x25/0x2b
[   47.970385]  kasan_save_alloc_info+0x1e/0x21
[   47.970392]  ____kasan_kmalloc+0x6f/0x7b
[   47.970398]  __kasan_kmalloc+0x9/0xb
[   47.970404]  __kmalloc+0x91/0xbf
[   47.970411]  kzalloc+0xf/0x11
[   47.970416]  fib6_info_alloc+0x26/0xa1
[   47.970422]  ip6_route_info_create+0x266/0x6c5
[   47.970428]  ip6_route_add+0x14/0x46
[   47.970433]  rt6_add_dflt_router+0x123/0x1bd
[   47.970439]  ndisc_router_discovery+0x5a4/0xfa6
[   47.970447]  ndisc_rcv+0x1a2/0x1b5

This is just aggressive RA's and aggressive gc with a hack to the stack
to toggle lifetime or metric on every RA (something that can be scripted
with an ra command - set and reset lifetime in every other RA and change
the metric in every RA).

I was targeting this code path because I noticed rt6_add_dflt_router
sets RTF_EXPIRES but does not pass in the expires value. There are races
(like the one you found with a different code path) in the handling of
RTF_EXPIRES, setting the timer, running gc and adding the route entry to
the gc list.

In short there are a number of places in the RA path that need the
lifetime handling cleaned up first to make it all consistent with the
idea of using a linked list to track entries with an expires tag.

Powered by blists - more mailing lists