Re: slab corruption with current -git (was Re: [git pull] vfs pile 1 (splice))

From: Florian Westphal
Date: Sun Oct 09 2016 - 20:52:19 EST


Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Sun, Oct 9, 2016 at 12:11 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > Anyway, I don't think I can bisect it, but I'll try to narrow it down
> > a *bit* at least.
> >
> > Not doing any more pulls on this unstable base, I've been puttering
> > around in trying to clean up some stupid printk logging issues
> > instead.
>
> So I finally got a oops with slub debugging enabled. It doesn't really
> narrow things down, though, it kind of extends on the possible
> suspects. Now adding David Miller and Pablo, because it looks like it
> may be netfilter that does something bad and corrupts memory.

Quite possible, the netns interactions are not nice :-/

> Without further ado, here's the new oops:
>
> general protection fault: 0000 [#1] SMP
> CPU: 7 PID: 169 Comm: kworker/u16:7 Not tainted 4.8.0-11288-gb66484cd7470 #1
> Hardware name: System manufacturer System Product Name/Z170-K, BIOS
..
> Call Trace:
> netfilter_net_exit+0x2f/0x60
> ops_exit_list.isra.4+0x38/0x60
> cleanup_net+0x1ba/0x2a0
> process_one_work+0x1f1/0x480
> worker_thread+0x48/0x4d0
> ? process_one_work+0x480/0x480

..

> like it's a pointer loaded from a free'd allocation.
>
> The code disassembles to
>
> 0: 0f b6 ca movzbl %dl,%ecx
> 3: 48 8d 84 c8 00 01 00 lea 0x100(%rax,%rcx,8),%rax
> a: 00
> b: 49 8b 5c c5 00 mov 0x0(%r13,%rax,8),%rbx
> 10: 48 85 db test %rbx,%rbx
> 13: 0f 84 cb 00 00 00 je 0xe4
> 19: 4c 3b 63 40 cmp 0x40(%rbx),%r12
> 1d: 48 8b 03 mov (%rbx),%rax
> 20: 0f 84 e9 00 00 00 je 0x10f
> 26: 48 85 c0 test %rax,%rax
> 29: 74 26 je 0x51
> 2b:* 4c 3b 60 40 cmp 0x40(%rax),%r12 <-- trapping instruction
> 2f: 75 08 jne 0x39
> 31: e9 ef 00 00 00 jmpq 0x125
> 36: 48 89 d8 mov %rbx,%rax
> 39: 48 8b 18 mov (%rax),%rbx
> 3c: 48 85 db test %rbx,%rbx
>
> and that oopsing instruction seems to be the compare of
> "hooks_entry->orig_ops" from hooks_entry in this expression:
>
> if (hooks_entry && hooks_entry->orig_ops == reg) {
>
> so hooks_entry() is bogus. It was gotten from
>
> hooks_entry = nf_hook_entry_head(net, reg);
>
> but that's as far as I dug. And yes, I do have
> CONFIG_NETFILTER_INGRESS=y in case that matters.
>
> And all this code has changed pretty radically in commit e3b37f11e6e4
> ("netfilter: replace list_head with single linked list"), and there
> was clearly already something wrong with that code, with commit
> 5119e4381a90 ("netfilter: Fix potential null pointer dereference")
> adding the test against NULL. But I suspect that only hid the "oops,
> it's actually not NULL, it loaded some uninitialized value" problem.
>
> Over to the networking guys.. Ideas?

Sorry, not off the top of my head.
Pablo is currently travelling back home from netdev 1.2 in Tokyo,
I can help starting Wednesday when I am back.

One shot in the dark (not even compile tested; wonder if we can end up
zapping bogus hook ...)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index c9d90eb..fd6a2ce 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -189,6 +189,9 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)

unlock:
mutex_unlock(&nf_hook_mutex);
+
+ WARN_ON(hooks_entry && hooks_entry->orig_ops != reg);
+
if (!hooks_entry) {
WARN(1, "nf_unregister_net_hook: hook not found!\n");
return;