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

[Aristo Chen]

Hi Simon,

On Thu, Jun 18, 2026 at 9:25 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Aristo,
>
> On Tue, 16 Jun 2026 at 11:02, Aristo Chen <aristo.chen at canonical.com> wrote:
> >
> > 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.
>
> Actually I do think putting it there is better, since adding a check
> for the FIT case is actually confusing - the check can never fail! So
> let's deal with this problem in the legacy code, so as to avoid it
> bleeding into the FIT code.

Sure, I will send a follow up patch for this.

>
> >
> > >
> > > >
> > > > 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