Re: [PATCH v3 net] net: ipv4/ipv6 addrconf: call igmp{,6}_group_dropped() while dev is still up

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Wed, Apr 12, 2023 at 04:34:00PM +0300, Ido Schimmel wrote:
> Even with the proposed fix, wouldn't you get the same leak with the
> following reproducer?
> 
> ip link set dev swp0 up
> bridge fdb add 01:02:03:04:05:06 dev swp0 self local
> ip link set dev swp0 down
> echo 0000:00:00.5 > /sys/bus/pci/drivers/mscc_felix/unbind
> 
> If so, I wonder how other drivers that allocate memory in their
> ndo_set_rx_mode() deal with this problem. I would imagine that they
> flush the addresses in their ndo_stop() or as part of device dismantle.

Hmm, yeah, fair. I was operating on the premise that callers of
dev_mc_add()/dev_mc_del() would be sane enough to keep track of their
calls, and at least aspire to not leave something behind. Clearly not the
case here, if user space can add a unicast/multicast filter and run...

So I guess I'm left to clean up the stale addresses at DSA removal time,
and remove the WARN_ON().

Actually, looking at ndo_dflt_fdb_add() and ndo_dflt_fdb_del() again,
what concerns me more is this:

	if (is_unicast_ether_addr(addr) || is_link_local_ether_addr(addr))
		err = dev_uc_add_excl(dev, addr);

I.O.W. this:

$ bridge fdb add 01:80:c2:00:00:0e dev swp0 self local

generates a filtering call for a unicast address.
And I don't know about other drivers, but in DSA, we assume that unicast
addresses are unicast, and we have separate API for that vs multicast...