Hi Alexey, nice find! I have a comment down below:
On 4/20/21 6:22 PM, Alexey Roytman wrote:
From: Alexey Roytman <[email protected]>
When ovn-sbctl lflow-list gets lflow argument with 0x prefix, e.g. 0x8131c8a8,
it prints correct output, but fails with coredump.
For example:
ovn-sbctl --uuid lflow-list sw1 0x8131c8a8
Datapath: "sw1" (4b1e53d8-9f0f-4768-b4a6-6cbc58a4bfda) Pipeline: egress
uuid=0x8131c8a8, table=10(ls_out_port_sec_l2 ), priority=100 ,
match=(eth.mcast), action=(output;)
free(): invalid pointer
[2] 616553 abort (core dumped) ovn-sbctl --uuid dump-flows sw1 0x8131c8a8
This patch fixes it.
Signed-off-by: Alexey Roytman <[email protected]>
---
utilities/ovn-sbctl.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index e3aa7a68e..099d31f08 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -764,6 +764,12 @@ sbctl_lflow_cmp(const void *a_, const void *b_)
return cmp ? cmp : strcmp(a->actions, b->actions);
}
+static bool
+is_uuid_with_prefix(const char *uuid)
+{
+ return uuid[0] == '0' && (uuid[1] == 'x' || uuid[1] == 'X');
+}
+
static char *
parse_partial_uuid(char *s)
{
@@ -774,8 +780,7 @@ parse_partial_uuid(char *s)
/* Accept a full or partial UUID prefixed by 0x, since "ovs-ofctl
* dump-flows" prints cookies prefixed by 0x. */
- if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X')
- && uuid_is_partial_string(s + 2)) {
+ if (is_uuid_with_prefix(s) && uuid_is_partial_string(s + 2)) {
return s + 2;
}
@@ -799,8 +804,11 @@ is_partial_uuid_match(const struct uuid *uuid, const char *match)
* from UUIDs, and cookie values are printed without leading zeros because
* they're just numbers. */
const char *s1 = strip_leading_zero(uuid_s);
- const char *s2 = strip_leading_zero(match);
-
+ const char *s2 = match;
+ if (is_uuid_with_prefix(s2)) {
+ s2 = s2 + 2;
+ }
+ s2 = strip_leading_zero(s2);
return !strncmp(s1, s2, strlen(s2));
}
@@ -1139,7 +1147,6 @@ cmd_lflow_list(struct ctl_context *ctx)
ctl_fatal("%s is not a UUID or the beginning of a UUID",
ctx->argv[i]);
}
- ctx->argv[i] = s;
Since you no longer assign s, this section could be re-written to get
rid of s:
if (!parse_partial_uuid(ctx->argv[i])) {
ctl_fatal("%s is not a UUID or the beginning of a UUID",
ctx->argv[i];
}
This then snowballs to the parse_partial_uuid() function. The reason the
crash was happening was because parse_partial_uuid() was returning a
pointer to the middle of an allocated string. We could increase the
safety of parse_partial_uuid() by having it return a boolean instead of
a dangerous pointer. This way, there is no chance of misusing the string
it returns since it no longer will return a string.
}
struct vconn *vconn = sbctl_open_vconn(&ctx->options);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
