Re: [PATCH v2 5/9] platform/x86: hp-bioscfg: fix off-by-one heap OOB write in audit_log_entries_show

On Wed, 12 Aug 2026, Muhammad Bilal wrote:

> The per-iteration guard in audit_log_entries_show() is:
> 
> 	if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE)
> 		break;
> 	...
> 	memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
> 	buf += LOG_ENTRY_SIZE;

Hi,

What about the preceeding check:

	if (count * LOG_ENTRY_SIZE > PAGE_SIZE)
                return -EIO;

Is the second check dead code or how can you get i large enough for it 
to ever be useful?

> At i == 256 (PAGE_SIZE / LOG_ENTRY_SIZE), LOG_ENTRY_SIZE * i equals
> PAGE_SIZE exactly, which is not ">" PAGE_SIZE, so the loop does not
> break and instead writes another LOG_ENTRY_SIZE (16) bytes starting at
> offset 4096 of the page-sized sysfs output buffer, one entry past its
> end.
> 
> This needs the BIOS to report more than 256 audit log entries, which
> already exceeds this driver's own documented LOG_MAX_ENTRIES of 254,
> so it requires a non-compliant or corrupted firmware value rather than
> the roughly 85 million entries an unrelated integer-overflow read of
> this code might suggest.
> 
> Fix by checking the bound against the offset the write is about to
> reach, (i + 1), instead of the offset already written.
> 
> Fixes: 63e8f906e94e ("platform/x86: hp-bioscfg: surestart-attributes")
> Cc: [email protected]
> Signed-off-by: Muhammad Bilal <[email protected]>
> ---
>  drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> index b57e42f29282..6b63fdb84606 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> @@ -90,7 +90,7 @@ static ssize_t audit_log_entries_show(struct kobject *kobj,
>  					   HPWMI_SURESTART,
>  					   audit_log_buffer, 1, 128);
>  
> -		if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE) {
> +		if (ret < 0 || (LOG_ENTRY_SIZE * (i + 1)) > PAGE_SIZE) {
>  			/*
>  			 * Encountered a failure while reading
>  			 * individual logs. Only a partial list of

Since this block uses break, the following else is unnecessary and should 
be removed in a separate patch.

-- 
 i.