[PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[Naveen Kumar Chaudhary]

bootstage_mark_code() allocated the label buffer without checking the
result and then dereferenced it, risking a NULL pointer crash on
allocation failure. The length calculation also failed to account for
the "," and ": " separators emitted by the snprintf() calls, so the
assembled string could be silently truncated. Additionally, when file
and func are NULL and linenum is -1, the buffer was passed on
uninitialized.

Account for the separator bytes, bail out on allocation failure, and
ensure the buffer is always NUL-terminated.

Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
---
 common/bootstage.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/common/bootstage.c b/common/bootstage.c
index 4532100acea..9e1a8609148 100644
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -175,12 +175,15 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum)
 	if (linenum != -1)
 		len = 11;
 	if (func)
-		len += strlen(func);
+		len += strlen(func) + 2;	/* ": " separator */
 	if (file)
-		len += strlen(file);
+		len += strlen(file) + 1;	/* "," separator */
 
 	str = malloc(len + 1);
+	if (!str)
+		return timer_get_boot_us();
 	p = str;
+	*p = '\0';
 	end = p + len;
 	if (file)
 		p += snprintf(p, end - p, "%s,", file);
-- 
2.43.0

Re: [PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[Simon Glass]

Hi Naveen,

On 2026-09-01T10:23:25, Naveen Kumar Chaudhary <naveen.osdev@gmail.com> wrote:
> bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()
>
> bootstage_mark_code() allocated the label buffer without checking the
> result and then dereferenced it, risking a NULL pointer crash on
> allocation failure. The length calculation also failed to account for
> the "," and ": " separators emitted by the snprintf() calls, so the
> assembled string could be silently truncated. Additionally, when file
> and func are NULL and linenum is -1, the buffer was passed on
> uninitialized.

Please rewrite in present tense per U-Boot / Linux convention, e.g.
'allocates the label buffer without checking the result', 'fails to
account for', 'is passed on uninitialised'. This patch aims to change
the current code.

Also worth noting that the only in-tree caller is the BOOTSTAGE_MARKER
macro, which always passes __FILE__, __func__ and __LINE__, so the
NULL/-1 case is theoretical hardening rather than an observed crash -
the truncation and unchecked malloc() are the real fixes.

>
> Account for the separator bytes, bail out on allocation failure, and
> ensure the buffer is always NUL-terminated.
>
> Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
>
> common/bootstage.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

> diff --git a/common/bootstage.c b/common/bootstage.c
> @@ -175,12 +175,15 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum)
>       if (linenum != -1)
>               len = 11;
>       if (func)
> -             len += strlen(func);
> +             len += strlen(func) + 2;        /* ": " separator */
>       if (file)
> -             len += strlen(file);
> +             len += strlen(file) + 1;        /* "," separator */

BTW each snprintf() reserves a byte for its own terminating NUL within
end - p, so the effective content budget is len - 1, not len. This
only matters if %d expands to the full 11 chars (a negative int),
which cannot happen for __LINE__, so it is not a real bug, just
thought I'd mention it.

> diff --git a/common/bootstage.c b/common/bootstage.c
> @@ -175,12 +175,15 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum)
>       str = malloc(len + 1);
> +     if (!str)
> +             return timer_get_boot_us();

Returning a valid-looking timestamp on allocation failure silently
drops the record with no indication to the caller. Returning 0 (or at
least a log_debug()) would be clearer - what do you think?

Regards,
Simon

Re: [PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

On Tue, Sep 01, 2026 at 07:47:36AM -0600, Simon Glass wrote:
> Hi Naveen,
> 
> On 2026-09-01T10:23:25, Naveen Kumar Chaudhary <naveen.osdev@gmail.com> wrote:
> > bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()
> >
> > bootstage_mark_code() allocated the label buffer without checking the
> > result and then dereferenced it, risking a NULL pointer crash on
> > allocation failure. The length calculation also failed to account for
> > the "," and ": " separators emitted by the snprintf() calls, so the
> > assembled string could be silently truncated. Additionally, when file
> > and func are NULL and linenum is -1, the buffer was passed on
> > uninitialized.
> 
> Please rewrite in present tense per U-Boot / Linux convention, e.g.
> 'allocates the label buffer without checking the result', 'fails to
> account for', 'is passed on uninitialised'. This patch aims to change
> the current code.

Hi Simon,

As I said the other day, please stop telling people to rewrite their
commit messages when it's already clear and understandable. This simply
leads to confusion and frustration among our contributors.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[Simon Glass]

Hi Tom,

On Tue, 1 Sept 2026 at 08:03, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Sep 01, 2026 at 07:47:36AM -0600, Simon Glass wrote:
> > Hi Naveen,
> >
> > On 2026-09-01T10:23:25, Naveen Kumar Chaudhary <naveen.osdev@gmail.com> wrote:
> > > bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()
> > >
> > > bootstage_mark_code() allocated the label buffer without checking the
> > > result and then dereferenced it, risking a NULL pointer crash on
> > > allocation failure. The length calculation also failed to account for
> > > the "," and ": " separators emitted by the snprintf() calls, so the
> > > assembled string could be silently truncated. Additionally, when file
> > > and func are NULL and linenum is -1, the buffer was passed on
> > > uninitialized.
> >
> > Please rewrite in present tense per U-Boot / Linux convention, e.g.
> > 'allocates the label buffer without checking the result', 'fails to
> > account for', 'is passed on uninitialised'. This patch aims to change
> > the current code.
>
> Hi Simon,
>
> As I said the other day, please stop telling people to rewrite their
> commit messages when it's already clear and understandable. This simply
> leads to confusion and frustration among our contributors.

Then do we need to change this?

https://docs.u-boot-project.org/en/latest/develop/sending_patches.html#commit-message-conventions

Also, we could perhaps introduce an AGENTS.md file, so at least the AI
assistants follow the guidelines?

Regards,
Simon

Re: [PATCH] bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]

On Wed, Sep 02, 2026 at 06:29:48AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 1 Sept 2026 at 08:03, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Sep 01, 2026 at 07:47:36AM -0600, Simon Glass wrote:
> > > Hi Naveen,
> > >
> > > On 2026-09-01T10:23:25, Naveen Kumar Chaudhary <naveen.osdev@gmail.com> wrote:
> > > > bootstage: fix unchecked malloc and undersized buffer in bootstage_mark_code()
> > > >
> > > > bootstage_mark_code() allocated the label buffer without checking the
> > > > result and then dereferenced it, risking a NULL pointer crash on
> > > > allocation failure. The length calculation also failed to account for
> > > > the "," and ": " separators emitted by the snprintf() calls, so the
> > > > assembled string could be silently truncated. Additionally, when file
> > > > and func are NULL and linenum is -1, the buffer was passed on
> > > > uninitialized.
> > >
> > > Please rewrite in present tense per U-Boot / Linux convention, e.g.
> > > 'allocates the label buffer without checking the result', 'fails to
> > > account for', 'is passed on uninitialised'. This patch aims to change
> > > the current code.
> >
> > Hi Simon,
> >
> > As I said the other day, please stop telling people to rewrite their
> > commit messages when it's already clear and understandable. This simply
> > leads to confusion and frustration among our contributors.
> 
> Then do we need to change this?
> 
> https://docs.u-boot-project.org/en/latest/develop/sending_patches.html#commit-message-conventions

No, it's conventions and guidelines. One should do that. And if there's
no commit message, or there's barely anything in a commit message,
that's useful. But if someone wrote something, and what they wrote
matches what they did, that's what's important.

> Also, we could perhaps introduce an AGENTS.md file, so at least the AI
> assistants follow the guidelines?

AI assistants are a bad at writing commit messages to start with.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]