public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl
@ 2026-05-29 12:08 Andrzej Kacprowski
  2026-05-29 12:23 ` Wachowski, Karol
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrzej Kacprowski @ 2026-05-29 12:08 UTC (permalink / raw)
  To: dri-devel
  Cc: oded.gabbay, jeff.hugo, lizhi.hou, karol.wachowski,
	dawid.osuchowski, Andrzej Kacprowski, stable

Add validation that the info size returned from the metric stream info
query is not exceeded when checked against the allocated buffer size.
If the firmware returns a size larger than the buffer, reject the
operation with -EOVERFLOW instead of proceeding with an incorrect
buffer copy.

Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support")
Cc: <stable@vger.kernel.org> # v6.18+
Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_ms.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c
index be43851f5f32..cd176e77b9a0 100644
--- a/drivers/accel/ivpu/ivpu_ms.c
+++ b/drivers/accel/ivpu/ivpu_ms.c
@@ -291,6 +291,13 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file *
 	if (ret)
 		goto unlock;
 
+	if (info_size > ivpu_bo_size(bo)) {
+		ivpu_warn_ratelimited(vdev, "MS info overflow: %#llx > %#zx\n",
+				      info_size, ivpu_bo_size(bo));
+		ret = -EOVERFLOW;
+		goto unlock;
+	}
+
 	if (args->buffer_size < info_size) {
 		ret = -ENOSPC;
 		goto unlock;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl
  2026-05-29 12:08 [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl Andrzej Kacprowski
@ 2026-05-29 12:23 ` Wachowski, Karol
  2026-06-02  5:49   ` Wachowski, Karol
  2026-06-04  6:35 ` Claude review: " Claude Code Review Bot
  2026-06-04  6:35 ` Claude Code Review Bot
  2 siblings, 1 reply; 5+ messages in thread
From: Wachowski, Karol @ 2026-05-29 12:23 UTC (permalink / raw)
  To: Andrzej Kacprowski, dri-devel
  Cc: oded.gabbay, jeff.hugo, lizhi.hou, dawid.osuchowski, stable

On 29-May-26 14:08, Andrzej Kacprowski wrote:
> Add validation that the info size returned from the metric stream info
> query is not exceeded when checked against the allocated buffer size.
> If the firmware returns a size larger than the buffer, reject the
> operation with -EOVERFLOW instead of proceeding with an incorrect
> buffer copy.
> 
> Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support")
> Cc: <stable@vger.kernel.org> # v6.18+
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>

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

> ---
>   drivers/accel/ivpu/ivpu_ms.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c
> index be43851f5f32..cd176e77b9a0 100644
> --- a/drivers/accel/ivpu/ivpu_ms.c
> +++ b/drivers/accel/ivpu/ivpu_ms.c
> @@ -291,6 +291,13 @@ int ivpu_ms_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file *
>   	if (ret)
>   		goto unlock;
>   
> +	if (info_size > ivpu_bo_size(bo)) {
> +		ivpu_warn_ratelimited(vdev, "MS info overflow: %#llx > %#zx\n",
> +				      info_size, ivpu_bo_size(bo));
> +		ret = -EOVERFLOW;
> +		goto unlock;
> +	}
> +
>   	if (args->buffer_size < info_size) {
>   		ret = -ENOSPC;
>   		goto unlock;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl
  2026-05-29 12:23 ` Wachowski, Karol
@ 2026-06-02  5:49   ` Wachowski, Karol
  0 siblings, 0 replies; 5+ messages in thread
From: Wachowski, Karol @ 2026-06-02  5:49 UTC (permalink / raw)
  To: Andrzej Kacprowski, dri-devel
  Cc: oded.gabbay, jeff.hugo, lizhi.hou, dawid.osuchowski, stable

On 29-May-26 14:23, Wachowski, Karol wrote:
> On 29-May-26 14:08, Andrzej Kacprowski wrote:
>> Add validation that the info size returned from the metric stream info
>> query is not exceeded when checked against the allocated buffer size.
>> If the firmware returns a size larger than the buffer, reject the
>> operation with -EOVERFLOW instead of proceeding with an incorrect
>> buffer copy.
>>
>> Fixes: cdfad4db7756 ("accel/ivpu: Add NPU profiling support")
>> Cc: <stable@vger.kernel.org> # v6.18+
>> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> 
> Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>

Applied to drm-misc-fixes.

> 
>> ---
>>   drivers/accel/ivpu/ivpu_ms.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c
>> index be43851f5f32..cd176e77b9a0 100644
>> --- a/drivers/accel/ivpu/ivpu_ms.c
>> +++ b/drivers/accel/ivpu/ivpu_ms.c
>> @@ -291,6 +291,13 @@ int ivpu_ms_get_info_ioctl(struct drm_device 
>> *dev, void *data, struct drm_file *
>>       if (ret)
>>           goto unlock;
>> +    if (info_size > ivpu_bo_size(bo)) {
>> +        ivpu_warn_ratelimited(vdev, "MS info overflow: %#llx > %#zx\n",
>> +                      info_size, ivpu_bo_size(bo));
>> +        ret = -EOVERFLOW;
>> +        goto unlock;
>> +    }
>> +
>>       if (args->buffer_size < info_size) {
>>           ret = -ENOSPC;
>>           goto unlock;
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: accel/ivpu: Add buffer overflow check in MS get_info_ioctl
  2026-05-29 12:08 [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl Andrzej Kacprowski
  2026-05-29 12:23 ` Wachowski, Karol
@ 2026-06-04  6:35 ` Claude Code Review Bot
  2026-06-04  6:35 ` Claude Code Review Bot
  2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  6:35 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: accel/ivpu: Add buffer overflow check in MS get_info_ioctl
Author: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
Patches: 3
Reviewed: 2026-06-04T16:35:50.824603

---

This is a single-patch fix for a buffer overflow vulnerability in the Intel NPU (IVPU) metric streamer `get_info` ioctl. The fix is straightforward and correct: it validates that the `info_size` value returned by firmware does not exceed the size of the kernel buffer object (`bo`) before that value is used in a `copy_to_user` call. Without this check, a misbehaving firmware could cause an out-of-bounds read from kernel memory into userspace.

The patch is appropriate for stable backport (tagged `v6.18+` to match when the profiling support was added). The fix is well-placed and minimal.

**Verdict: Looks good. One minor nit on the format string.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: accel/ivpu: Add buffer overflow check in MS get_info_ioctl
  2026-05-29 12:08 [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl Andrzej Kacprowski
  2026-05-29 12:23 ` Wachowski, Karol
  2026-06-04  6:35 ` Claude review: " Claude Code Review Bot
@ 2026-06-04  6:35 ` Claude Code Review Bot
  2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04  6:35 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Purpose and correctness:**

The call chain is:
1. `ivpu_jsm_metric_streamer_info()` is called with `ivpu_bo_size(bo)` as the buffer size limit (line 290), and fills `info_size` (a `u64`) with the actual data size returned by firmware.
2. `info_size` is then used directly at line 299 in `copy_to_user(..., info_size)`.

If firmware returns an `info_size` larger than `ivpu_bo_size(bo)`, the `copy_to_user` would read past the end of the BO's mapped memory. The new check correctly catches this case before the data is used.

The placement is correct — right after the JSM call succeeds (line 291-292) and before either the `args->buffer_size` comparison or the `copy_to_user`. This ensures the firmware-returned size is validated against the kernel buffer before any further use.

**Format string nit:**

```c
ivpu_warn_ratelimited(vdev, "MS info overflow: %#llx > %#zx\n",
                      info_size, ivpu_bo_size(bo));
```

`info_size` is `u64` — in the kernel, the canonical format specifier for `u64` is `%llu` (or `%#llx` for hex, which is used here). That's fine. `ivpu_bo_size()` returns `size_t`, and `%zx` is appropriate for that. So the format specifiers are correct.

However, one could argue that a decimal format would be more readable for sizes in a warning message (comparing `0x10001` vs `0x10000` is harder to eyeball than `65537 > 65536`). This is purely a style preference and not a bug.

**Error code:**

`-EOVERFLOW` is a reasonable choice for this condition — it clearly conveys that the returned data would overflow the allocated buffer. Some drivers use `-ENOSPC` for related conditions, but `EOVERFLOW` better captures the "firmware returned more than it should have" semantics, distinguishing it from the existing `-ENOSPC` check on the user buffer (line 294).

**No concerns with the fix itself.** The patch is correct, minimal, and addresses a real vulnerability where firmware could cause an out-of-bounds kernel memory read that gets copied to userspace.

Reviewed-by assessment: **Acceptable as-is.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-04  6:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 12:08 [PATCH] accel/ivpu: Add buffer overflow check in MS get_info_ioctl Andrzej Kacprowski
2026-05-29 12:23 ` Wachowski, Karol
2026-06-02  5:49   ` Wachowski, Karol
2026-06-04  6:35 ` Claude review: " Claude Code Review Bot
2026-06-04  6:35 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox