Re: [PATCH net] bonding: Fix bonding crash

On 9/2/16 8:45 AM, Eric Dumazet wrote:

On Fri, 2016-09-02 at 08:30 -0600, David Ahern wrote:

quoted

This check duplicates what netdev_rx_handler_register does. Why not
move the call to netdev_rx_handler_register here and then call
unregister on failure paths?

As soon as you call netdev_rx_handler_register(), incoming packets will
hit your driver and we'll likely crash since the enslaving is not done
yet.

Really think about RCU, we do rcu_assign_pointer() because we want all
prior changes being committed to memory before 'enabling' readers to see
the updated rcu protected pointer.

There are 9 call sites where netdev_rx_handler_register() is used.

We can get rid of the extra check in netdev_rx_handler_register() once
all of them are using netdev_is_rx_handler_busy()

Since this patch takes care of bonding only, we need to keep the
existing check in netdev_rx_handler_register()

Anyway, we are speaking of control function, an extra check is simply
safer, like all the ASSERT_RTNL() we do have...

I hit this same problem yesterday but with the bridge. I forgot I had a macvlan device on an interface and tried to enslave it to a bridge. It failed with EBUSY without crashing the kernel so it is one example that handles the conflict, and the bridge also calls the register before the enslaving is done.