* Re: [PATCH 1/1] libavb: fix avb_replace() OOM handling [not found] ` <20260521163248.15866-2-josh2@disroot.org> @ 2026-05-22 18:35 ` Tom Rini 2026-05-26 13:17 ` Mattijs Korpershoek 0 siblings, 1 reply; 5+ messages in thread From: Tom Rini @ 2026-05-22 18:35 UTC (permalink / raw) To: Josh Law; +Cc: u-boot, mkorpershoek, igor.opaniuk [-- Attachment #1: Type: text/plain, Size: 2280 bytes --] 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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] libavb: fix avb_replace() OOM handling 2026-05-22 18:35 ` [PATCH 1/1] libavb: fix avb_replace() OOM handling Tom Rini @ 2026-05-26 13:17 ` Mattijs Korpershoek 2026-05-26 13:18 ` Josh Law 0 siblings, 1 reply; 5+ messages in thread From: Mattijs Korpershoek @ 2026-05-26 13:17 UTC (permalink / raw) To: Tom Rini, Josh Law; +Cc: u-boot, mkorpershoek, igor.opaniuk 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] libavb: fix avb_replace() OOM handling 2026-05-26 13:17 ` Mattijs Korpershoek @ 2026-05-26 13:18 ` Josh Law 0 siblings, 0 replies; 5+ messages in thread From: Josh Law @ 2026-05-26 13:18 UTC (permalink / raw) To: Mattijs Korpershoek, Tom Rini; +Cc: u-boot, mkorpershoek, igor.opaniuk On May 26, 2026 2:17:08 PM GMT+01:00, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote: >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? Heh. The like guardian of the "first post" lists, took a while, so I subscribed to the list, thought that would get it approved on-list It happens. Honest mistake :) >> >> -- >> Tom > Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 0/1] libavb: fix avb_replace() OOM handling @ 2026-05-21 16:51 Josh Law 2026-05-21 16:51 ` [PATCH 1/1] " Josh Law 0 siblings, 1 reply; 5+ messages in thread From: Josh Law @ 2026-05-21 16:51 UTC (permalink / raw) To: u-boot; +Cc: mkorpershoek, igor.opaniuk, trini, Josh Law Hi folks, Since I am a new contributor, I would like to introduce myself first. I am 15, I am from the UK, and I write C. I am interested in lib/, ARM devices, and Android. Enough of the introduction. Let's talk about the patch. avb_replace() is documented to return NULL on OOM. It already did that before any output was built, but if a later allocation failed after the first replacement, it returned the partial buffer instead. The callers only treat NULL as failure, so they could keep booting with truncated AVB bootargs. This patch frees the partial buffer and returns NULL. The existing callers can then report OOM instead of accepting a shortened result. Josh Law (1): libavb: fix avb_replace() OOM handling lib/libavb/avb_util.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- 2.47.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] libavb: fix avb_replace() OOM handling 2026-05-21 16:51 [PATCH 0/1] " Josh Law @ 2026-05-21 16:51 ` Josh Law 2026-05-25 15:15 ` Simon Glass 0 siblings, 1 reply; 5+ messages in thread From: Josh Law @ 2026-05-21 16:51 UTC (permalink / raw) To: u-boot; +Cc: mkorpershoek, igor.opaniuk, trini, Josh Law 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(). */ -- 2.47.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] libavb: fix avb_replace() OOM handling 2026-05-21 16:51 ` [PATCH 1/1] " Josh Law @ 2026-05-25 15:15 ` Simon Glass 0 siblings, 0 replies; 5+ messages in thread From: Simon Glass @ 2026-05-25 15:15 UTC (permalink / raw) To: josh2; +Cc: u-boot, mkorpershoek, igor.opaniuk, trini On 2026-05-21T16:51:21, Josh Law <josh2@disroot.org> wrote: > libavb: fix avb_replace() OOM handling > > 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(-) Reviewed-by: Simon Glass <sjg@chromium.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-26 13:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260521163248.15866-1-josh2@disroot.org>
[not found] ` <20260521163248.15866-2-josh2@disroot.org>
2026-05-22 18:35 ` [PATCH 1/1] libavb: fix avb_replace() OOM handling Tom Rini
2026-05-26 13:17 ` Mattijs Korpershoek
2026-05-26 13:18 ` Josh Law
2026-05-21 16:51 [PATCH 0/1] " Josh Law
2026-05-21 16:51 ` [PATCH 1/1] " Josh Law
2026-05-25 15:15 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.