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
