Re: [PATCH net] net_sched: act_ct: drop all packets when not attached to ingress

[Paolo Abeni <pabeni@redhat.com>]

Adding Florian, too

On 2/17/26 3:49 PM, Ilya Maximets wrote:
> On 2/17/26 10:38 AM, Paolo Abeni wrote:
>> Since the blamed commit below, classify can return TC_ACT_CONSUMED while
>> the current skb being held by the defragmentation engine. As reported by
>> GangMin Kim, if such packet is that may cause a UaF when the defrag engine
>> later on tries to tuch again such packet.
>>
>> act_ct was never meant to be used outside of the ingress path. Making
>> defrag really works for act_ct outside such constraints range from very
>> difficult to completely impossible.
>>
>> Address the issue making act_ct drop any packet when not attached to the
>> ingress path and additionally emit a warning about the bad
>> configuration.
>>
>> Reported-by: GangMin Kim <km.kim1503@gmail.com>
>> Fixes: 8f9516daedd6 ("sched: Add enqueue/dequeue of dualpi2 qdisc")
>> CC: stable@vger.kernel.org
>> Link: https://patch.msgid.link/16f6b264373ad60ab18eb8525809e7267442afa7.1770394932.git.pabeni@redhat.com
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> Catching the bad configuration at runtime instead of init time to reduce
>> complexity
>> ---
>>  net/sched/act_ct.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
>> index 81d488655793..e8eb0d195f4a 100644
>> --- a/net/sched/act_ct.c
>> +++ b/net/sched/act_ct.c
>> @@ -987,6 +987,11 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
>>  	tcf_lastuse_update(&c->tcf_tm);
>>  	tcf_action_update_bstats(&c->common, skb);
>>  
>> +	if (!skb_at_tc_ingress(skb)) {
>> +		pr_warn_once("act_CT should be attached at ingress!\n");
> 
> Hi, Paolo.  I didn't test this yet, but I'm a little concerned about this
> change.  For TC offload of OVS tunnels, we create egress qdisc for internal
> ports, a.k.a. bridge ports.  This is necessary for tunnels to work.
> A typical setup looks like this:
> 
>     Bridge br-int (10.0.0.1)
>         Port br-int
>             Interface br-int
>                 type: internal
>         Port vm
>             Interface vm
>         Port vxlan0
>             Interface vxlan0
>                 type: vxlan
>                 options: {remote_ip="172.31.1.1"}
> 
>     Bridge br-phy (172.31.1.100)
>         Port br-phy
>             Interface br-phy
>                 type: internal
>         Port eth0
>             Interface eth0
> 
> When a VM sends a packet that supposed to go through the tunnel, it enters
> from the vm port and OVS forwards it into vxlan0 LWT with an appropriate
> tunnel metadata.  Then it gets encapsulated and put into kernel routing to
> find the next hop, which is via br-phy bridge.  Packet enters br-phy and
> OVS forwards the packet to eth0.  There can be stateful processing in the
> br-phy bridge involving passing packets through conntrack.
> 
> With the TC offload enabled, OVS creates following filters:
> 
> 1. Ingress filter on vm interface that forwards packets to vxlan0.
> 2. Egress filter on br-phy interface that forwards encapsulated packets
>    to eth0 interface.
> 
> If some stateful processing is involved, both of those could have act_ct
> in them.
> 
> AFAIU, we have to use Egress qdisc for the br-phy, because the packet is
> egressing from the kernel routing via br-phy and the ingress qdisc is not
> invoked.  Ingress will be at play when packets are flowing in the opposite
> direction from eth0 to br-phy, as that's where they will ingress into
> standard kernel routing.
> 
> If act_tc will not be allowed on Egress, then stateful processing will
> not be possible in this case in br-phy bridge.
> 
> Thoughts?

This looks very problematic. A slightly different patch tried to
somewhat preserve functionality, but it simply can't work in presence of
IP fragments.

Why stateful processing on  br-int/port vm ingress is not enough?

/P