nss: introduce signed integer overflow check in malloc

[Carlos O'Donell]

On 6/19/26 7:17 AM, Arjun Shankar wrote:
> Hi Marcus,
> 
> Thanks for the patch. The fix itself looks good to me.
> 
> There are some issues with the commit title and message. Here's a review:
> 
>> Subject: [PATCH] nss: introduce signed integer overflow check in malloc
> 
> The arguments are unsigned, and "in malloc" might give the impression
> that something is fixed *within* malloc. We could also use a bug
> number in the commit title. I suggest something like this instead for
> the commit title:
> 
> "nss: Use reallocarray to prevent integer overflow in getaddrinfo  (bug 33977)"
> 
> Then, for the commit body,
> 
>> This concerns https://sourceware.org/bugzilla/show_bug.cgi?id=33977
>> replacing realloc by realloc array introduces a basic overflow check.
>> (old + count) might still overflow, but since the backend is trusted, we do not consider this to be a valid case.
> 
> 1. We can drop the bug URL since we already referred to the bug number
> in the commit title,
> 2. reallocarray should be one word without a space,
> 3. Suggest "NSS backend" instead of "backend"
> 4. Break any long lines after edits, the last line of the commit
> message is a bit long
> 
> Looking forward to a v2!
> 
> Cheers
> 
>> ---
>>   nss/getaddrinfo.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/nss/getaddrinfo.c b/nss/getaddrinfo.c
>> index 4f6ac3358a..78f5bababc 100644
>> --- a/nss/getaddrinfo.c
>> +++ b/nss/getaddrinfo.c
>> @@ -234,7 +234,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
>>         array = array->next;
>>       }
>>
>> -  array = realloc (res->at, (old + count) * sizeof (*array));
>> +  array = reallocarray (res->at, (old + count), sizeof (*array));

This is exactly what I was thinking.

Adding reallocarray improves the code and makes implicit that we're allocating
an array, and guards the multiplication from overflow.

It's very unlikely that "old + count" overflow, and to protect against that we'd
want to put checks in place early up the call chain in NSS.

> 
> OK. reallocarray looks for a multiplication overflow and fails if there's one.
> 
>>
>>     if (array == NULL)
>>       return false;
>> --
>> 2.47.3
>>
> 


-- 
Cheers,
Carlos.