[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Help-smalltalk] [PATCH] gst-tool: Fix ASAN issue on comparing optio
From: |
Paolo Bonzini |
Subject: |
Re: [Help-smalltalk] [PATCH] gst-tool: Fix ASAN issue on comparing options |
Date: |
Fri, 24 Nov 2017 11:20:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 24/11/2017 10:30, Holger Freyther wrote:
>
>> On 12. Feb 2017, at 13:16, Holger Freyther <address@hidden> wrote:
>>
>
> Dear Paolo,
>
>
>>> Wouldn't the '\0' mismatch first?
>>
>> "Both strings are assumed to be n bytes long". I guess an optimized memcmp
>> will fetch 32/64 bytes at a time (manual loop unrolling?).
>
>
> I noticed I didn't push this change yet. I think in the case ASAN
> reported, name and all_opts->name are of different size and 1-3 bytes
> after the \0 will be fetched? If I see this correctly right now we want
> to check if name is a prefix of all_opts->name? For this to be true
> all_opts->name must be at least as long as the prefix (name)? Am I wrong?
Lee is right, see also
https://trust-in-soft.com/memcmp-requires-pointers-to-fully-valid-buffers/
However, I am not sure strlen is needed. The code is:
if (!p)
len = strlen (name);
else
len = p++ - name;
for (all_opts = long_opts; all_opts; all_opts = all_opts->next)
- if (!memcmp (name, all_opts->name, len))
+ if (strlen(all_opts->name) >= len && !strncmp (name, all_opts->name, len))
so len <= strlen(name): there is no NULL byte in the first LEN
bytes of NAME. For the first LEN bytes to be equal in the two
arguments to strncmp, there must be no NULL byte in all_opts->name,
and thus strlen(all_opts->name) >= len too.
Thanks,
Paolo