netdev - Re: [PATCH net-next 2/3] net: sched: set IPS_CONFIRMED in tmpl status only when commit is set in act

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 19 Jul 2023 12:07:46 -0400
From: Aaron Conole <aconole@...hat.com>
To: Xin Long <lucien.xin@...il.com>
Cc: network dev <netdev@...r.kernel.org>,  dev@...nvswitch.org,
  davem@...emloft.net,  kuba@...nel.org,  Eric Dumazet
 <edumazet@...gle.com>,  Paolo Abeni <pabeni@...hat.com>,  Pravin B Shelar
 <pshelar@....org>,  Jamal Hadi Salim <jhs@...atatu.com>,  Cong Wang
 <xiyou.wangcong@...il.com>,  Jiri Pirko <jiri@...nulli.us>,  Pablo Neira
 Ayuso <pablo@...filter.org>,  Florian Westphal <fw@...len.de>,  Marcelo
 Ricardo Leitner <marcelo.leitner@...il.com>,  Davide Caratti
 <dcaratti@...hat.com>
Subject: Re: [PATCH net-next 2/3] net: sched: set IPS_CONFIRMED in tmpl
 status only when commit is set in act_ct

Xin Long <lucien.xin@...il.com> writes:

> With the following flows, the packets will be dropped if OVS TC offload is
> enabled.
>
>   'ip,ct_state=-trk,in_port=1 actions=ct(zone=1)'
>   'ip,ct_state=+trk+new+rel,in_port=1 actions=ct(commit,zone=1)'
>   'ip,ct_state=+trk+new+rel,in_port=1 actions=ct(commit,zone=2),normal'
>
> In the 1st flow, it finds the exp from the hashtable and removes it then
> creates the ct with this exp in act_ct. However, in the 2nd flow it goes
> to the OVS upcall at the 1st time. When the skb comes back from userspace,
> it has to create the ct again without exp(the exp was removed last time).
> With no 'rel' set in the ct, the 3rd flow can never get matched.
>
> In OVS conntrack, it works around it by adding its own exp lookup function
> ovs_ct_expect_find() where it doesn't remove the exp. Instead of creating
> a real ct, it only updates its keys with the exp and its master info. So
> when the skb comes back, the exp is still in the hashtable.
>
> However, we can't do this trick in act_ct, as tc flower match is using a
> real ct, and passing the exp and its master info to flower parsing via
> tc_skb_cb is also not possible (tc_skb_cb size is not big enough).
>
> The simple and clear fix is to not remove the exp at the 1st flow, namely,
> not set IPS_CONFIRMED in tmpl when commit is not set in act_ct.
>
> Reported-by: Shuang Li <shuali@...hat.com>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> ---

Acked-by: Aaron Conole <aconole@...hat.com>