[PATCH v1] bootm: bound-check OS index in bootm_os_get_boot_func()

[Aristo Chen]

Hi Simon,

On Sat, Jun 13, 2026 at 2:22 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Aristo,
>
> On Sun, 24 May 2026 at 17:11, Aristo Chen <aristo.chen at canonical.com> wrote:
> >
> > The boot_os[] table in bootm_os.c is a sparse array whose compile-time
> > size is set by its largest designated initializer (IH_OS_ELF), giving
> > it IH_OS_ELF + 1 entries. The accessor bootm_os_get_boot_func() returns
> > boot_os[os] without any bound check, even though the caller in
> > bootm_run_states() passes images->os.os straight through. That field is
> > populated by image_get_os() from the raw 8-bit ih_os byte of a legacy
> > uImage, and by fit_image_get_os() for a FIT, neither of which clamps
> > the value against the table size.
> >
> > An attacker-supplied image whose OS field falls outside the populated
> > range therefore drives an out-of-bounds read of boot_os[]. The caller
> > only rejects a NULL return, so a non-NULL adjacent global is accepted
> > as a valid handler and invoked through the indirect call in
> > boot_selected_os(), turning an unsigned image with a malformed header
> > into a jump through an attacker-influenced function pointer. FIT
> > signature verification covers the os property and mitigates this path
> > for signed images, but legacy bootm and unsigned FIT do not.
> >
> > Reject out-of-range indices in bootm_os_get_boot_func() so the existing
> > NULL handling in bootm_run_states() reports an unsupported OS and
> > declines to boot the image.
> >
> > Signed-off-by: Aristo Chen <aristo.chen at canonical.com>
> > ---
> >  boot/bootm_os.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> I would rather put this check earlier and only for legacy images, to
> reduce code size. For FIT I believe this cannot happen, right?

Yes, you're right, FIT can't reach this. boot_get_kernel() loads the
kernel via fit_image_load() with IH_TYPE_KERNEL, and the os_ok check
there already rejects any image whose os isn't a known type before
bootm_find_os() reads it, so images.os.os is always in range. Only the
legacy path takes the raw ih_os byte unchecked.

I'd lean toward keeping the test in bootm_os_get_boot_func() since it
sits right at the boot_os[] dereference that every caller goes
through, and the cost is just a couple of instructions in
CONFIG_CMD_BOOTM code (not SPL). But if you'd prefer it scoped to the
legacy path, I'm happy to send a follow-up moving it there.

>
> >
> > diff --git a/boot/bootm_os.c b/boot/bootm_os.c
> > index ae20b555f5c..69aa577a2fc 100644
> > --- a/boot/bootm_os.c
> > +++ b/boot/bootm_os.c
> > @@ -599,5 +599,7 @@ int boot_selected_os(int state, struct bootm_info *bmi, boot_os_fn *boot_fn)
> >
> >  boot_os_fn *bootm_os_get_boot_func(int os)
> >  {
> > +       if (os < 0 || os >= ARRAY_SIZE(boot_os))
> > +               return NULL;
> >         return boot_os[os];
> >  }
> > --
> > 2.43.0
> >
>
> Regards,
> SImon

Best regards,
Aristo