From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 52FDA1C578D; Tue, 27 Aug 2024 14:44:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724769885; cv=none; b=B0G0k7upnlxexEHN+RKdsjOvB/Bze33eB2ajENfecNCfsDsQzGeubbyT5yYCk13rYgWasaf7imdA3BvfqTNoaXHEHq+u0Iy+/cIhdU87E+0IrX9VLWhayfXoesMfFKDP15444qTfjB+POL8xicyTULyjbGXi/J4hhxSUQJsiAVM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724769885; c=relaxed/simple; bh=EHrcsOM3DhPbgWq24e1TzTvpqlLIURR7ZaSC8OAMH1c=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=SRC/HFQvG/VBzlc8gmAVU/k23E+NPvtzbfOd/CQ4q1PPmzdP6EO2ZDRWECvdeCOCSIXyjyTe4R9e7MuPQbYzn+QVWIBcn+P64CbDQ0DlFQ6boDmT4M9PPIN4FOWcFhBnNh6b7LvhCLJqbSq7zklhP5UCZgaoTF1pU0rcVMmX6pc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=wSGP+p0S; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="wSGP+p0S" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A6794C6105D; Tue, 27 Aug 2024 14:44:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1724769885; bh=EHrcsOM3DhPbgWq24e1TzTvpqlLIURR7ZaSC8OAMH1c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wSGP+p0SjfDjeO2LCo6euy2FcbVOmEol1CJtOQKHGOj2GjKh53S4f0gyzkGSXib4S EZrmIxElLTlmjcqmsk2lgKlJH0K+Lq4iSyl7ZV1BJLqdvpL9ZGdTGW3FHPayidelg7 +PxOCiQDlWzPUpXYBcxjjTWM2kcKwY12Voo2utfo= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Florian Westphal , Pablo Neira Ayuso , Sasha Levin Subject: [PATCH 6.6 065/341] netfilter: nf_queue: drop packets with cloned unconfirmed conntracks Date: Tue, 27 Aug 2024 16:34:56 +0200 Message-ID: <20240827143845.887864271@linuxfoundation.org> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240827143843.399359062@linuxfoundation.org> References: <20240827143843.399359062@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 6.6-stable review patch. If anyone has any objections, please let me know. ------------------ From: Florian Westphal [ Upstream commit 7d8dc1c7be8d3509e8f5164dd5df64c8e34d7eeb ] 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 Signed-off-by: Pablo Neira Ayuso Signed-off-by: Sasha Levin --- net/bridge/br_netfilter_hooks.c | 6 +++++- net/netfilter/nfnetlink_queue.c | 35 +++++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c index d848c84ed030d..68d5538613032 100644 --- a/net/bridge/br_netfilter_hooks.c +++ b/net/bridge/br_netfilter_hooks.c @@ -618,8 +618,12 @@ static unsigned int br_nf_local_in(void *priv, if (likely(nf_ct_is_confirmed(ct))) return NF_ACCEPT; + if (WARN_ON_ONCE(refcount_read(&nfct->use) != 1)) { + nf_reset_ct(skb); + return NF_ACCEPT; + } + WARN_ON_ONCE(skb_shared(skb)); - WARN_ON_ONCE(refcount_read(&nfct->use) != 1); /* We can't call nf_confirm here, it would create a dependency * on nf_conntrack module. diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index dfc856b3e1fa4..09209b4952ad1 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -668,10 +668,41 @@ static bool nf_ct_drop_unconfirmed(const struct nf_queue_entry *entry) { #if IS_ENABLED(CONFIG_NF_CONNTRACK) static const unsigned long flags = IPS_CONFIRMED | IPS_DYING; - const struct nf_conn *ct = (void *)skb_nfct(entry->skb); + struct nf_conn *ct = (void *)skb_nfct(entry->skb); + unsigned long status; + unsigned int use; - if (ct && ((ct->status & flags) == IPS_DYING)) + if (!ct) + return false; + + status = READ_ONCE(ct->status); + if ((status & flags) == IPS_DYING) return true; + + if (status & IPS_CONFIRMED) + return false; + + /* in some cases skb_clone() can occur after initial conntrack + * pickup, but conntrack assumes exclusive skb->_nfct ownership for + * unconfirmed entries. + * + * This happens for br_netfilter and with ip multicast routing. + * We can't be solved with serialization here because one clone could + * have been queued for local delivery. + */ + use = refcount_read(&ct->ct_general.use); + if (likely(use == 1)) + return false; + + /* Can't decrement further? Exclusive ownership. */ + if (!refcount_dec_not_one(&ct->ct_general.use)) + return false; + + skb_set_nfct(entry->skb, 0); + /* No nf_ct_put(): we already decremented .use and it cannot + * drop down to 0. + */ + return true; #endif return false; } -- 2.43.0