Re: [RFC PATCH] net: bridge: call br_multicast_del_port before the port leaves

[Vladimir Oltean]

On Fri, Oct 16, 2020 at 01:43:06PM +0000, Nikolay Aleksandrov wrote:
> It can potentially use after free, multicast resources (per-cpu stats) are freed
> in br_multicast_del_port() and can be used due to a race with port state
> sync on other CPUs since the handler can still process packets. That has a
> chance of happening if vlans are not used.

Interesting, thanks for pointing this out, I haven't observed
use-after-free in my limited testing of this patch.

> Interesting that br_stp_disable_port() calls br_multicast_disable_port() which
> flushes all non-permanent mdb entries, so I'm guessing you have problem only
> with permanent ones?

Indeed, I'm testing out your L2 multicast patch.

> Perhaps we can flush them all before. Either by passing an argument to
> br_stp_disable_port() that we're deleting the port which will be
> passed down to br_multicast_disable_port() or by calling an additional
> helper to flush all which can be re-used by both disable_port() and
> stop_multicast() calls. Adding an argument to br_stp_disable_port() to
> be passed down sounds cleaner to me. What do you think?

That sounds a bit complicated, to be honest.
In fact, the reason why I submitted this as RFC only is because it isn't
solving all my problems. You know that saying "- it hurts when I do that
- then don't do that"? I think I can just change the ocelot driver to
stop remapping the untagged MDB entries to its pvid, and then I can drop
all my charges to the bridge driver.