Re: [PATCH v2 3/3] platform/x86: panasonic-laptop: Fix sentinel write past pcc->sinf[]
From: Ilpo Järvinen
Date: Tue Aug 18 2026 - 07:15:59 EST
On Thu, 13 Aug 2026, Hilgad Montelo wrote:
> acpi_pcc_retrieve_biosdata() rejects SINF packages only when
> pcc->num_sifr is strictly less than hkey->package.count, then
> unconditionally writes a trailing sentinel at
> pcc->sinf[hkey->package.count]. But pcc->sinf[] is allocated with
> exactly pcc->num_sifr elements (valid indices 0..num_sifr-1), so that
> write needs num_sifr strictly greater than package.count to stay in
> bounds -- num_sifr == package.count passes the existing check but
> still overflows by one element.
>
> This is exactly the case probe()'s existing num_sifr++ workaround
> ("Some DSDT-s have an off-by-one bug where the SINF package count is
> one higher than the SQTY reported value") is written to accommodate:
> when a DSDT's SINF package count equals SQTY+1, the workaround makes
> num_sifr equal to package.count, which is precisely the boundary that
> overflows here. Found via UBSan (array-index-out-of-bounds) on
> hardware where HKEY.SQTY returns 37 and HKEY.SINF()'s package has 38
> elements: num_sifr becomes 38 after the += 1 workaround, the loop
> correctly fills indices 0..37, and the sentinel write then targets
> index 38, one past the end -- a silent 4-byte heap overflow on kernels
> without CONFIG_UBSAN.
>
> Tightening the rejection check to num_sifr <= package.count would
> avoid the overflow but breaks probe() entirely on exactly this
> hardware, since num_sifr == package.count is the case the off-by-one
> workaround exists to support. Nothing else in the driver reads this
> sentinel value back, so simply skip the write when there is no room
> for it instead.
>
> Signed-off-by: Hilgad Montelo <hilgad.montelo@xxxxxxxxx>
> ---
> drivers/platform/x86/panasonic-laptop.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 93e6511..9511440 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -476,7 +476,16 @@ static int acpi_pcc_retrieve_biosdata(struct pcc_acpi *pcc)
> } else
> pr_err("Invalid HKEY.SINF data\n");
> }
> - pcc->sinf[hkey->package.count] = -1;
> + /*
> + * pcc->sinf[] has pcc->num_sifr elements (valid indices
> + * 0..num_sifr-1). On DSDTs where SINF's package count equals
> + * num_sifr exactly -- the off-by-one case probe()'s num_sifr++
> + * already allocates a spare element for -- there is no room left
> + * for this trailing sentinel; nothing reads it back, so just skip
> + * the write rather than running one element past the flex array.
> + */
> + if (hkey->package.count < pcc->num_sifr)
> + pcc->sinf[hkey->package.count] = -1;
Hi,
Thanks for the patch. I've applied this fix patch 3 (only) into the
review-ilpo-next to get it in within this cycle.
I did add:
Fixes: a3d0dbd18ce9 ("platform/x86: panasonic-laptop: simplify allocation of sinf")
...because I think it removed one extra entry (there were initially 2
extra entries and we didn't realize the second one was probably for this
particular assignment) allowing write past the array.
I'll consider the other two patches of this series later.
--
i.