diff mbox series

[ovs-dev] bridge: Fix null dereference on ct_timeout_policy record

Message ID 1593195666-28111-1-git-send-email-yihung.wei@gmail.com
State Accepted
Commit 98670b77ffe8b36c9d5089022ee36680aeecc542
Headers show
Series [ovs-dev] bridge: Fix null dereference on ct_timeout_policy record | expand

Commit Message

Yi-Hung Wei June 26, 2020, 6:21 p.m. UTC 
Accoridng to vswitch.ovsschema, each CT_Zone record may have
zero or one associcated CT_Timeout_policy.  Thus, this patch
checks if ovsrec_ct_timeout_policy exist before accesses the
record.

VMWare-BZ: 2585825
Fixes: 45339539f69d ("ovs-vsctl: Add conntrack zone commands.")
Fixes: 993cae678bca ("ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables")
Reported-by: Yang Song <yangsong@vmware.com>
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
Travis CI: https://travis-ci.org/github/YiHungWei/ovs/builds/699829754

Please backport this fix to branch-2.13.
---
 tests/ovs-vsctl.at    |  8 ++++++++
 utilities/ovs-vsctl.c | 10 +++++++---
 vswitchd/bridge.c     |  6 ++++--
 3 files changed, 19 insertions(+), 5 deletions(-)

Comments

William Tu June 27, 2020, 11:41 p.m. UTC | #1 
On Fri, Jun 26, 2020 at 11:21:06AM -0700, Yi-Hung Wei wrote:
> Accoridng to vswitch.ovsschema, each CT_Zone record may have
> zero or one associcated CT_Timeout_policy.  Thus, this patch
> checks if ovsrec_ct_timeout_policy exist before accesses the
> record.
> 
> VMWare-BZ: 2585825
> Fixes: 45339539f69d ("ovs-vsctl: Add conntrack zone commands.")
> Fixes: 993cae678bca ("ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables")
> Reported-by: Yang Song <yangsong@vmware.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> ---
> Travis CI: https://travis-ci.org/github/YiHungWei/ovs/builds/699829754
> 
> Please backport this fix to branch-2.13.

Thanks, applied to master and 2.13.
William
diff mbox series

Patch

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 55c7a6e179cd..c8babe36120a 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -966,6 +966,14 @@  AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
 ])
 
+AT_CHECK(
+  [RUN_OVS_VSCTL_TOGETHER([--id=@n create CT_Zone external_ids:"test"="123"],
+                          [--id=@m create Datapath datapath_version=0 ct_zones:"10"=@n],
+                          [set Open_vSwitch . datapaths:"netdev"=@m])],
+  [0], [stdout])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:10, Timeout Policies: system default
+])
+
 AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], [0], [stdout])
 AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
 ])
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index bd3972636e66..37cc72d401d3 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1344,9 +1344,13 @@  cmd_list_zone_tp(struct ctl_context *ctx)
 
         struct ovsrec_ct_timeout_policy *tp = zone->timeout_policy;
 
-        for (int j = 0; j < tp->n_timeouts; j++) {
-            ds_put_format(&ctx->output, "%s=%"PRIu64" ",
-                          tp->key_timeouts[j], tp->value_timeouts[j]);
+        if (tp) {
+            for (int j = 0; j < tp->n_timeouts; j++) {
+                ds_put_format(&ctx->output, "%s=%"PRIu64" ",
+                        tp->key_timeouts[j], tp->value_timeouts[j]);
+            }
+        } else {
+            ds_put_cstr(&ctx->output, "system default");
         }
         ds_chomp(&ctx->output, ' ');
         ds_put_char(&ctx->output, '\n');
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f312efd8e128..0bb4fa6524e9 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -635,8 +635,10 @@  static void
 get_timeout_policy_from_ovsrec(struct simap *tp,
                                const struct ovsrec_ct_timeout_policy *tp_cfg)
 {
-    for (size_t i = 0; i < tp_cfg->n_timeouts; i++) {
-        simap_put(tp, tp_cfg->key_timeouts[i], tp_cfg->value_timeouts[i]);
+    if (tp_cfg) {
+        for (size_t i = 0; i < tp_cfg->n_timeouts; i++) {
+            simap_put(tp, tp_cfg->key_timeouts[i], tp_cfg->value_timeouts[i]);
+        }
     }
 }