Re: acpi(4): Fix out of range access in System States

Mark Kettenis <[email protected]> Fri, 31 Jul 2026 19:22:20 +0200
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
> Date: Fri, 31 Jul 2026 17:36:53 +0200
> From: Jan Klemkow <[email protected]>
> 
> ping?

ok kettenis@

> On Wed, Jul 08, 2026 at 11:43:07PM +0200, Jan Klemkow wrote:
> > When booting a snapshot kernel on the "cloud hypervisor" [1] based on
> > Linux/KVM, it runs straight into a protection fault in acpi(4):
> > 
> > OpenBSD 7.9-current (GENERIC.MP) #24: Wed Jul  8 04:05:05 MDT 2026
> >     [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > real mem = 4262391808 (4064MB)
> > avail mem = 4111220736 (3920MB)
> > random: good seed from bootblocks
> > mpath0 at root
> > scsibus0 at mpath0: 256 targets
> > mainbus0 at root
> > bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xbf13e000 (3 entries)
> > bios0: vendor cloud-hypervisor version "0"
> > efi0 at bios0: UEFI 2.7
> > efi0: EDK II rev 0x10000
> > acpi0 at bios0: ACPI 6.3
> > acpi0: sleep stateskernel: protection fault trap, code=0
> > Stopped at      aml_val2int+0x25:       movl    0(%rdi),%eax
> > ddb{0}>
> > 
> > This is caused by blindly reading the "System State Package" without
> > checking the length of the package in before.  In the ACPI spec it looks like
> > there have to be 4 values.  But, the second one has the comment:
> > 
> > 	On HW-reduced platforms, this value is ignored.
> > 
> > The Linux kernel also does a length check before touching the package elements.
> > So, I would just add the following length here check, too.
> > 
> > ok?
> > 
> > [1]: https://www.cloudhypervisor.org/
> > [2]: https://uefi.org/specs/ACPI/6.5/07_Power_and_Performance_Mgmt.html#sx-system-states
> > 
> > Index: dev/acpi/acpi.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> > diff -u -p -r1.457 acpi.c
> > --- dev/acpi/acpi.c	11 Mar 2026 16:18:42 -0000	1.457
> > +++ dev/acpi/acpi.c	8 Jul 2026 21:07:03 -0000
> > @@ -2569,7 +2569,7 @@ acpi_init_states(struct acpi_softc *sc)
> >  		sc->sc_sleeptype[i].slp_typb = -1;
> >  		if (aml_evalname(sc, sc->sc_root, name, 0, NULL, &res) != 0)
> >  			continue;
> > -		if (res.type != AML_OBJTYPE_PACKAGE) {
> > +		if (res.type != AML_OBJTYPE_PACKAGE || res.length < 2) {
> >  			aml_freevalue(&res);
> >  			continue;
> >  		}
> 
>