I don't think the patch will resolve the problem. Once ctb->lock is released, other thread may have chance to acquire the lock and modify ctb. In general, ctb->lock can not be released in this function, another approach is needed.
On Wed, Aug 29, 2018 at 3:31 PM Darrell Ball <[email protected]> wrote: > > nat_clean has a defunct optimization for calculating a hash outside the > scope of a bucket lock which can lead to a race in referencing a freed > conntrack entry. Adjust to avoid this. Needs backporting to 2.8. > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351629.html > Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.") > Signed-off-by: Darrell Ball <[email protected]> > --- > lib/conntrack.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index be8debb..692f2b8 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -778,20 +778,22 @@ nat_clean(struct conntrack *ct, struct conn *conn, > { > ct_rwlock_wrlock(&ct->resources_lock); > nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis); > - ct_rwlock_unlock(&ct->resources_lock); > - ct_lock_unlock(&ctb->lock); > unsigned bucket_rev_conn = > hash_to_bucket(conn_key_hash(&conn->rev_key, ct->hash_basis)); > + struct conn_key rev_key = conn->rev_key; > + ct_rwlock_unlock(&ct->resources_lock); > + ct_lock_unlock(&ctb->lock); > + > ct_lock_lock(&ct->buckets[bucket_rev_conn].lock); > ct_rwlock_wrlock(&ct->resources_lock); > long long now = time_msec(); > - struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now); > + struct conn *rev_conn = conn_lookup(ct, &rev_key, now); > struct nat_conn_key_node *nat_conn_key_node = > - nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key, > + nat_conn_keys_lookup(&ct->nat_conn_keys, &rev_key, > ct->hash_basis); > > - /* In the unlikely event, rev conn was recreated, then skip > - * rev_conn cleanup. */ > + /* In the unlikely event, 'rev_conn' was recreated, then skip > + * 'rev_conn' cleanup. */ > if (rev_conn && (!nat_conn_key_node || > conn_key_cmp(&nat_conn_key_node->value, > &rev_conn->rev_key))) { > -- > 1.9.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
