From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 6D38D1B808 for ; Wed, 30 Sep 2026 00:44:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790729052; cv=none; b=Kwqhx8HZrZ9+C9KUWlaCgrbfco0jJqNC1KEiLyeGFWFf7Pag1AW2t1qHE4LCOHD0ignUiSJMtFL91ZG3NwHn4jmXwfJa7G7i9145B3ozy/hGaE6Ygwt1ESr2sdeYyK1Uow7ZQmo+0ATwxudlE8txRNWyUrOgRaNCU8KNcSTgtsE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790729052; c=relaxed/simple; bh=MXEH5lX4uCsmvdgRwbk/8b4x0mJWyrMhmkt+0S4fJjs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=SlpfyXvp/iJcjjmuRegA9LGPldz0bzh5GDFFOYgIDRiA1Xc9FKkDNOtnA20ogIr4p/1QIMLdFCO5YZZdiTv9uScXl9NRG+Cpbc8G8YOOxdwATcGPHkdSUmlkix5+z5F3naRjs12QY19nKRvnonN1mLAsmnzNrH6mzTdDAN4WtfY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=YZFEw5uH; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="YZFEw5uH" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 843241F00899; Wed, 30 Sep 2026 00:44:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790729051; bh=JaHO8r5JqHvMV5ty5iqb5pm9iSKPb8GHpkYm46RwW0s=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=YZFEw5uH/ohiBjK9HlV2FhTo0hdwUleJbjG0juvbBThb8vb+Wd5xuQzfhbMLxEcbz jx5GO/Ael8ucy0QDBRmsAwxK9WaNQxmPYySxnQAo/RHy/TjzU5+Ie1JfNlh6xQIShf VO5eXBrfHRuyHrlueG6MWlyNCyQRgb3t/93bbl7cWi6omT3JKaNew0me+UH5RCW+oq I4ie4//bH6LB26dn7EDOQnKmvW/aKPJfu1aaIFHRMe0R5PI65dAGB7k9/rD6YGzanM mHuaJfOpG8+Sh3Out7mNJvc7nYCdo6rkY8wOxJiwkvw5RUJcAsoIQk3AElA1zWxLzJ /FBzmHefu+E7g== From: Sasha Levin To: stable@vger.kernel.org Cc: Ilya Maximets , Axel Mierczuk , Aaron Conole , Xin Long , Jamal Hadi Salim , Jakub Kicinski , Sasha Levin Subject: [PATCH 6.6.y 2/2] net/sched: act_ct: avoid modifying shared unconfirmed ct entry Date: Tue, 29 Sep 2026 20:44:08 -0400 Message-ID: <20260930004408.2953522-2-sashal@kernel.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260930004408.2953522-1-sashal@kernel.org> References: <2026092928-preacher-reconvene-df6d@gregkh> <20260930004408.2953522-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: Ilya Maximets [ Upstream commit f85009dfcd65e5969526b0db7a49b5413746e630 ] In a case where skb with an unconfirmed ct entry gets cloned, we may end up processing both again but with different sets of extensions. The series of events: 1. The first clone wants to commit and runs the helpers wiring up the extension pointer into the expectation list. 2. Then it looses the confirmation keeping the entry unconfirmed. 3. Second clone now wants to commit labels or run NAT and adds the new extension for that breaking the pointer in the expectation list causing UAF on the destruction path later. While this is possible to trigger, there should be no practical network pipeline where we need to process both clones without modifications in the same zone. So, let's just reset the entry in case for some reason we got an skb with a shared one. This doesn't affect any known use cases, but avoids any potential problems with sharing and modification of the unconfirmed ct entry. Unlike openvswitch module, act_ct allows for NAT without commit. Changing that would be a uAPI break. So, act_ct needs to reset on NAT regardless of the commit flag to avoid reallocation of the extension space. This, however, doesn't really change the picture for sensible networking cases as there should be no need to run the same packet twice (before and after the clone) through conntrack without packet header or zone changes and without commit. The fixes tag points to the introduction of helpers, since that's the main UAF trigger for the sharing. Fixes: a21b06e73191 ("net: sched: add helper support in act_ct") Cc: stable@vger.kernel.org Reported-by: Axel Mierczuk Signed-off-by: Ilya Maximets Reviewed-by: Aaron Conole Reviewed-by: Xin Long Reviewed-by: Jamal Hadi Salim Link: https://patch.msgid.link/20260921145655.3167436-5-i.maximets@ovn.org Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- net/sched/act_ct.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index 7a0dff24c5d15..20414e4513dbb 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -974,11 +974,11 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res) { struct net *net = dev_net(skb->dev); + bool cached, commit, clear, nat; enum ip_conntrack_info ctinfo; struct tcf_ct *c = to_ct(a); struct nf_conn *tmpl = NULL; struct nf_hook_state state; - bool cached, commit, clear; int nh_ofs, err, retval; struct tcf_ct_params *p; bool add_helper = false; @@ -993,6 +993,7 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, retval = READ_ONCE(c->tcf_action); commit = p->ct_action & TCA_CT_ACT_COMMIT; clear = p->ct_action & TCA_CT_ACT_CLEAR; + nat = p->ct_action & TCA_CT_ACT_NAT; tmpl = p->tmpl; tcf_lastuse_update(&c->tcf_tm); @@ -1041,6 +1042,19 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, * different zone. */ cached = tcf_ct_skb_nfct_cached(net, skb, p); + + /* If the ct entry is not confirmed and shared with some other skb, + * e.g., a cloned one, we can't just modify it with a commit or nat + * as we must not modify the extension set. Reset. + */ + if (cached && (commit || nat)) { + ct = nf_ct_get(skb, &ctinfo); + if (ct && !nf_ct_is_confirmed(ct) && nf_ct_shared(ct)) { + nf_reset_ct(skb); + cached = false; + } + } + if (!cached) { if (tcf_ct_flow_table_lookup(p, skb, family)) { skip_add = true; @@ -1078,7 +1092,7 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, if (err) goto drop; add_helper = true; - if (p->ct_action & TCA_CT_ACT_NAT && !nfct_seqadj(ct)) { + if (nat && !nfct_seqadj(ct)) { if (!nfct_seqadj_ext_add(ct)) goto drop; } -- 2.53.0