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

[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

[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>