Re: [PATCH 1/1] libavb: fix avb_replace() OOM handling

[Mattijs Korpershoek <mkorpershoek@kernel.org>]

Hi Josh,

On Fri, May 22, 2026 at 12:35, Tom Rini <trini@konsulko.com> wrote:

> On Thu, May 21, 2026 at 04:32:48PM +0000, Josh Law wrote:
>> avb_replace() promises NULL on OOM. Once it had built the first
>> replacement, a later allocation failure returned that partial buffer.
>> Callers treat any result as success, so AVB could keep booting with
>> truncated bootargs.
>> 
>> Free the partial result and return NULL. The existing callers can then
>> take their OOM path.
>> 
>> Signed-off-by: Josh Law <josh2@disroot.org>
>> ---
>>  lib/libavb/avb_util.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lib/libavb/avb_util.c b/lib/libavb/avb_util.c
>> index 8719ede15a7..9e2e6ea3495 100644
>> --- a/lib/libavb/avb_util.c
>> +++ b/lib/libavb/avb_util.c
>> @@ -272,7 +272,7 @@ char* avb_replace(const char* str, const char* search, const char* replace) {
>>        num_new = num_before + replace_len + 1;
>>        ret = avb_malloc(num_new);
>>        if (ret == NULL) {
>> -        goto out;
>> +        goto fail;
>>        }
>>        avb_memcpy(ret, str, num_before);
>>        avb_memcpy(ret + num_before, replace, replace_len);
>> @@ -283,7 +283,7 @@ char* avb_replace(const char* str, const char* search, const char* replace) {
>>        num_new = ret_len + num_before + replace_len + 1;
>>        new_str = avb_malloc(num_new);
>>        if (new_str == NULL) {
>> -        goto out;
>> +        goto fail;
>>        }
>>        avb_memcpy(new_str, ret, ret_len);
>>        avb_memcpy(new_str + ret_len, str, num_before);
>> @@ -308,7 +308,7 @@ char* avb_replace(const char* str, const char* search, const char* replace) {
>>      size_t num_new = ret_len + num_remaining + 1;
>>      char* new_str = avb_malloc(num_new);
>>      if (new_str == NULL) {
>> -      goto out;
>> +      goto fail;
>>      }
>>      avb_memcpy(new_str, ret, ret_len);
>>      avb_memcpy(new_str + ret_len, str_after_last_replace, num_remaining);
>> @@ -320,6 +320,10 @@ char* avb_replace(const char* str, const char* search, const char* replace) {
>>  
>>  out:
>>    return ret;
>> +
>> +fail:
>> +  avb_free(ret);
>> +  return NULL;
>>  }
>>  
>>  /* We only support a limited amount of strings in avb_strdupv(). */
>
> Thanks for the explanation and patch. This seems fine but I'll defer to
> Mattijs as it's his area.

This patch seems to posted a second time here:
https://lore.kernel.org/all/20260521165122.17475-1-josh2@disroot.org/

Can you explain why it has been send twice, please?

>
> -- 
> Tom