[PATCH] accel/ivpu: Reject firmware log with size smaller than header

[PATCH] accel/ivpu: Reject firmware log with size smaller than header

[Jhonraushan]

fw_log_from_bo() validates the tracing buffer header_size and that the
log fits within the BO, but never checks that log->size is at least
log->header_size. fw_log_print_buffer() then computes:

  u32 data_size = log->size - log->header_size;

which underflows to a near-U32_MAX value when firmware reports a log whose
size is smaller than its header. That huge data_size defeats the
log_start/log_end bounds clamps added by commit dd1311bcf0e6 ("accel/ivpu:
Add bounds checks for firmware log indices"), so fw_log_print_lines() reads
far past the small real data region of the BO. A size of 0 also makes
fw_log_from_bo() advance the offset by 0, causing the callers to loop
forever on the same header.

Reject logs whose size is smaller than the header (which also rejects
size == 0).

Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
Cc: stable@vger.kernel.org
Signed-off-by: Jhonraushan <raushan.jhon@gmail.com>
---
 drivers/accel/ivpu/ivpu_fw_log.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
index 275baf844b56..716467aa3156 100644
--- a/drivers/accel/ivpu/ivpu_fw_log.c
+++ b/drivers/accel/ivpu/ivpu_fw_log.c
@@ -43,6 +43,10 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
 		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log->header_size);
 		return -EINVAL;
 	}
+	if (log->size < log->header_size) {
+		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
+		return -EINVAL;
+	}
 	if ((char *)log + log->size > (char *)ivpu_bo_vaddr(bo) + ivpu_bo_size(bo)) {
 		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
 		return -EINVAL;
-- 
2.53.0

Re: [PATCH] accel/ivpu: Reject firmware log with size smaller than header

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The validation of `log->size` and `log->header_size` is vulnerable to a Time-Of-Check to Time-Of-Use (TOCTOU) race condition because the fields are read multiple times directly from firmware-shared BO memory, allowing malicious firmware to bypass bounds checks.
- [Medium] The firmware-controlled `log->name` character array is printed using an unbounded `%s` format specifier, which can lead to out-of-bounds reads if the device does not null-terminate the string.
--

--- Patch [1]: [PATCH] accel/ivpu: Reject firmware log with size smaller than header ---
commit 8efc93a32eb39ab04f9058367f2b3c6355d62590
Author: Jhonraushan <raushan.jhon@gmail.com>

accel/ivpu: Reject firmware log with size smaller than header

This commit adds a check to fw_log_from_bo() to reject firmware logs where
the log size is smaller than the header size. This is intended to prevent
an integer underflow when calculating the data size in fw_log_print_buffer(),
which could defeat bounds checks.

> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
> index 275baf844b562..716467aa31567 100644
> --- a/drivers/accel/ivpu/ivpu_fw_log.c
> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
> @@ -43,6 +43,10 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
>  		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log->header_size);
>  		return -EINVAL;
>  	}
> +	if (log->size < log->header_size) {
> +		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
> +		return -EINVAL;
> +	}

[Severity: High]
This is a pre-existing issue, but since the log pointer accesses BO memory
shared directly with the firmware, could a malicious firmware alter log->size
or log->header_size after these checks are performed?

For example, in fw_log_from_bo() the offset is later updated:

	*offset += log->size;

And in fw_log_print_buffer() data_size is calculated:

	u32 data_size = log->size - log->header_size;

If the firmware changes the values between the validation here and these
subsequent reads, could this bypass the bounds checks and lead to the very
same underflow or infinite loop this patch aims to fix? Would it be safer
to read these values once into local variables?

[Severity: Medium]
This is also a pre-existing issue, but I noticed that later in
fw_log_from_bo(), the log->name array is printed using the %s format:

	ivpu_dbg(vdev, FW_BOOT,
		 "FW log name \"%s\", write offset 0x%x size 0x%x, wrap count %d, hdr version %d size %d format %d, alignment %d",
		 log->name, log->write_index, log->size, log->wrap_count, log->header_version,
		 log->header_size, log->format, log->alignment);

Since log->name is a 16-byte character array in the shared firmware BO
memory, what happens if the firmware fills it without a null terminator?

Could the %s format specifier read past the array bounds and leak memory
or cause a page fault?

This similarly occurs in fw_log_print_buffer() when using drm_printf()
with %s on log->name.

>  	if ((char *)log + log->size > (char *)ivpu_bo_vaddr(bo) + ivpu_bo_size(bo)) {
>  		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715074206.867712-1-raushan.jhon@gmail.com?part=1

Re: [PATCH] accel/ivpu: Reject firmware log with size smaller than header

[Wachowski, Karol]

On 15-Jul-26 9:42, Jhonraushan wrote:
> fw_log_from_bo() validates the tracing buffer header_size and that the
> log fits within the BO, but never checks that log->size is at least
> log->header_size. fw_log_print_buffer() then computes:
> 
>    u32 data_size = log->size - log->header_size;
> 
> which underflows to a near-U32_MAX value when firmware reports a log whose
> size is smaller than its header. That huge data_size defeats the
> log_start/log_end bounds clamps added by commit dd1311bcf0e6 ("accel/ivpu:
> Add bounds checks for firmware log indices"), so fw_log_print_lines() reads
> far past the small real data region of the BO. A size of 0 also makes
> fw_log_from_bo() advance the offset by 0, causing the callers to loop
> forever on the same header.
> 
> Reject logs whose size is smaller than the header (which also rejects
> size == 0).
> 
> Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jhonraushan <raushan.jhon@gmail.com>
> ---
>   drivers/accel/ivpu/ivpu_fw_log.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
> index 275baf844b56..716467aa3156 100644
> --- a/drivers/accel/ivpu/ivpu_fw_log.c
> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
> @@ -43,6 +43,10 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
>   		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log->header_size);
>   		return -EINVAL;
>   	}
> +	if (log->size < log->header_size) {
> +		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
> +		return -EINVAL;
> +	}
>   	if ((char *)log + log->size > (char *)ivpu_bo_vaddr(bo) + ivpu_bo_size(bo)) {
>   		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
>   		return -EINVAL;

Thanks for submission.

Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>

Re: [PATCH] accel/ivpu: Reject firmware log with size smaller than header

[Wachowski, Karol]

On 15-Jul-26 10:21, Wachowski, Karol wrote:
> On 15-Jul-26 9:42, Jhonraushan wrote:
>> fw_log_from_bo() validates the tracing buffer header_size and that the
>> log fits within the BO, but never checks that log->size is at least
>> log->header_size. fw_log_print_buffer() then computes:
>>
>>    u32 data_size = log->size - log->header_size;
>>
>> which underflows to a near-U32_MAX value when firmware reports a log 
>> whose
>> size is smaller than its header. That huge data_size defeats the
>> log_start/log_end bounds clamps added by commit dd1311bcf0e6 ("accel/ 
>> ivpu:
>> Add bounds checks for firmware log indices"), so fw_log_print_lines() 
>> reads
>> far past the small real data region of the BO. A size of 0 also makes
>> fw_log_from_bo() advance the offset by 0, causing the callers to loop
>> forever on the same header.
>>
>> Reject logs whose size is smaller than the header (which also rejects
>> size == 0).
>>
>> Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jhonraushan <raushan.jhon@gmail.com>
>> ---
>>   drivers/accel/ivpu/ivpu_fw_log.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ 
>> ivpu_fw_log.c
>> index 275baf844b56..716467aa3156 100644
>> --- a/drivers/accel/ivpu/ivpu_fw_log.c
>> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
>> @@ -43,6 +43,10 @@ static int fw_log_from_bo(struct ivpu_device *vdev, 
>> struct ivpu_bo *bo, u32 *off
>>           ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log- 
>> >header_size);
>>           return -EINVAL;
>>       }
>> +    if (log->size < log->header_size) {
>> +        ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
>> +        return -EINVAL;
>> +    }
>>       if ((char *)log + log->size > (char *)ivpu_bo_vaddr(bo) + 
>> ivpu_bo_size(bo)) {
>>           ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
>>           return -EINVAL;
> 
> Thanks for submission.
> 
> Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>

Applied to drm-misc-fixes.