Skip to content

Commit 7d8dc1c

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: nf_queue: drop packets with cloned unconfirmed conntracks
Conntrack assumes an unconfirmed entry (not yet committed to global hash table) has a refcount of 1 and is not visible to other cores. With multicast forwarding this assumption breaks down because such skbs get cloned after being picked up, i.e. ct->use refcount is > 1. Likewise, bridge netfilter will clone broad/mutlicast frames and all frames in case they need to be flood-forwarded during learning phase. For ip multicast forwarding or plain bridge flood-forward this will "work" because packets don't leave softirq and are implicitly serialized. With nfqueue this no longer holds true, the packets get queued and can be reinjected in arbitrary ways. Disable this feature, I see no other solution. After this patch, nfqueue cannot queue packets except the last multicast/broadcast packet. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent e976713 commit 7d8dc1c

File tree

2 files changed

+38
-3
lines changed

2 files changed

+38
-3
lines changed

net/bridge/br_netfilter_hooks.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,12 @@ static unsigned int br_nf_local_in(void *priv,
622622
if (likely(nf_ct_is_confirmed(ct)))
623623
return NF_ACCEPT;
624624

625+
if (WARN_ON_ONCE(refcount_read(&nfct->use) != 1)) {
626+
nf_reset_ct(skb);
627+
return NF_ACCEPT;
628+
}
629+
625630
WARN_ON_ONCE(skb_shared(skb));
626-
WARN_ON_ONCE(refcount_read(&nfct->use) != 1);
627631

628632
/* We can't call nf_confirm here, it would create a dependency
629633
* on nf_conntrack module.

net/netfilter/nfnetlink_queue.c

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -820,10 +820,41 @@ static bool nf_ct_drop_unconfirmed(const struct nf_queue_entry *entry)
820820
{
821821
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
822822
static const unsigned long flags = IPS_CONFIRMED | IPS_DYING;
823-
const struct nf_conn *ct = (void *)skb_nfct(entry->skb);
823+
struct nf_conn *ct = (void *)skb_nfct(entry->skb);
824+
unsigned long status;
825+
unsigned int use;
824826

825-
if (ct && ((ct->status & flags) == IPS_DYING))
827+
if (!ct)
828+
return false;
829+
830+
status = READ_ONCE(ct->status);
831+
if ((status & flags) == IPS_DYING)
826832
return true;
833+
834+
if (status & IPS_CONFIRMED)
835+
return false;
836+
837+
/* in some cases skb_clone() can occur after initial conntrack
838+
* pickup, but conntrack assumes exclusive skb->_nfct ownership for
839+
* unconfirmed entries.
840+
*
841+
* This happens for br_netfilter and with ip multicast routing.
842+
* We can't be solved with serialization here because one clone could
843+
* have been queued for local delivery.
844+
*/
845+
use = refcount_read(&ct->ct_general.use);
846+
if (likely(use == 1))
847+
return false;
848+
849+
/* Can't decrement further? Exclusive ownership. */
850+
if (!refcount_dec_not_one(&ct->ct_general.use))
851+
return false;
852+
853+
skb_set_nfct(entry->skb, 0);
854+
/* No nf_ct_put(): we already decremented .use and it cannot
855+
* drop down to 0.
856+
*/
857+
return true;
827858
#endif
828859
return false;
829860
}

0 commit comments

Comments
 (0)