[PATCH v2 0/3] accel/ivpu: Harden parsing of firmware-shared buffers

[PATCH v2 0/3] accel/ivpu: Harden parsing of firmware-shared buffers

[Dawid Osuchowski]

The IPC and firmware tracing buffers live in memory shared with the NPU
firmware, so every field the host reads from them is untrusted input.
This series fixes places where the host trusted such input and could
end up dereferencing addresses outside the underlying BO.

All patches are tagged for stable.

Changes since v1:
 - Added a new patch ("accel/ivpu: Limit firmware log name prints to
   field size") after Sashiko pointed out that log->name was still being
   printed with an unbounded "%s" conversion. The field is fixed-size
   and expected to be NUL-terminated, but a firmware bug could drop the
   terminator; harden the host against that case.
 - No changes to the other two patches.

Dawid Osuchowski (1):
  accel/ivpu: Limit firmware log name prints to field size

Magdalena Schulfer (2):
  accel/ivpu: Validate full buffer range in ivpu_to_cpu_addr
  accel/ivpu: Validate firmware log buffer metadata

 drivers/accel/ivpu/ivpu_fw_log.c | 87 +++++++++++++++++++-------------
 drivers/accel/ivpu/ivpu_gem.h    | 14 +++--
 drivers/accel/ivpu/ivpu_ipc.c    |  7 +--
 3 files changed, 67 insertions(+), 41 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] accel/ivpu: Validate full buffer range in ivpu_to_cpu_addr

[Dawid Osuchowski]

From: Magdalena Schulfer <magdalena.schulfer@intel.com>

Add a size parameter to ivpu_to_cpu_addr() and validate that the
whole [vpu_addr, vpu_addr + size) range stays within the BO.

Cc: stable@vger.kernel.org
Fixes: 647371a6609d ("accel/ivpu: Add GEM buffer object management")
Signed-off-by: Magdalena Schulfer <magdalena.schulfer@intel.com>
Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_gem.h | 14 +++++++++++---
 drivers/accel/ivpu/ivpu_ipc.c |  7 ++++---
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_gem.h b/drivers/accel/ivpu/ivpu_gem.h
index 0c3350f22b55..b1ae020a4fc2 100644
--- a/drivers/accel/ivpu/ivpu_gem.h
+++ b/drivers/accel/ivpu/ivpu_gem.h
@@ -87,15 +87,23 @@ static inline bool ivpu_bo_is_resident(struct ivpu_bo *bo)
 	return !!bo->base.pages;
 }
 
-static inline void *ivpu_to_cpu_addr(struct ivpu_bo *bo, u32 vpu_addr)
+static inline void *ivpu_to_cpu_addr(struct ivpu_bo *bo, u64 vpu_addr, u64 size)
 {
+	u64 bo_size = ivpu_bo_size(bo);
+	u64 offset;
+
 	if (vpu_addr < bo->vpu_addr)
 		return NULL;
 
-	if (vpu_addr >= (bo->vpu_addr + ivpu_bo_size(bo)))
+	if (size > bo_size)
+		return NULL;
+
+	offset = vpu_addr - bo->vpu_addr;
+
+	if (offset > bo_size - size)
 		return NULL;
 
-	return ivpu_bo_vaddr(bo) + (vpu_addr - bo->vpu_addr);
+	return ivpu_bo_vaddr(bo) + offset;
 }
 
 static inline u32 cpu_to_vpu_addr(struct ivpu_bo *bo, void *cpu_addr)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index 978bc3d8704f..8f69fb133e2e 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -79,7 +79,7 @@ ivpu_ipc_tx_prepare(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
 		return -ENOMEM;
 	}
 
-	tx_buf = ivpu_to_cpu_addr(ipc->mem_tx, tx_buf_vpu_addr);
+	tx_buf = ivpu_to_cpu_addr(ipc->mem_tx, tx_buf_vpu_addr, sizeof(*tx_buf));
 	if (drm_WARN_ON(&vdev->drm, !tx_buf)) {
 		gen_pool_free(ipc->mm_tx, tx_buf_vpu_addr, sizeof(*tx_buf));
 		return -EIO;
@@ -420,7 +420,7 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
 			return;
 		}
 
-		ipc_hdr = ivpu_to_cpu_addr(ipc->mem_rx, vpu_addr);
+		ipc_hdr = ivpu_to_cpu_addr(ipc->mem_rx, vpu_addr, sizeof(*ipc_hdr));
 		if (!ipc_hdr) {
 			ivpu_warn_ratelimited(vdev, "IPC msg 0x%x out of range\n", vpu_addr);
 			continue;
@@ -429,7 +429,8 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
 
 		jsm_msg = NULL;
 		if (ipc_hdr->channel != IVPU_IPC_CHAN_BOOT_MSG) {
-			jsm_msg = ivpu_to_cpu_addr(ipc->mem_rx, ipc_hdr->data_addr);
+			jsm_msg = ivpu_to_cpu_addr(ipc->mem_rx, ipc_hdr->data_addr,
+						   sizeof(*jsm_msg));
 			if (!jsm_msg) {
 				ivpu_warn_ratelimited(vdev, "JSM msg 0x%x out of range\n",
 						      ipc_hdr->data_addr);
-- 
2.43.0

Re: [PATCH v2 1/3] accel/ivpu: Validate full buffer range in ivpu_to_cpu_addr

[Wachowski, Karol]

On 01-Sep-26 14:57, Dawid Osuchowski wrote:
> From: Magdalena Schulfer <magdalena.schulfer@intel.com>
> 
> Add a size parameter to ivpu_to_cpu_addr() and validate that the
> whole [vpu_addr, vpu_addr + size) range stays within the BO.
> 
> Cc: stable@vger.kernel.org
> Fixes: 647371a6609d ("accel/ivpu: Add GEM buffer object management")
> Signed-off-by: Magdalena Schulfer <magdalena.schulfer@intel.com>
> Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> ---
>   drivers/accel/ivpu/ivpu_gem.h | 14 +++++++++++---
>   drivers/accel/ivpu/ivpu_ipc.c |  7 ++++---
>   2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_gem.h b/drivers/accel/ivpu/ivpu_gem.h
> index 0c3350f22b55..b1ae020a4fc2 100644
> --- a/drivers/accel/ivpu/ivpu_gem.h
> +++ b/drivers/accel/ivpu/ivpu_gem.h
> @@ -87,15 +87,23 @@ static inline bool ivpu_bo_is_resident(struct ivpu_bo *bo)
>   	return !!bo->base.pages;
>   }
>   
> -static inline void *ivpu_to_cpu_addr(struct ivpu_bo *bo, u32 vpu_addr)
> +static inline void *ivpu_to_cpu_addr(struct ivpu_bo *bo, u64 vpu_addr, u64 size)
>   {
> +	u64 bo_size = ivpu_bo_size(bo);
> +	u64 offset;
> +
>   	if (vpu_addr < bo->vpu_addr)
>   		return NULL;
>   
> -	if (vpu_addr >= (bo->vpu_addr + ivpu_bo_size(bo)))
> +	if (size > bo_size)
> +		return NULL;
> +
> +	offset = vpu_addr - bo->vpu_addr;
> +
> +	if (offset > bo_size - size)
>   		return NULL;
>   
> -	return ivpu_bo_vaddr(bo) + (vpu_addr - bo->vpu_addr);
> +	return ivpu_bo_vaddr(bo) + offset;
>   }
>   
>   static inline u32 cpu_to_vpu_addr(struct ivpu_bo *bo, void *cpu_addr)
> diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
> index 978bc3d8704f..8f69fb133e2e 100644
> --- a/drivers/accel/ivpu/ivpu_ipc.c
> +++ b/drivers/accel/ivpu/ivpu_ipc.c
> @@ -79,7 +79,7 @@ ivpu_ipc_tx_prepare(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
>   		return -ENOMEM;
>   	}
>   
> -	tx_buf = ivpu_to_cpu_addr(ipc->mem_tx, tx_buf_vpu_addr);
> +	tx_buf = ivpu_to_cpu_addr(ipc->mem_tx, tx_buf_vpu_addr, sizeof(*tx_buf));
>   	if (drm_WARN_ON(&vdev->drm, !tx_buf)) {
>   		gen_pool_free(ipc->mm_tx, tx_buf_vpu_addr, sizeof(*tx_buf));
>   		return -EIO;
> @@ -420,7 +420,7 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
>   			return;
>   		}
>   
> -		ipc_hdr = ivpu_to_cpu_addr(ipc->mem_rx, vpu_addr);
> +		ipc_hdr = ivpu_to_cpu_addr(ipc->mem_rx, vpu_addr, sizeof(*ipc_hdr));
>   		if (!ipc_hdr) {
>   			ivpu_warn_ratelimited(vdev, "IPC msg 0x%x out of range\n", vpu_addr);
>   			continue;
> @@ -429,7 +429,8 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev)
>   
>   		jsm_msg = NULL;
>   		if (ipc_hdr->channel != IVPU_IPC_CHAN_BOOT_MSG) {
> -			jsm_msg = ivpu_to_cpu_addr(ipc->mem_rx, ipc_hdr->data_addr);
> +			jsm_msg = ivpu_to_cpu_addr(ipc->mem_rx, ipc_hdr->data_addr,
> +						   sizeof(*jsm_msg));
>   			if (!jsm_msg) {
>   				ivpu_warn_ratelimited(vdev, "JSM msg 0x%x out of range\n",
>   						      ipc_hdr->data_addr);
Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>

[PATCH v2 2/3] accel/ivpu: Validate firmware log buffer metadata

[Dawid Osuchowski]

From: Magdalena Schulfer <magdalena.schulfer@intel.com>

The tracing log headers parsed by fw_log_print_buffer() reside in
DMA-shared BOs that the NPU firmware can write to.

fw_log_from_bo() validated log->header_size and log->size, but
fw_log_print_buffer() re-read those same fields from shared memory
afterwards, allowing a TOCTOU where firmware changes them between the
check and the use, and making the host dereference out-of-bounds
addresses while printing logs.

Snapshot the validated values once with READ_ONCE() and pass them down
explicitly in a new struct ivpu_fw_log_desc instead of re-reading them
from the shared struct.

Cc: stable@vger.kernel.org
Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
Signed-off-by: Magdalena Schulfer <magdalena.schulfer@intel.com>
Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_fw_log.c | 76 +++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
index 716467aa3156..4f9055aa9d33 100644
--- a/drivers/accel/ivpu/ivpu_fw_log.c
+++ b/drivers/accel/ivpu/ivpu_fw_log.c
@@ -26,10 +26,17 @@ MODULE_PARM_DESC(fw_log_level,
 		 " error=" __stringify(IVPU_FW_LOG_ERROR)
 		 " fatal=" __stringify(IVPU_FW_LOG_FATAL));
 
+struct ivpu_fw_log_desc {
+	struct vpu_tracing_buffer_header *log;
+	u32 header_size;
+	u32 size;
+};
+
 static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *offset,
-			  struct vpu_tracing_buffer_header **out_log)
+			  struct ivpu_fw_log_desc *desc)
 {
 	struct vpu_tracing_buffer_header *log;
+	u32 header_size, size;
 
 	if ((*offset + sizeof(*log)) > ivpu_bo_size(bo))
 		return -EINVAL;
@@ -39,26 +46,32 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
 	if (log->vpu_canary_start != VPU_TRACING_BUFFER_CANARY)
 		return -EINVAL;
 
-	if (log->header_size < sizeof(*log) || log->header_size > 1024) {
-		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log->header_size);
+	header_size = READ_ONCE(log->header_size);
+	size = READ_ONCE(log->size);
+
+	if (header_size < sizeof(*log) || header_size > 1024) {
+		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", header_size);
 		return -EINVAL;
 	}
-	if (log->size < log->header_size) {
-		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
+	if ((char *)log + size > (char *)ivpu_bo_vaddr(bo) + ivpu_bo_size(bo)) {
+		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", 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);
+	if (size < header_size) {
+		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x < header size 0x%x\n",
+			 size, header_size);
 		return -EINVAL;
 	}
 
-	*out_log = log;
-	*offset += log->size;
+	desc->log = log;
+	desc->header_size = header_size;
+	desc->size = size;
+	*offset += size;
 
 	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);
+		 log->name, log->write_index, size, log->wrap_count, log->header_version,
+		 header_size, log->format, log->alignment);
 
 	return 0;
 }
@@ -94,11 +107,12 @@ static void fw_log_print_lines(char *buffer, u32 size, struct drm_printer *p)
 		drm_printf(p, "%s", line);
 }
 
-static void fw_log_print_buffer(struct vpu_tracing_buffer_header *log, const char *prefix,
+static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefix,
 				bool only_new_msgs, struct drm_printer *p)
 {
-	char *log_data = (void *)log + log->header_size;
-	u32 data_size = log->size - log->header_size;
+	struct vpu_tracing_buffer_header *log = desc->log;
+	char *log_data = (void *)log + desc->header_size;
+	u32 data_size = desc->size - desc->header_size;
 	u32 log_start = only_new_msgs ? READ_ONCE(log->read_index) : 0;
 	u32 log_end = READ_ONCE(log->write_index);
 
@@ -134,11 +148,11 @@ static void
 fw_log_print_all_in_bo(struct ivpu_device *vdev, const char *name,
 		       struct ivpu_bo *bo, bool only_new_msgs, struct drm_printer *p)
 {
-	struct vpu_tracing_buffer_header *log;
+	struct ivpu_fw_log_desc desc;
 	u32 next = 0;
 
-	while (fw_log_from_bo(vdev, bo, &next, &log) == 0)
-		fw_log_print_buffer(log, name, only_new_msgs, p);
+	while (fw_log_from_bo(vdev, bo, &next, &desc) == 0)
+		fw_log_print_buffer(&desc, name, only_new_msgs, p);
 }
 
 void ivpu_fw_log_print(struct ivpu_device *vdev, bool only_new_msgs, struct drm_printer *p)
@@ -149,36 +163,36 @@ void ivpu_fw_log_print(struct ivpu_device *vdev, bool only_new_msgs, struct drm_
 
 void ivpu_fw_log_mark_read(struct ivpu_device *vdev)
 {
-	struct vpu_tracing_buffer_header *log;
+	struct ivpu_fw_log_desc desc;
 	u32 next;
 
 	next = 0;
-	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &log) == 0) {
-		log->read_index = READ_ONCE(log->write_index);
-		log->read_wrap_count = READ_ONCE(log->wrap_count);
+	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &desc) == 0) {
+		desc.log->read_index = READ_ONCE(desc.log->write_index);
+		desc.log->read_wrap_count = READ_ONCE(desc.log->wrap_count);
 	}
 
 	next = 0;
-	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &log) == 0) {
-		log->read_index = READ_ONCE(log->write_index);
-		log->read_wrap_count = READ_ONCE(log->wrap_count);
+	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &desc) == 0) {
+		desc.log->read_index = READ_ONCE(desc.log->write_index);
+		desc.log->read_wrap_count = READ_ONCE(desc.log->wrap_count);
 	}
 }
 
 void ivpu_fw_log_reset(struct ivpu_device *vdev)
 {
-	struct vpu_tracing_buffer_header *log;
+	struct ivpu_fw_log_desc desc;
 	u32 next;
 
 	next = 0;
-	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &log) == 0) {
-		log->read_index = 0;
-		log->read_wrap_count = 0;
+	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &desc) == 0) {
+		desc.log->read_index = 0;
+		desc.log->read_wrap_count = 0;
 	}
 
 	next = 0;
-	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &log) == 0) {
-		log->read_index = 0;
-		log->read_wrap_count = 0;
+	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &desc) == 0) {
+		desc.log->read_index = 0;
+		desc.log->read_wrap_count = 0;
 	}
 }
-- 
2.43.0

Re: [PATCH v2 2/3] accel/ivpu: Validate firmware log buffer metadata

[Wachowski, Karol]

On 01-Sep-26 14:57, Dawid Osuchowski wrote:
> From: Magdalena Schulfer <magdalena.schulfer@intel.com>
> 
> The tracing log headers parsed by fw_log_print_buffer() reside in
> DMA-shared BOs that the NPU firmware can write to.
> 
> fw_log_from_bo() validated log->header_size and log->size, but
> fw_log_print_buffer() re-read those same fields from shared memory
> afterwards, allowing a TOCTOU where firmware changes them between the
> check and the use, and making the host dereference out-of-bounds
> addresses while printing logs.
> 
> Snapshot the validated values once with READ_ONCE() and pass them down
> explicitly in a new struct ivpu_fw_log_desc instead of re-reading them
> from the shared struct.
> 
> Cc: stable@vger.kernel.org
> Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
> Signed-off-by: Magdalena Schulfer <magdalena.schulfer@intel.com>
> Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> ---
>   drivers/accel/ivpu/ivpu_fw_log.c | 76 +++++++++++++++++++-------------
>   1 file changed, 45 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
> index 716467aa3156..4f9055aa9d33 100644
> --- a/drivers/accel/ivpu/ivpu_fw_log.c
> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
> @@ -26,10 +26,17 @@ MODULE_PARM_DESC(fw_log_level,
>   		 " error=" __stringify(IVPU_FW_LOG_ERROR)
>   		 " fatal=" __stringify(IVPU_FW_LOG_FATAL));
>   
> +struct ivpu_fw_log_desc {
> +	struct vpu_tracing_buffer_header *log;
> +	u32 header_size;
> +	u32 size;
> +};
> +
>   static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *offset,
> -			  struct vpu_tracing_buffer_header **out_log)
> +			  struct ivpu_fw_log_desc *desc)
>   {
>   	struct vpu_tracing_buffer_header *log;
> +	u32 header_size, size;
>   
>   	if ((*offset + sizeof(*log)) > ivpu_bo_size(bo))
>   		return -EINVAL;
> @@ -39,26 +46,32 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
>   	if (log->vpu_canary_start != VPU_TRACING_BUFFER_CANARY)
>   		return -EINVAL;
>   
> -	if (log->header_size < sizeof(*log) || log->header_size > 1024) {
> -		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", log->header_size);
> +	header_size = READ_ONCE(log->header_size);
> +	size = READ_ONCE(log->size);
> +
> +	if (header_size < sizeof(*log) || header_size > 1024) {
> +		ivpu_dbg(vdev, FW_BOOT, "Invalid header size 0x%x\n", header_size);
>   		return -EINVAL;
>   	}
> -	if (log->size < log->header_size) {
> -		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", log->size);
> +	if ((char *)log + size > (char *)ivpu_bo_vaddr(bo) + ivpu_bo_size(bo)) {
> +		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x\n", 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);
> +	if (size < header_size) {
> +		ivpu_dbg(vdev, FW_BOOT, "Invalid log size 0x%x < header size 0x%x\n",
> +			 size, header_size);
>   		return -EINVAL;
>   	}
>   
> -	*out_log = log;
> -	*offset += log->size;
> +	desc->log = log;
> +	desc->header_size = header_size;
> +	desc->size = size;
> +	*offset += size;
>   
>   	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);
> +		 log->name, log->write_index, size, log->wrap_count, log->header_version,
> +		 header_size, log->format, log->alignment);
>   
>   	return 0;
>   }
> @@ -94,11 +107,12 @@ static void fw_log_print_lines(char *buffer, u32 size, struct drm_printer *p)
>   		drm_printf(p, "%s", line);
>   }
>   
> -static void fw_log_print_buffer(struct vpu_tracing_buffer_header *log, const char *prefix,
> +static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefix,
>   				bool only_new_msgs, struct drm_printer *p)
>   {
> -	char *log_data = (void *)log + log->header_size;
> -	u32 data_size = log->size - log->header_size;
> +	struct vpu_tracing_buffer_header *log = desc->log;
> +	char *log_data = (void *)log + desc->header_size;
> +	u32 data_size = desc->size - desc->header_size;
>   	u32 log_start = only_new_msgs ? READ_ONCE(log->read_index) : 0;
>   	u32 log_end = READ_ONCE(log->write_index);
>   
> @@ -134,11 +148,11 @@ static void
>   fw_log_print_all_in_bo(struct ivpu_device *vdev, const char *name,
>   		       struct ivpu_bo *bo, bool only_new_msgs, struct drm_printer *p)
>   {
> -	struct vpu_tracing_buffer_header *log;
> +	struct ivpu_fw_log_desc desc;
>   	u32 next = 0;
>   
> -	while (fw_log_from_bo(vdev, bo, &next, &log) == 0)
> -		fw_log_print_buffer(log, name, only_new_msgs, p);
> +	while (fw_log_from_bo(vdev, bo, &next, &desc) == 0)
> +		fw_log_print_buffer(&desc, name, only_new_msgs, p);
>   }
>   
>   void ivpu_fw_log_print(struct ivpu_device *vdev, bool only_new_msgs, struct drm_printer *p)
> @@ -149,36 +163,36 @@ void ivpu_fw_log_print(struct ivpu_device *vdev, bool only_new_msgs, struct drm_
>   
>   void ivpu_fw_log_mark_read(struct ivpu_device *vdev)
>   {
> -	struct vpu_tracing_buffer_header *log;
> +	struct ivpu_fw_log_desc desc;
>   	u32 next;
>   
>   	next = 0;
> -	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &log) == 0) {
> -		log->read_index = READ_ONCE(log->write_index);
> -		log->read_wrap_count = READ_ONCE(log->wrap_count);
> +	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &desc) == 0) {
> +		desc.log->read_index = READ_ONCE(desc.log->write_index);
> +		desc.log->read_wrap_count = READ_ONCE(desc.log->wrap_count);
>   	}
>   
>   	next = 0;
> -	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &log) == 0) {
> -		log->read_index = READ_ONCE(log->write_index);
> -		log->read_wrap_count = READ_ONCE(log->wrap_count);
> +	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &desc) == 0) {
> +		desc.log->read_index = READ_ONCE(desc.log->write_index);
> +		desc.log->read_wrap_count = READ_ONCE(desc.log->wrap_count);
>   	}
>   }
>   
>   void ivpu_fw_log_reset(struct ivpu_device *vdev)
>   {
> -	struct vpu_tracing_buffer_header *log;
> +	struct ivpu_fw_log_desc desc;
>   	u32 next;
>   
>   	next = 0;
> -	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &log) == 0) {
> -		log->read_index = 0;
> -		log->read_wrap_count = 0;
> +	while (fw_log_from_bo(vdev, vdev->fw->mem_log_crit, &next, &desc) == 0) {
> +		desc.log->read_index = 0;
> +		desc.log->read_wrap_count = 0;
>   	}
>   
>   	next = 0;
> -	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &log) == 0) {
> -		log->read_index = 0;
> -		log->read_wrap_count = 0;
> +	while (fw_log_from_bo(vdev, vdev->fw->mem_log_verb, &next, &desc) == 0) {
> +		desc.log->read_index = 0;
> +		desc.log->read_wrap_count = 0;
>   	}
>   }

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

[PATCH v2 3/3] accel/ivpu: Limit firmware log name prints to field size

[Dawid Osuchowski]

The name in struct vpu_tracing_buffer_header is a fixed-size array
populated by the NPU firmware. It is expected to be NUL-terminated,
but nothing on the host side enforces this, so printing it with an
unbounded string conversion would read past the field if the
terminator is ever missing and expose adjacent bytes of the shared
tracing BO through dmesg and the debugfs FW log output.

Print at most as many characters as the name field holds, so the output
never runs past it even if the string is not NUL-terminated.

Cc: stable@vger.kernel.org
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260827102339.281799-1-dawid.osuchowski@linux.intel.com?part=2
Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_fw_log.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
index 4f9055aa9d33..9eafc42120b6 100644
--- a/drivers/accel/ivpu/ivpu_fw_log.c
+++ b/drivers/accel/ivpu/ivpu_fw_log.c
@@ -69,9 +69,9 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
 	*offset += size;
 
 	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, size, log->wrap_count, log->header_version,
-		 header_size, log->format, log->alignment);
+		 "FW log name \"%.*s\", write offset 0x%x size 0x%x, wrap count %d, hdr version %d size %d format %d, alignment %d",
+		 (int)ARRAY_SIZE(log->name), log->name, log->write_index, size, log->wrap_count,
+		 log->header_version, header_size, log->format, log->alignment);
 
 	return 0;
 }
@@ -123,7 +123,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
 
 	if (log->wrap_count == log->read_wrap_count) {
 		if (log_end <= log_start) {
-			drm_printf(p, "==== %s \"%s\" log empty ====\n", prefix, log->name);
+			drm_printf(p, "==== %s \"%.*s\" log empty ====\n", prefix,
+				   (int)ARRAY_SIZE(log->name), log->name);
 			return;
 		}
 	} else if (log->wrap_count == log->read_wrap_count + 1) {
@@ -133,7 +134,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
 		log_start = log_end;
 	}
 
-	drm_printf(p, "==== %s \"%s\" log start ====\n", prefix, log->name);
+	drm_printf(p, "==== %s \"%.*s\" log start ====\n", prefix, (int)ARRAY_SIZE(log->name),
+		   log->name);
 	if (log_end > log_start) {
 		fw_log_print_lines(log_data + log_start, log_end - log_start, p);
 	} else {
@@ -141,7 +143,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
 		fw_log_print_lines(log_data, log_end, p);
 	}
 	drm_printf(p, "\n\x1b[0m"); /* add new line and clear formatting */
-	drm_printf(p, "==== %s \"%s\" log end   ====\n", prefix, log->name);
+	drm_printf(p, "==== %s \"%.*s\" log end   ====\n", prefix, (int)ARRAY_SIZE(log->name),
+		   log->name);
 }
 
 static void
-- 
2.43.0

Re: [PATCH v2 3/3] accel/ivpu: Limit firmware log name prints to field size

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded string read from firmware data in `ivpu_fw_parse`.
--

commit 7d0d9c625b89d41a9d3cbe859e6e2b33348102cf
Author: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

accel/ivpu: Limit firmware log name prints to field size
    
This patch fixes an issue where the NPU firmware log name field is printed
using an unbounded string conversion. By enforcing a precision limit based
on the field's size, it prevents out-of-bounds memory reads if the firmware
string is missing its NUL terminator.

Closes: https://sashiko.dev/#/patchset/20260827102339.281799-1-dawid.osuchowski@linux.intel.com?part=2

> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
> index 4f9055aa9d33f..9eafc42120b6a 100644
> --- a/drivers/accel/ivpu/ivpu_fw_log.c
> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
> @@ -69,9 +69,9 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
>  	*offset += size;
>  
>  	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, size, log->wrap_count, log->header_version,
> -		 header_size, log->format, log->alignment);
> +		 "FW log name \"%.*s\", write offset 0x%x size 0x%x, wrap count %d, hdr version %d size %d format %d, alignment %d",
> +		 (int)ARRAY_SIZE(log->name), log->name, log->write_index, size, log->wrap_count,
> +		 log->header_version, header_size, log->format, log->alignment);

[Severity: High]
This isn't a bug introduced by this patch, but since this patch fixes an
unbounded string read, is there a similar issue in ivpu_fw_parse() when
reading the firmware version?

Looking at drivers/accel/ivpu/ivpu_fw.c:ivpu_fw_parse():

	if (!scnprintf(fw->version, sizeof(fw->version), "%s", fw->file->data + VPU_FW_HEADER_SIZE))
		ivpu_warn(vdev, "Missing firmware version\n");

The scnprintf() function evaluates the length of the source string until a NUL
byte is found when no precision is provided for "%s", regardless of the
destination buffer size.

If the loaded firmware file lacks a NUL terminator at VPU_FW_HEADER_SIZE,
could this lead to an out-of-bounds read past the loaded firmware data?

This could trigger a page fault and expose adjacent kernel memory if a
malformed firmware file is supplied.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901125749.404338-1-dawid.osuchowski@linux.intel.com?part=3

Re: [PATCH v2 3/3] accel/ivpu: Limit firmware log name prints to field size

[Wachowski, Karol]

On 01-Sep-26 14:57, Dawid Osuchowski wrote:
> The name in struct vpu_tracing_buffer_header is a fixed-size array
> populated by the NPU firmware. It is expected to be NUL-terminated,
> but nothing on the host side enforces this, so printing it with an
> unbounded string conversion would read past the field if the
> terminator is ever missing and expose adjacent bytes of the shared
> tracing BO through dmesg and the debugfs FW log output.
> 
> Print at most as many characters as the name field holds, so the output
> never runs past it even if the string is not NUL-terminated.
> 
> Cc: stable@vger.kernel.org
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260827102339.281799-1-dawid.osuchowski@linux.intel.com?part=2
> Fixes: d4e4257afa6e ("accel/ivpu: Add firmware tracing support")
> Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> ---
>   drivers/accel/ivpu/ivpu_fw_log.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_fw_log.c b/drivers/accel/ivpu/ivpu_fw_log.c
> index 4f9055aa9d33..9eafc42120b6 100644
> --- a/drivers/accel/ivpu/ivpu_fw_log.c
> +++ b/drivers/accel/ivpu/ivpu_fw_log.c
> @@ -69,9 +69,9 @@ static int fw_log_from_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, u32 *off
>   	*offset += size;
>   
>   	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, size, log->wrap_count, log->header_version,
> -		 header_size, log->format, log->alignment);
> +		 "FW log name \"%.*s\", write offset 0x%x size 0x%x, wrap count %d, hdr version %d size %d format %d, alignment %d",
> +		 (int)ARRAY_SIZE(log->name), log->name, log->write_index, size, log->wrap_count,
> +		 log->header_version, header_size, log->format, log->alignment);
>   
>   	return 0;
>   }
> @@ -123,7 +123,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
>   
>   	if (log->wrap_count == log->read_wrap_count) {
>   		if (log_end <= log_start) {
> -			drm_printf(p, "==== %s \"%s\" log empty ====\n", prefix, log->name);
> +			drm_printf(p, "==== %s \"%.*s\" log empty ====\n", prefix,
> +				   (int)ARRAY_SIZE(log->name), log->name);
>   			return;
>   		}
>   	} else if (log->wrap_count == log->read_wrap_count + 1) {
> @@ -133,7 +134,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
>   		log_start = log_end;
>   	}
>   
> -	drm_printf(p, "==== %s \"%s\" log start ====\n", prefix, log->name);
> +	drm_printf(p, "==== %s \"%.*s\" log start ====\n", prefix, (int)ARRAY_SIZE(log->name),
> +		   log->name);
>   	if (log_end > log_start) {
>   		fw_log_print_lines(log_data + log_start, log_end - log_start, p);
>   	} else {
> @@ -141,7 +143,8 @@ static void fw_log_print_buffer(struct ivpu_fw_log_desc *desc, const char *prefi
>   		fw_log_print_lines(log_data, log_end, p);
>   	}
>   	drm_printf(p, "\n\x1b[0m"); /* add new line and clear formatting */
> -	drm_printf(p, "==== %s \"%s\" log end   ====\n", prefix, log->name);
> +	drm_printf(p, "==== %s \"%.*s\" log end   ====\n", prefix, (int)ARRAY_SIZE(log->name),
> +		   log->name);
>   }
>   
>   static void

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