[ovs-dev] conntrack: Fix conn_type need be checked when remove rev_conn.

Message ID 1505123379-12144-1-git-send-email-wangyunjian@huawei.com
State New
Headers show
Series
  • [ovs-dev] conntrack: Fix conn_type need be checked when remove rev_conn.
Related show

Commit Message

w00273186 Sept. 11, 2017, 9:49 a.m.
From: Yunjian Wang <wangyunjian@huawei.com>

The rev_conn need will be removed, only when conn_type is CT_CONN_TYPE_UN_NAT.
This crash will be triggered when remove conn in ct-clean thread.

Reported-by: Lili Huang <huanglili.huang@huawei.com>
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 lib/conntrack.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Darrell Ball Sept. 12, 2017, 4:37 a.m. | #1
We cannot merge this patch.

Can you provide answers to the questions I asked here

https://mail.openvswitch.org/pipermail/ovs-discuss/2017-September/045308.html

Thanks Darrell


On Mon, Sep 11, 2017 at 2:49 AM, w00273186 <wangyunjian@huawei.com> wrote:

> From: Yunjian Wang <wangyunjian@huawei.com>
>
> The rev_conn need will be removed, only when conn_type is
> CT_CONN_TYPE_UN_NAT.
> This crash will be triggered when remove conn in ct-clean thread.
>
> Reported-by: Lili Huang <huanglili.huang@huawei.com>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  lib/conntrack.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 419cb1d..c1adb56 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -684,9 +684,10 @@ nat_clean(struct conntrack *ct, struct conn *conn,
>
>      /* 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))) {
> +    if (rev_conn &&
> +        (rev_conn->conn_type == CT_CONN_TYPE_UN_NAT) &&
> +        (!nat_conn_key_node || conn_key_cmp(&nat_conn_key_node->value,
> +                                            &rev_conn->rev_key))) {
>          hmap_remove(&ct->buckets[bucket_rev_conn].connections,
>                      &rev_conn->node);
>          free(rev_conn);
> --
> 1.8.3.1
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 419cb1d..c1adb56 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -684,9 +684,10 @@  nat_clean(struct conntrack *ct, struct conn *conn,
 
     /* 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))) {
+    if (rev_conn &&
+        (rev_conn->conn_type == CT_CONN_TYPE_UN_NAT) &&
+        (!nat_conn_key_node || conn_key_cmp(&nat_conn_key_node->value,
+                                            &rev_conn->rev_key))) {
         hmap_remove(&ct->buckets[bucket_rev_conn].connections,
                     &rev_conn->node);
         free(rev_conn);