On Tue, May 10, 2022 at 1:40 AM Ales Musil <[email protected]> wrote:

> When the "ovn-chassis-mac-mappings" had wrong format it would
> crash the ovn-controller as it tried to deref NULL pointer.
>
> Add check if the token is a valid pointer, if not log warning
> and skip this value.
>
> Reported-at: https://bugzilla.redhat.com/2082341
> Signed-off-by: Ales Musil <[email protected]>
>

Thanks for fixing this.

---
>  controller/physical.c   |  6 ++++++
>  tests/ovn-controller.at | 28 ++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 734444320..71e22cbf0 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -426,6 +426,12 @@ populate_remote_chassis_macs(const struct
> sbrec_chassis *my_chassis,
>              char *save_ptr2 = NULL;
>              char *chassis_mac_bridge = strtok_r(token, ":", &save_ptr2);
>              char *chassis_mac_str = strtok_r(NULL, "", &save_ptr2);
> +            if(!chassis_mac_str) {
>

As per the coding guidelines,  this should be
if (!chassis_mac_str)

I think Mark/Han can fix it before applying it.

Acked-by: Numan Siddique <[email protected]>

Numan

+                VLOG_WARN("Parsing of ovn-chassis-mac-mappings failed for:
> "
> +                          "\"%s\", the correct format is
> \"br-name1:MAC1\".",
> +                          token);
> +                continue;
> +            }
>              struct remote_chassis_mac *remote_chassis_mac = NULL;
>              remote_chassis_mac = xmalloc(sizeof *remote_chassis_mac);
>              hmap_insert(&remote_chassis_macs,
> &remote_chassis_mac->hmap_node,
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 88ec2f269..dcadab1f3 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2154,3 +2154,31 @@ AT_CHECK_UNQUOTED([echo $lflow_run_1], [0],
> [$lflow_run_2
>
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
> +
> +
> +AT_SETUP([ovn-controller - check ovn-chassis-mac-mappings])
> +
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +pid=$(cat hv1/ovn-controller.pid)
> +
> +# Add chassis with some ovn-chassis-mac-mappings
> +AT_CHECK([ovn-sbctl chassis-add foo geneve 127.0.0.2])
> +AT_CHECK([ovn-sbctl set chassis foo
> other_config:ovn-chassis-mac-mappings="invalid1,invalid2,br1:00:00:00:00:00:00"])
> +AT_CHECK([ovn-nbctl --wait=hv sync])
> +
> +# Check if ovn-controller is still alive
> +AT_CHECK([ps $pid], [0], [ignore])
> +# Check if we got warnings for invalid
> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed"
> hv1/ovn-controller.log | grep -q invalid1])
> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed"
> hv1/ovn-controller.log | grep -q invalid2])
> +AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings failed"
> hv1/ovn-controller.log | grep -q br1], [1])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> --
> 2.35.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

Reply via email to