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

[Simon Glass]

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.

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