On Tue, May 12, 2026 at 1:29 AM Nikolay Aleksandrov <[email protected]> wrote: > > On 12/05/2026 07:31, Ren Wei wrote: > > From: Nan Li <[email protected]> > > > > The bridge local receive path may be deferred by netfilter and resumed > > later. By the time br_handle_local_finish() runs, skb->dev may still be > > valid while its bridge port association has already been removed. > > > > br_handle_local_finish() unconditionally looks up the bridge port from > > skb->dev and dereferences it for source learning. If the port is no > > longer attached to the bridge, the lookup returns NULL and the deferred > > local receive path can no longer rely on the port state being present. > > > > Skip the learning step when the bridge port lookup fails. In that case > > there is no port state left to learn on, so returning early preserves > > the normal behavior for existing ports while avoiding access to stale > > state. > > > > Fixes: 8626c56c8279 ("bridge: fix potential use-after-free when hook > > returns QUEUE or STOLEN verdict") > > I don't think that is the correct commit, it seems to me this bug > has existed for a very long time. From a quick search I think (Florian > please correct me if I'm wrong, but I think NF_QUEUE existed back then) > it was introduced in 2010 by: > f350a0a87374 ("bridge: use rx_handler_data pointer to store net_bridge_port > pointer")
After checking the history, I believe f350a0a87374 is indeed the commit that introduced the underlying root cause. The 8626c56c8279 commit in the patch is the one that actually made the bug reachable in practice. I am a bit unsure which commit should be used in the Fixes: tag. We have run into this situation several times already, where the commit that introduced the root cause is different from the commit that actually made the bug triggerable/reachable. What do you think would be best? Also, if the Fixes: tag should be changed, would you prefer that we resend the patch, or would you rather have the committer adjust the Fixes: tag when applying it in order to reduce traffic on netdev? > > because that commit removed the same check for a NULL port. > The patch itself is ok, it restores the check that was there before the commit > I mentioned. > > > Cc: [email protected] > > Reported-by: Yuan Tan <[email protected]> > > Reported-by: Yifan Wu <[email protected]> > > Reported-by: Juefei Pu <[email protected]> > > Reported-by: Xin Liu <[email protected]> > > Signed-off-by: Nan Li <[email protected]> > > Signed-off-by: Ren Wei <[email protected]> > > --- > > net/bridge/br_input.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > index 2cbae0f9ae1f..5b0d7450de5f 100644 > > --- a/net/bridge/br_input.c > > +++ b/net/bridge/br_input.c > > @@ -247,6 +247,9 @@ static void __br_handle_local_finish(struct sk_buff > > *skb) > > struct net_bridge_port *p = br_port_get_rcu(skb->dev); > > u16 vid = 0; > > > > + if (unlikely(!p)) > > + return; > > + > > /* check if vlan is allowed, to avoid spoofing */ > > if ((p->flags & BR_LEARNING) && > > nbp_state_should_learn(p) && > >
