netdev - Re: [PATCH net] openvswitch: get related ct labels from its master if it is not confirmed

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]

Message-ID: <89ce2aef-6ec4-4ab1-b180-f390188170f7@ovn.org>
Date: Thu, 20 Jun 2024 19:14:17 +0200
From: Ilya Maximets <i.maximets@....org>
To: Xin Long <lucien.xin@...il.com>, network dev <netdev@...r.kernel.org>,
 dev@...nvswitch.org
Cc: i.maximets@....org, davem@...emloft.net, kuba@...nel.org,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Pravin B Shelar <pshelar@....org>, Aaron Conole <aconole@...hat.com>,
 Florian Westphal <fw@...len.de>,
 Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Subject: Re: [PATCH net] openvswitch: get related ct labels from its master if
 it is not confirmed

On 6/20/24 00:08, Xin Long wrote:
> Ilya found a failure in running check-kernel tests with at_groups=144
> (144: conntrack - FTP SNAT orig tuple) in OVS repo. After his further
> investigation, the root cause is that the labels sent to userspace
> for related ct are incorrect.
> 
> The labels for unconfirmed related ct should use its master's labels.
> However, the changes made in commit 8c8b73320805 ("openvswitch: set
> IPS_CONFIRMED in tmpl status only when commit is set in conntrack")
> led to getting labels from this related ct.
> 
> So fix it in ovs_ct_get_labels() by changing to copy labels from its
> master ct if it is a unconfirmed related ct. Note that there is no
> fix needed for ct->mark, as it was already copied from its master
> ct for related ct in init_conntrack().
> 
> Fixes: 8c8b73320805 ("openvswitch: set IPS_CONFIRMED in tmpl status only when commit is set in conntrack")
> Reported-by: Ilya Maximets <i.maximets@....org>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> ---
>  net/openvswitch/conntrack.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 331730fd3580..920e802ff01e 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -167,8 +167,13 @@ static u32 ovs_ct_get_mark(const struct nf_conn *ct)
>  static void ovs_ct_get_labels(const struct nf_conn *ct,
>  			      struct ovs_key_ct_labels *labels)
>  {
> -	struct nf_conn_labels *cl = ct ? nf_ct_labels_find(ct) : NULL;
> +	struct nf_conn_labels *cl = NULL;
>  
> +	if (ct) {
> +		if (ct->master && !nf_ct_is_confirmed(ct))
> +			ct = ct->master;
> +		cl = nf_ct_labels_find(ct);
> +	}
>  	if (cl)
>  		memcpy(labels, cl->bits, OVS_CT_LABELS_LEN);
>  	else

Thanks, Xin!  LGTM.

Tested with OVS testsuite and it works fine.  Also re-checked OVN
system tests and they also work as expected.

Reviewed-by: Ilya Maximets <i.maximets@....org>
Tested-by: Ilya Maximets <i.maximets@....org>