| Message ID | cec4e0f3bb2c77ac03a6154a8508d3930beb5f0f.1674154348.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 71ab9c3e2253619136c31c89dbb2c69305cc89b1 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: fix UaF in netns ops registration error path | expand |
On Thu, Jan 19, 2023 at 07:55:45PM +0100, Paolo Abeni wrote: > If net_assign_generic() fails, the current error path in ops_init() tries > to clear the gen pointer slot. Anyway, in such error path, the gen pointer > itself has not been modified yet, and the existing and accessed one is > smaller than the accessed index, causing an out-of-bounds error: > > BUG: KASAN: slab-out-of-bounds in ops_init+0x2de/0x320 > Write of size 8 at addr ffff888109124978 by task modprobe/1018 > > CPU: 2 PID: 1018 Comm: modprobe Not tainted 6.2.0-rc2.mptcp_ae5ac65fbed5+ #1641 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.1-2.fc37 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x6a/0x9f > print_address_description.constprop.0+0x86/0x2b5 > print_report+0x11b/0x1fb > kasan_report+0x87/0xc0 > ops_init+0x2de/0x320 > register_pernet_operations+0x2e4/0x750 > register_pernet_subsys+0x24/0x40 > tcf_register_action+0x9f/0x560 > do_one_initcall+0xf9/0x570 > do_init_module+0x190/0x650 > load_module+0x1fa5/0x23c0 > __do_sys_finit_module+0x10d/0x1b0 > do_syscall_64+0x58/0x80 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > RIP: 0033:0x7f42518f778d > Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 > 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff > ff 73 01 c3 48 8b 0d cb 56 2c 00 f7 d8 64 89 01 48 > RSP: 002b:00007fff96869688 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > RAX: ffffffffffffffda RBX: 00005568ef7f7c90 RCX: 00007f42518f778d > RDX: 0000000000000000 RSI: 00005568ef41d796 RDI: 0000000000000003 > RBP: 00005568ef41d796 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000000000 > R13: 00005568ef7f7d30 R14: 0000000000040000 R15: 0000000000000000 > </TASK> > > This change addresses the issue by skipping the gen pointer > de-reference in the mentioned error-path. > > Found by code inspection and verified with explicit error injection > on a kasan-enabled kernel. > > Fixes: d266935ac43d ("net: fix UAF issue in nfqnl_nf_hook_drop() when ops_init() failed") > Signed-off-by: Paolo Abeni <pabeni@redhat.com> I don't think the error handling in net_assign_generic or ops_init are helping us to get this right. But, FWIIW: Reviewed-by: Simon Horman <simon.horman@corigine.com> > --- > net/core/net_namespace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c > index 5581d22cc191..078a0a420c8a 100644 > --- a/net/core/net_namespace.c > +++ b/net/core/net_namespace.c > @@ -137,12 +137,12 @@ static int ops_init(const struct pernet_operations *ops, struct net *net) > return 0; > > if (ops->id && ops->size) { > -cleanup: > ng = rcu_dereference_protected(net->gen, > lockdep_is_held(&pernet_ops_rwsem)); > ng->ptr[*ops->id] = NULL; > } > > +cleanup: > kfree(data); > > out: > -- > 2.39.0 >
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 19 Jan 2023 19:55:45 +0100 you wrote: > If net_assign_generic() fails, the current error path in ops_init() tries > to clear the gen pointer slot. Anyway, in such error path, the gen pointer > itself has not been modified yet, and the existing and accessed one is > smaller than the accessed index, causing an out-of-bounds error: > > BUG: KASAN: slab-out-of-bounds in ops_init+0x2de/0x320 > Write of size 8 at addr ffff888109124978 by task modprobe/1018 > > [...] Here is the summary with links: - [net] net: fix UaF in netns ops registration error path https://git.kernel.org/netdev/net/c/71ab9c3e2253 You are awesome, thank you!
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 5581d22cc191..078a0a420c8a 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -137,12 +137,12 @@ static int ops_init(const struct pernet_operations *ops, struct net *net) return 0; if (ops->id && ops->size) { -cleanup: ng = rcu_dereference_protected(net->gen, lockdep_is_held(&pernet_ops_rwsem)); ng->ptr[*ops->id] = NULL; } +cleanup: kfree(data); out:
If net_assign_generic() fails, the current error path in ops_init() tries to clear the gen pointer slot. Anyway, in such error path, the gen pointer itself has not been modified yet, and the existing and accessed one is smaller than the accessed index, causing an out-of-bounds error: BUG: KASAN: slab-out-of-bounds in ops_init+0x2de/0x320 Write of size 8 at addr ffff888109124978 by task modprobe/1018 CPU: 2 PID: 1018 Comm: modprobe Not tainted 6.2.0-rc2.mptcp_ae5ac65fbed5+ #1641 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.1-2.fc37 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x6a/0x9f print_address_description.constprop.0+0x86/0x2b5 print_report+0x11b/0x1fb kasan_report+0x87/0xc0 ops_init+0x2de/0x320 register_pernet_operations+0x2e4/0x750 register_pernet_subsys+0x24/0x40 tcf_register_action+0x9f/0x560 do_one_initcall+0xf9/0x570 do_init_module+0x190/0x650 load_module+0x1fa5/0x23c0 __do_sys_finit_module+0x10d/0x1b0 do_syscall_64+0x58/0x80 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x7f42518f778d Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d cb 56 2c 00 f7 d8 64 89 01 48 RSP: 002b:00007fff96869688 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 RAX: ffffffffffffffda RBX: 00005568ef7f7c90 RCX: 00007f42518f778d RDX: 0000000000000000 RSI: 00005568ef41d796 RDI: 0000000000000003 RBP: 00005568ef41d796 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000003 R11: 0000000000000246 R12: 0000000000000000 R13: 00005568ef7f7d30 R14: 0000000000040000 R15: 0000000000000000 </TASK> This change addresses the issue by skipping the gen pointer de-reference in the mentioned error-path. Found by code inspection and verified with explicit error injection on a kasan-enabled kernel. Fixes: d266935ac43d ("net: fix UAF issue in nfqnl_nf_hook_drop() when ops_init() failed") Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- net/core/net_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)