> On Fri, 16 Jun 2017 17:23:51 +0300 > Serhey Popovych <serhe.popov...@gmail.com> wrote: > >> Interface index is signed integer, we can pass ifm->ifi_index >> from userspace via netlink and create network device with >> negative ifindex value. >> >> Fixes: 9c7dafbfab15 ("net: Allow to create links with given ifindex") >> Signed-off-by: Serhey Popovych <serhe.popov...@gmail.com> >> --- >> net/core/dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/core/dev.c b/net/core/dev.c >> index 8658074..dae8010 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -7491,7 +7491,7 @@ int register_netdevice(struct net_device *dev) >> } >> >> ret = -EBUSY; >> - if (!dev->ifindex) >> + if (dev->ifindex <= 0) >> dev->ifindex = dev_new_index(net); >> else if (__dev_get_by_index(net, dev->ifindex)) >> goto err_uninit; > > You should fix this by adding error check in the netlink portion > that allows creating devices with given ifindex. Passing < 0 > should be an error.But should this break some setups if I add such check to > netlink portion? In my opinion it is better to choose silently different ifindex rather than reporting failure. That's why I prefer doing this in register_netdevice().
Also there is similar problem for drivers/net/veth.c, it might happen that other places will be added later that setup dev->ifindex and then call register_netdevice(). What do you think? Thanks for quick review! > -- Thanks, Serhey