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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 30 Nov 2009 15:42:13 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, hadi@...erus.ca, dlezcano@...ibm.com,
	adobriyan@...il.com, kaber@...sh.net, opurdila@...acom.com
Subject: Re: [PATCH 01/20] net: NETDEV_UNREGISTER_PERNET -> NETDEV_UNREGISTER_BATCH

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

> From: "Eric W. Biederman" <ebiederm@...ssion.com>
> Date: Sun, 29 Nov 2009 17:45:58 -0800
>
>> Additionally it appears that we moved the route cache flush after
>> the final synchronize_net, which seems wrong and there was no
>> explanation.  So I have restored the original location of the final
>> synchronize_net.
>
> I think the idea was that it's important for all pending stale
> RCU references to the device to be gone before we send out the
> notifier.  And that's why the synchronize_net() comes first.
>
> This needs more thought.

The only case the affects (today) is the invalidation of the route/flow cache.

The routing table until the addition of NETDEV_UNREGISTER_PERNET happened
in the notifier NETDEV_UNREGISTER, which comes before the final synchronize_net.

The routing cache has rcu references to the network device.

So it appears to me that with the current pernet batched routing code cleanup we
can end up calling kfree before all of the call_rcu functions triggered by
rt_do_flush will be run.

    netdev_run_todo()
       kobject_put()
         netdev_release()
           kfree()

NETDEV_UNREGISTER_PERCPU
    rt_cache_flush(xxx, 0)
       rt_do_flush()
         rt_free
           call_rcu

So it appears to me that moving the final syncrhonize_net() back where it was before
the NETDEV_UNREGISTER_PERNET changes (as I did) is necessary for correctness.

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

Powered by blists - more mailing lists