Yuan Tan <[email protected]> wrote:
> > >>> 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.
> > You should update the Fixes: tag but also wait 24h before re-posting another
> > patch version.
>
> Ok we will send the v2 with fix tag f350a0a87374 after 24h
Would you mind exploring an alternative fix for this?
As nfnetlink_queue'd skbs leave rcu read locked section, great
care has to be taken on reinject.
Either bridge could call nf_queue_nf_hook_drop() on bridge
port removal, or nfqnl_reinject() could revalidate that skb->dev
is part of a bridge and munge verdict to NF_DROP in case
the assocication was removed while packet was out.
static bool nfqnl_bridge_port_removed(const struct nf_queue_entry *e)
{
return e->state.pf == NFPROTO_BRIDGE &&
!br_port_get_rcu(e->skb->dev) == NULL;
}
static void nfqnl_reinject(struct nf_queue_entry *entry, unsigned int verdict)
{
const struct nf_ct_hook *ct_hook;
if (verdict == NF_ACCEPT ||
verdict == NF_REPEAT ||
verdict == NF_STOP) {
unsigned int ct_verdict = verdict;
....
if (nfqnl_bridge_port_removed(entry))
verdict = NF_DROP;
Its more work, but it places the extra checks to where
they are really needed.
Also see related bug fix: At this time, entire bridge device can go
away while packet is out.
https://patchwork.ozlabs.org/project/netfilter-devel/patch/ca7ee343bbcb44905e1f5b853df2f3a5b7d40548.1778493188.git.royenhe...@gmail.com/
