Re: [PATCH] fortify: Improve buffer overflow reporting

[Kees Cook]

On March 2, 2023 3:21:11 PM PST, Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote:
>On Thu, Mar 2, 2023 at 2:58 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>>
>> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
>> index c9de1f59ee80..981e2838f99a 100644
>> --- a/include/linux/fortify-string.h
>> +++ b/include/linux/fortify-string.h
>> @@ -170,11 +170,13 @@ __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
>>  char *strcat(char * const POS p, const char *q)
>>  {
>>         size_t p_size = __member_size(p);
>> +       size_t size;
>>
>>         if (p_size == SIZE_MAX)
>>                 return __underlying_strcat(p, q);
>> -       if (strlcat(p, q, p_size) >= p_size)
>> -               fortify_panic(__func__);
>> +       size = strlcat(p, q, p_size);
>> +       if (p_size < size)
>
>What happens when they're equal? I think this patch changes
>behavior...? Intentional?
>
>Did flipping this conditional drop what should be `<=`?
>
>Was there an off by one, or is this version of this patch potentially
>introducing one? Or am I misremembering my boolean algebra?

Whoops! Thanks for catching that. I was going too fast. And I'm bothered that my regression tests missed it. :|

I will send a v2...

-Kees


-- 
Kees Cook