Commit 3c186054 authored by Xin Long's avatar Xin Long Committed by Jakub Kicinski

openvswitch: add nf_ct_is_confirmed check before assigning the helper

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 248d45f1 ("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: 248d45f1 ("openvswitch: Allow attaching helper in later commit")
Reported-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Acked-by: default avatarAaron Conole <aconole@redhat.com>
Tested-by: default avatarAaron Conole <aconole@redhat.com>
Link: https://lore.kernel.org/r/c5c9092a22a2194650222bffaf786902613deb16.1665085502.git.lucien.xin@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 4f0f2121
...@@ -1015,7 +1015,8 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, ...@@ -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 * connections which we will commit, we may need to attach
* the helper here. * 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, int err = __nf_ct_try_assign_helper(ct, info->ct,
GFP_ATOMIC); GFP_ATOMIC);
if (err) if (err)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment