netdev - Re: [PATCH net] openvswitch: avoid allocating labels_ext in ovs_ct_set

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: <295e2902-9036-46c9-a110-bf5bf27ed473@nvidia.com>
Date: Wed, 5 Mar 2025 09:30:42 +0800
From: Jianbo Liu <jianbol@...dia.com>
To: Xin Long <lucien.xin@...il.com>, network dev <netdev@...r.kernel.org>,
	<dev@...nvswitch.org>, <ovs-dev@...nvswitch.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, Eric Dumazet
	<edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Pravin B Shelar
	<pshelar@....org>, Ilya Maximets <i.maximets@....org>, Aaron Conole
	<aconole@...hat.com>, Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
	Florian Westphal <fw@...len.de>
Subject: Re: [PATCH net] openvswitch: avoid allocating labels_ext in
 ovs_ct_set_labels



On 3/5/2025 1:15 AM, Xin Long wrote:
> Currently, ovs_ct_set_labels() is only called for *confirmed* conntrack
> entries (ct) within ovs_ct_commit(). However, if the conntrack entry
> does not have the labels_ext extension, attempting to allocate it in
> ovs_ct_get_conn_labels() for a confirmed entry triggers a warning in
> nf_ct_ext_add():
> 
>    WARN_ON(nf_ct_is_confirmed(ct));
> 
> This happens when the conntrack entry is created externally before OVS
> increases net->ct.labels_used. The issue has become more likely since
> commit fcb1aa5163b1 ("openvswitch: switch to per-action label counting
> in conntrack"), which switched to per-action label counting.
> 
> To prevent this warning, this patch modifies ovs_ct_set_labels() to
> call nf_ct_labels_find() instead of ovs_ct_get_conn_labels() where
> it allocates the labels_ext if it does not exist, aligning its
> behavior with tcf_ct_act_set_labels().
> 
> Fixes: fcb1aa5163b1 ("openvswitch: switch to per-action label counting in conntrack")
> Reported-by: Jianbo Liu <jianbol@...dia.com>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> ---
>   net/openvswitch/conntrack.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 3bb4810234aa..f13fbab4c942 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -426,7 +426,7 @@ static int ovs_ct_set_labels(struct nf_conn *ct, struct sw_flow_key *key,
>   	struct nf_conn_labels *cl;
>   	int err;
>   
> -	cl = ovs_ct_get_conn_labels(ct);
> +	cl = nf_ct_labels_find(ct);

I don't think it's correct fix. The label is not added and packets can't 
pass the next rule to match ct_label.

I tested it and used the configuration posted before, ping can't work.

>   	if (!cl)
>   		return -ENOSPC;
>