[ovs-dev] [PATCH v3 ovn] ovn-sbctl: Prevent core dump from ovn-sbctl lflow-list [datpath] 0xflow

[Mark Michelson]

I pushed this to master, branch-21.03 and branch-20.12.

On 4/27/21 2:46 PM, Mark Michelson wrote:
> Excellent! Looks good to me!
> 
> Acked-by: Mark Michelson <mmichels at redhat.com>
> 
> On 4/21/21 12:17 PM, Alexey Roytman wrote:
>> From: Alexey Roytman <roytman at il.ibm.com>
>>
>> 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 <roytman at il.ibm.com>
>> ---
>>   utilities/ovn-sbctl.c | 28 +++++++++++++++++-----------
>>   1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
>> index e3aa7a68e..99c112358 100644
>> --- a/utilities/ovn-sbctl.c
>> +++ b/utilities/ovn-sbctl.c
>> @@ -764,23 +764,28 @@ sbctl_lflow_cmp(const void *a_, const void *b_)
>>       return cmp ? cmp : strcmp(a->actions, b->actions);
>>   }
>> -static char *
>> +static bool
>> +is_uuid_with_prefix(const char *uuid)
>> +{
>> +     return uuid[0] == '0' && (uuid[1] == 'x' || uuid[1] == 'X');
>> +}
>> +
>> +static bool
>>   parse_partial_uuid(char *s)
>>   {
>>       /* Accept a full or partial UUID. */
>>       if (uuid_is_partial_string(s)) {
>> -        return s;
>> +        return true;
>>       }
>>       /* 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)) {
>> -        return s + 2;
>> +    if (is_uuid_with_prefix(s) && uuid_is_partial_string(s + 2)) {
>> +        return true;
>>       }
>>       /* Not a (partial) UUID. */
>> -    return NULL;
>> +    return false;
>>   }
>>   static const char *
>> @@ -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));
>>   }
>> @@ -1134,12 +1142,10 @@ cmd_lflow_list(struct ctl_context *ctx)
>>       }
>>       for (size_t i = 1; i < ctx->argc; i++) {
>> -        char *s = parse_partial_uuid(ctx->argv[i]);
>> -        if (!s) {
>> +        if (!parse_partial_uuid(ctx->argv[i])) {
>>               ctl_fatal("%s is not a UUID or the beginning of a UUID",
>>                         ctx->argv[i]);
>>           }
>> -        ctx->argv[i] = s;
>>       }
>>       struct vconn *vconn = sbctl_open_vconn(&ctx->options);
>>
>