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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Oct 2022 15:33:35 +0200 From: Paolo Abeni <pabeni@...hat.com> To: Xin Long <lucien.xin@...il.com>, network dev <netdev@...r.kernel.org>, dev@...nvswitch.org Cc: davem@...emloft.net, kuba@...nel.org, Eric Dumazet <edumazet@...gle.com>, Pravin B Shelar <pshelar@....org>, Florian Westphal <fw@...len.de>, Pablo Neira Ayuso <pablo@...filter.org>, Ilya Maximets <i.maximets@....org>, Eelco Chaudron <echaudro@...hat.com>, Yi-Hung Wei <yihung.wei@...il.com> Subject: Re: [PATCH net] openvswitch: add nf_ct_is_confirmed check before assigning the helper Hello, On Thu, 2022-10-06 at 15:45 -0400, Xin Long wrote: > A WARN_ON call trace would be triggered when 'ct(commit, alg=helper)' > applies on a confirmed connection: > > WARNING: CPU: 0 PID: 1251 at net/netfilter/nf_conntrack_extend.c:98 > RIP: 0010:nf_ct_ext_add+0x12d/0x150 [nf_conntrack] > Call Trace: > <TASK> > nf_ct_helper_ext_add+0x12/0x60 [nf_conntrack] > __nf_ct_try_assign_helper+0xc4/0x160 [nf_conntrack] > __ovs_ct_lookup+0x72e/0x780 [openvswitch] > ovs_ct_execute+0x1d8/0x920 [openvswitch] > do_execute_actions+0x4e6/0xb60 [openvswitch] > ovs_execute_actions+0x60/0x140 [openvswitch] > ovs_packet_cmd_execute+0x2ad/0x310 [openvswitch] > genl_family_rcv_msg_doit.isra.15+0x113/0x150 > genl_rcv_msg+0xef/0x1f0 > > which can be reproduced with these OVS flows: > > table=0, in_port=veth1,tcp,tcp_dst=2121,ct_state=-trk > actions=ct(commit, table=1) > table=1, in_port=veth1,tcp,tcp_dst=2121,ct_state=+trk+new > actions=ct(commit, alg=ftp),normal > > The issue was introduced by commit 248d45f1e193 ("openvswitch: Allow > attaching helper in later commit") where it somehow removed the check > of nf_ct_is_confirmed before asigning the helper. This patch is to fix > it by bringing it back. > > Fixes: 248d45f1e193 ("openvswitch: Allow attaching helper in later commit") > Reported-by: Pablo Neira Ayuso <pablo@...filter.org> > Signed-off-by: Xin Long <lucien.xin@...il.com> > --- > net/openvswitch/conntrack.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index 4e70df91d0f2..6862475f0f88 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -1015,7 +1015,8 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, > * connections which we will commit, we may need to attach > * the helper here. > */ > - if (info->commit && info->helper && !nfct_help(ct)) { > + if (!nf_ct_is_confirmed(ct) && info->commit && > + info->helper && !nfct_help(ct)) { > int err = __nf_ct_try_assign_helper(ct, info->ct, > GFP_ATOMIC); > if (err) The patch LGTM, but it would be great if someone from the OVS side could confirm this does not break existing use-case. Thanks! Paolo
Powered by blists - more mailing lists