if_clone.c allows creating multiple interfaces with the same name

[Daan Vreeken]

Hi All,

Recently I've discovered a bug in if_clone.c and if.c where the code allows
multiple interfaces to be created with exactly the same name (which leads to
all sorts of other interesting problems).
I've submitted a PR about this with patches, which can be found here :

http://www.freebsd.org/cgi/query-pr.cgi?pr=162789

Could anyone take a look at it?

Regards,
--
Daan Vreeken
Vitsch Electronics
http://Vitsch.nl
tel: +31-(0)40-7113051 / +31-(0)6-46210825
KvK nr: 17174380
_______________________________________________
freebsd...@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-curre...@freebsd.org"

[Gleb Smirnoff]

On Thu, Nov 24, 2011 at 09:28:51AM +0100, Daan Vreeken wrote:
D> Hi All,
D>
D> Recently I've discovered a bug in if_clone.c and if.c where the code allows
D> multiple interfaces to be created with exactly the same name (which leads to
D> all sorts of other interesting problems).
D> I've submitted a PR about this with patches, which can be found here :
D>
D> http://www.freebsd.org/cgi/query-pr.cgi?pr=162789
D>
D> Could anyone take a look at it?

I'll try to handle this.

--
Totus tuus, Glebius.

[Gleb Smirnoff]

On Thu, Nov 24, 2011 at 09:28:51AM +0100, Daan Vreeken wrote:
D> Recently I've discovered a bug in if_clone.c and if.c where the code allows
D> multiple interfaces to be created with exactly the same name (which leads to
D> all sorts of other interesting problems).
D> I've submitted a PR about this with patches, which can be found here :
D>
D> http://www.freebsd.org/cgi/query-pr.cgi?pr=162789
D>

D> Could anyone take a look at it?

I decided to simply if_clone code utilizing generic unit allocator. Patch
atteched. Now I'll try to merge it with your ideas.

--
Totus tuus, Glebius.

[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.