Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628444 > unrolled thread

Re: net/core: BUG in unregister_netdevice_many

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: net/core: BUG in unregister_netdevice_many Linus Torvalds <torvalds@linux-foundation.org> - 2017-04-21 20:30 +0200
    Re: net/core: BUG in unregister_netdevice_many Cong Wang <xiyou.wangcong@gmail.com> - 2017-04-21 21:00 +0200
      Re: net/core: BUG in unregister_netdevice_many David Miller <davem@davemloft.net> - 2017-04-21 21:40 +0200

#1628444 — Re: net/core: BUG in unregister_netdevice_many

[Multipart message — attachments visible in raw view] — view raw

On Fri, Apr 21, 2017 at 5:48 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> ------------[ cut here ]------------
> kernel BUG at net/core/dev.c:6813!

Another useless BUG_ON() that

 (a) kills the machine
 (b) doesn't tell the actual useful information.

Grr.

But that BUG_ON() is ancient, and actually goes back to pre-git days.
So you do seem to have triggered some code-path that doesn't ever
happen in any normal use.

This code looks odd:

  void unregister_netdevice_many(struct list_head *head)
  {
        struct net_device *dev;

        if (!list_empty(head)) {
                rollback_registered_many(head);
                list_for_each_entry(dev, head, unreg_list)
                        net_set_todo(dev);
                list_del(head);
        }
  }

In particular the pattern of "look if list is empty, do something to
the list, and then delete the list" is a rather nasty pattern.

Why? That final "delete the list" looks like garbage to me.

Either that list is never used again - in which case the list_del() is
pointless - or it _is_ used again, in which case the list_del is
wrong.

Why is it wrong? Because "list_del()" only removes the list from the
head, but leaves the head itself untouched. So it will still end up
pointing to the list entries that we've just walked.

Now, almost every single case I looked at, the head is always a
temporary list that was created just for this
"unregister_netdevice_many()", so the code works fine, and it's a case
of "that list_del() is just pointless".

But it's a very dangerous pattern. Either the list head should be left
alone, or it should be cleaned up with list_del_init()

Anyway, because each user seems fine, and really just uses it as a
temporary list, this is not the cause of the bug.

I'm assuming that the real cause is simply that "dev->reg_state" ends
up being NETREG_UNREGISTERING or something. Maybe the BUG_ON() could
be just removed, and replaced by the previous warning about
NETREG_UNINITIALIZED.

Something like the attached (TOTALLY UNTESTED) patch.

We really shouldn't have BUG_ON()'s in the kernel, particularly for
cases that we already have error handling for and are ignoring. But
whatever.

Eric Dumazet seems to be the main person to look at this.

                  Linus

[toc] | [next] | [standalone]

#1628499

On Fri, Apr 21, 2017 at 10:25 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Apr 21, 2017 at 5:48 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> ------------[ cut here ]------------
>> kernel BUG at net/core/dev.c:6813!
>
> Another useless BUG_ON() that

I think we are double-unregister'ing the pim6reg device,
we probably need something like:

commit 7dc00c82cbb0119cf4663f65bbaa2cc55f961db2
Author: Wang Chen <wangchen@cn.fujitsu.com>
Date:   Mon Jul 14 20:56:34 2008 -0700

    ipv4: Fix ipmr unregister device oops

    An oops happens during device unregister.

[toc] | [prev] | [next] | [standalone]

#1628554

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 21 Apr 2017 11:55:04 -0700

> On Fri, Apr 21, 2017 at 10:25 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Fri, Apr 21, 2017 at 5:48 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>>
>>> I've got the following error report while fuzzing the kernel with syzkaller.
>>>
>>> ------------[ cut here ]------------
>>> kernel BUG at net/core/dev.c:6813!
>>
>> Another useless BUG_ON() that
> 
> I think we are double-unregister'ing the pim6reg device,
> we probably need something like:

This particular bug is fixed by Nikolay's fix.

I'm not saying you haven't spotted another bug.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web