Hi Nikolay,
Thanks for the comments.
On Thu, Apr 26, 2018 at 05:22:46PM +0300, Nikolay Aleksandrov wrote:
> > Not all upper devs are masters. This can break some setups.
Ah, like vlan device.. So how about
+ if (netdev_master_upper_dev_get(dev))
return -EBUSY;
> >
> >
>
> Also it's not really a bug, the device begins to get initialized but it
> will get removed at netdev_master_upper_dev_link() anyway if there's
> already a master. Why would it be better ?
> It's clearly wrong to try and enslave a device that already has a master
> via ioctl, rtnetlink already deals with that and the old ioctl interface
> will get an error, yes it will initialize some structs but they'll get
> freed later. This is common practice, check the bonding for example.
Bonding use netdev_is_rx_handler_busy(slave_dev) to check if the slave
already has a master, which is another solution.
>
> If anything do the check in the ioctl interface (add_del_if) only and
> maybe target net-next, there's really no bug fix here. IMO it's not
What if someone do like
while true; do brctl addif br0 bond_slave &; done
I know this is stupid and almost no one will do that in real world.
But syzbot run some similar test and get warn from kobject_add_internal()
with -ENOMEM. That's why I think we should fix it before allocate any
resource.
What do you think?
[1]
https://syzkaller.appspot.com/bug?id=3e0339080acd6a2a350a900bc6533b03f5498490
Thanks
Hangbin
