[PATCH] accel/habanalabs: reject zero-element timestamp buffer allocation

[PATCH] accel/habanalabs: reject zero-element timestamp buffer allocation

[Ziyi Guo]

A user can issue a DRM_IOCTL_HL_MEMORY ioctl with
op=HL_MEM_OP_TS_ALLOC and num_of_elements=0. The
allocate_timestamps_buffers() function only validates the upper bound
(> TS_MAX_ELEMENTS_NUM) but not zero, allowing num_of_elements=0 to
reach vmalloc_user(0 * sizeof(u64)), which triggers WARN_ON_ONCE(!size)
in __vmalloc_node_range().

On systems with panic_on_warn=1, this allows a local user with device
access to crash the kernel.

Add a zero check to the existing validation, matching the pattern
already present in HL_MEM_OP_ALLOC (memory.c:2214).

Fixes: 9158bf69e74f ("habanalabs: Timestamps buffers registration")
Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
---
 drivers/accel/habanalabs/common/memory.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/habanalabs/common/memory.c b/drivers/accel/habanalabs/common/memory.c
index 633db4bff46f..37dbb9a013bf 100644
--- a/drivers/accel/habanalabs/common/memory.c
+++ b/drivers/accel/habanalabs/common/memory.c
@@ -2176,8 +2176,9 @@ static int allocate_timestamps_buffers(struct hl_fpriv *hpriv, struct hl_mem_in
 	struct hl_mem_mgr *mmg = &hpriv->mem_mgr;
 	struct hl_mmap_mem_buf *buf;
 
-	if (args->num_of_elements > TS_MAX_ELEMENTS_NUM) {
-		dev_err(mmg->dev, "Num of elements exceeds Max allowed number (0x%x > 0x%x)\n",
+	if (args->num_of_elements > TS_MAX_ELEMENTS_NUM ||
+				args->num_of_elements == 0) {
+		dev_err(mmg->dev, "Invalid num of elements %u, valid range [1, 0x%x]\n",
 				args->num_of_elements, TS_MAX_ELEMENTS_NUM);
 		return -EINVAL;
 	}
-- 
2.34.1

Claude review: accel/habanalabs: reject zero-element timestamp buffer allocation

[Claude Code Review Bot]

Overall Series Review

Subject: accel/habanalabs: reject zero-element timestamp buffer allocation
Author: Ziyi Guo <n7l8m4@u.northwestern.edu>
Patches: 1
Reviewed: 2026-02-14T07:24:29.658724

---

This is a single-patch submission that adds a zero-element check to the timestamp buffer allocation path in the habanalabs (accel) driver. The fix addresses a real issue: a userspace caller can pass `num_of_elements=0` via the `DRM_IOCTL_HL_MEMORY` ioctl with `op=HL_MEM_OP_TS_ALLOC`, which flows unchecked into `vmalloc_user(0)` and triggers a `WARN_ON_ONCE(!size)` in `__vmalloc_node_range()`. On `panic_on_warn=1` systems, this is a local denial-of-service.

The fix is straightforward and correct. The approach — extending the existing upper-bound check to also reject zero — is the right thing to do. The commit message is well-written, explains the root cause clearly, references the matching pattern at `HL_MEM_OP_ALLOC`, and includes an appropriate Fixes tag.

No significant issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/habanalabs: reject zero-element timestamp buffer allocation

[Claude Code Review Bot]

Patch Review

The analysis checks out. The call chain is:

`hl_mem_ioctl()` → `allocate_timestamps_buffers()` → `hl_mmap_mem_buf_alloc()` → `hl_ts_alloc_buf()`

In `hl_ts_alloc_buf()`, the num_of_elements value is used in `vmalloc_user(num_elements * sizeof(u64))`. With `num_of_elements=0`, the size is zero, which hits the `WARN_ON_ONCE` in the vmalloc internals. The value comes directly from userspace via the ioctl `args->num_of_elements` with no intervening validation for zero.

The fix itself:

> ```
> -	if (args->num_of_elements > TS_MAX_ELEMENTS_NUM) {
> -		dev_err(mmg->dev, "Num of elements exceeds Max allowed number (0x%x > 0x%x)\n",
> +	if (args->num_of_elements > TS_MAX_ELEMENTS_NUM ||
> +				args->num_of_elements == 0) {
> +		dev_err(mmg->dev, "Invalid num of elements %u, valid range [1, 0x%x]\n",
>  				args->num_of_elements, TS_MAX_ELEMENTS_NUM);
>  		return -EINVAL;
>  	}
> ```

One minor style note: the condition could be written more naturally as `!args->num_of_elements || args->num_of_elements > TS_MAX_ELEMENTS_NUM` (checking the lower bound first, then the upper bound), which reads as a range check. But this is purely a readability preference — the logic is correct either way.

The updated error message is a good improvement — it now describes the valid range rather than only the upper-bound violation, which is more informative when the zero case is hit.

No bugs found. The patch is correct and ready to apply.

---
Generated by Claude Code Patch Reviewer