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

Linus Torvalds [off-list ref] wrote:

On Sun, Oct 9, 2016 at 12:11 PM, Linus Torvalds
[off-list ref] wrote:

quoted

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;