netdev - Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <871u4mh781.fsf@tw-ebiederman.twitter.com>
Date:	Wed, 18 Sep 2013 01:19:58 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	David Miller <davem@...emloft.net>
Cc:	fruggeri@...stanetworks.com, edumazet@...gle.com, jiri@...nulli.us,
	alexander.h.duyck@...el.com, amwang@...hat.com,
	netdev@...r.kernel.org
Subject: Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering

David Miller <davem@...emloft.net> writes:

> From: Francesco Ruggeri <fruggeri@...stanetworks.com>
> Date: Tue, 17 Sep 2013 20:50:41 -0700

>> I am not sure that would work.
>> list_empty(&net_todo_list) does not guarantee that there are no
>> unregistering devices still in flight.
>> Another process may have copied net_todo_run into its local list, left
>> net_todo_list empty and still be in the middle of processing
>> unregistering devices (without the rtnl lock) when
>> default_device_exit_batch starts executing.
>
> Ok, now I understand.
>
> Eric B., when you get a chance can you resubmit your patch and perhaps
> elaborate on the situation in the commit message.

The code is on my computer at home so I won't be able to get back to
this until Sunday or Monday.  This should give Francesco time to verify
I really have closed the holes he is seeing.

> If I was confused I'm sure other people will be if they look into
> this in the future.

Agreed.  It is easy to get confused with this issue.  And I was in a
rush so I expect my wording could be improved.



There is still the other part of this issue that dev_close reorders
devices in default_device_exit_batch.

With that reordering we can get the call chain:
  netdev_run_todo
    call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL,...)
      dst_dev_event
        dst_ifdown(..., unregister == 1)
          dst->dev = dev_net(dst->dev)->loopback_dev;
	  dev_hold(dst->dev);

With dev_net(dst->dev)->loopback_dev == NULL;

So my patch to split the close_list out of the unreg_list is also needed
(or something else to address that issue). 

Hopefully my exaspearation with dev_close_many didn't obscure what is
happening there.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html