if_clone.c allows creating multiple interfaces with the same name

[Gleb Smirnoff]

On Fri, Nov 25, 2011 at 05:19:35PM +0400, Gleb Smirnoff wrote:
T> On Thu, Nov 24, 2011 at 09:28:51AM +0100, Daan Vreeken wrote:
T> D> Recently I've discovered a bug in if_clone.c and if.c where the code allows 
T> D> multiple interfaces to be created with exactly the same name (which leads to 
T> D> all sorts of other interesting problems).
T> D> I've submitted a PR about this with patches, which can be found here :
T> D> 
T> D> 	http://www.freebsd.org/cgi/query-pr.cgi?pr=162789
T> D> 
T> D> Could anyone take a look at it?
T> 
T> I decided to simply if_clone code utilizing generic unit allocator. Patch
T> atteched. Now I'll try to merge it with your ideas.

Here is if_cloner patched with additional ifunit() check, as you suggested. Please
review my patch and test it, and then we can commit it.

Considering the second part, that adds locking. Unfortunately, right now we have
numerous races in the network configuration ocde. Many SIOCSsomething ioctls
can race with each other producing unpredictable results and kernel panics.
So, running two ifconfig(8) in parallel is a bad idea today. :( Your patch with
IFNET_NAMING_LOCK() just plumbs one race case: a race between two SIOCSIFNAME
ioctls. And it doesn't plumb a race between SIOCSIFNAME vs SIOCIFCREATE,
because IFNET_NAMING_LOCK() is dropped after unit allocation, but prior to
interface attachement to global interface list.

>From my point of view, we need a generic approach to ioctl() vs ioctl() races,
may be some global serializer of all re-configuration requests of interfaces
and addresses.

-- 
Totus tuus, Glebius.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: if_clone_alloc_unr2.diff
Type: text/x-diff
Size: 5399 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20111125/babed51f/if_clone_alloc_unr2.bin