Re: [PATCH v2 3/3] platform/x86: panasonic-laptop: Fix sentinel write past pcc->sinf[]

Ilpo Järvinen <[email protected]>
Newsgroups org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.