netdev - Re: [PATCH] netns: remove useless synchronize

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 10 Feb 2009 23:51:48 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Daniel Lezcano <daniel.lezcano@...e.fr>
Cc:	nicolas.dichtel@....6wind.com, David Miller <davem@...emloft.net>,
	netdev@...r.kernel.org, containers@...ts.osdl.org
Subject: Re: [PATCH] netns: remove useless synchronize_net()

Daniel Lezcano <daniel.lezcano@...e.fr> writes:
>
> Hmm, at the first glance I would say it is useless but perhaps there is a trick
> here I do not understand.
> Eric, is there any particular reason to call synchronize_net before exiting the
> dev_change_net_namespace function ?

I haven't thought about that part of the code path in detail in a long
time.  dev_change_net_namespace() is a condensed version of
register_netdevice() unregister_netdevice().  With the calls down into
the driver removed.

On a side note.  It looks like we now cope with:
call_netdevice_notifiers(NETDEV_REGISTER, dev); failing in
register_netdev, but no one updated dev_change_net_namespace to handle
the change, looks like a real pain to cope with.

As for the synchronize_net, and in response to the original
comment as best as I can tell we do have things being being
deleted that are at least candidates for synchronize_net.

dev_addr_discard(dev);
dev_net_set(dev, net);
netdev_unregister_kobject(dev);

We very much do access dev->net with only rcu protection.

Hmm.

It looks like I originally took the second synchronize_net from what
became rollback_registered, which happens just before we start freeing
the netdevice.

The nastiest case that I can envision is if we happen to receive a
packet (on another cpu) for the network device that we are moving,
just after it has registered in the new network namespace.  If we read
the old network namespace and forward it up the network stack in that
context I can imagine it being a recipe for all kinds of strange
non-deterministic behavior.

So unless there is a reason for this change beyond general cleanup I
would prefer not to think about it potential weirdness, and keep the
code the way it is.

I seem to remember a conversation about this synchronize_net when the
code was merged as well so if we are going to change it, let's look
up those arguments if we can and see if there was something useful
said.

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