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.