[PATCH] gpu: host1x: Fix passing zero to ERR_PTR in host1x_iommu_attach()

[PATCH] gpu: host1x: Fix passing zero to ERR_PTR in host1x_iommu_attach()

[Alper Ak]

When iommu_attach_group() returns -ENODEV, the code sets err to 0 but
still falls through to the error path, returning ERR_PTR(0).

Returning ERR_PTR(0) evaluates to NULL and breaks the ERR_PTR/IS_ERR
contract, causing the error to be silently ignored and potentially
leading to NULL pointer dereferences by callers.

Fix this by returning NULL when err is zero, and ERR_PTR(err) only
for actual error codes.

This issue was reported by the Smatch static analyzer.

Fixes: 06867a362de0 ("gpu: host1x: Set DMA mask based on IOMMU setup")
Signed-off-by: Alper Ak <alperyasinak1@gmail.com>
---
 drivers/gpu/host1x/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 3f475f0e6545..46a570b861ac 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -450,7 +450,7 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
 	iommu_group_put(host->group);
 	host->group = NULL;
 
-	return ERR_PTR(err);
+	return err ? ERR_PTR(err) : NULL;
 }
 
 static int host1x_iommu_init(struct host1x *host)
-- 
2.43.0

Claude review: gpu: host1x: Fix passing zero to ERR_PTR in host1x_iommu_attach()

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: host1x: Fix passing zero to ERR_PTR in
 host1x_iommu_attach()
Author: Alper Ak <alperyasinak1@gmail.com>
Patches: 1
Reviewed: 2026-02-11T17:05:33.927031

---

This is a single patch submission that addresses an error handling bug in the host1x IOMMU attachment code. The patch fixes a violation of the ERR_PTR/IS_ERR contract where ERR_PTR(0) could be returned, which evaluates to NULL and breaks the error handling convention in kernel code.

**Series Assessment:**
- **Scope**: Appropriately focused on a single bug fix
- **Target**: Correctly marked for stable backport (Cc: stable@vger.kernel.org)
- **Static Analysis Finding**: Identified by Smatch, which is a legitimate source for such bugs
- **Fix Quality**: The fix needs careful examination of the broader error handling semantics

**Key Concerns:**
1. The fix changes error handling semantics in a subtle way that requires validation
2. Need to verify all callers handle both NULL and ERR_PTR return values correctly
3. The original code path's intention when setting err=0 needs clarification

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: host1x: Fix passing zero to ERR_PTR in host1x_iommu_attach()

[Claude Code Review Bot]

Patch Review

**Commit Message Quality:**
- Clear problem statement ✓
- Explains the impact (NULL pointer dereferences) ✓
- Includes Fixes: tag ✓
- Includes Smatch attribution ✓
- Missing: Does not explain *why* err is set to 0 in the original code

**Technical Analysis:**

```c
-	return ERR_PTR(err);
+	return err ? ERR_PTR(err) : NULL;
```

**Issue #1: Incomplete Analysis of the Root Cause**

The commit message states that when `iommu_attach_group()` returns `-ENODEV`, the code sets `err` to 0. However, the patch doesn't show the context where this happens. Looking at the error path at line 117-122:

```
iommu_group_put(host->group);
host->group = NULL;

return err ? ERR_PTR(err) : NULL;
```

**Critical Question**: Where in the function is `err` being set to 0? The patch doesn't show this code path, making it impossible to verify:
1. Whether setting `err = 0` was intentional
2. Whether the condition for setting it to 0 is correct
3. Whether returning NULL in that case is the desired behavior

**Issue #2: Semantic Change in Error Handling**

The fix fundamentally changes the return value semantics:
- **Before**: Always returns an error pointer on this code path (even if erroneously ERR_PTR(0))
- **After**: May return NULL or an error pointer

This requires verification that all callers of `host1x_iommu_attach()` correctly handle both:
- `IS_ERR()` return values (for actual errors)
- `NULL` return values (for... what condition exactly?)

**Issue #3: Missing Context**

The patch doesn't show:
1. The function signature of `host1x_iommu_attach()`
2. The code path where `err` becomes 0
3. How callers check the return value
4. Whether NULL is a valid "success" return or a different error condition

**Recommendation:**

**NACK** in current form. The patch needs:

1. **Complete context**: Show the full function or at least the code path where `err = 0` is set
2. **Caller analysis**: Demonstrate that all callers of `host1x_iommu_attach()` handle NULL returns correctly
3. **Clarify semantics**: Explain what NULL represents vs ERR_PTR values:
   - If NULL means "IOMMU not available but that's OK", that should be documented
   - If NULL means something else, explain it
4. **Consider alternatives**: If `err = 0` on `-ENODEV` is intentional (meaning "gracefully handle missing IOMMU"), perhaps the function should return success on that path instead of falling through to the error path

**Suggested Investigation:**

```bash
# Show the full function context
git show HEAD:drivers/gpu/host1x/dev.c | grep -A 30 "host1x_iommu_attach"

# Find all callers
git grep -n "host1x_iommu_attach" drivers/
```

**Alternative Fix to Consider:**

If `-ENODEV` is meant to be non-fatal (thus `err = 0`), the code should probably return early:

```c
err = iommu_attach_group(domain, host->group);
if (err == -ENODEV) {
    /* IOMMU not available, gracefully fall back */
    iommu_domain_free(domain);
    iommu_group_put(host->group);
    host->group = NULL;
    return NULL;  /* Explicitly return NULL for "no IOMMU" case */
}
if (err) {
    /* Actual error */
    goto fail;
}
```

This would make the intent clearer and avoid the ERR_PTR(0) issue entirely.

**Verdict**: The patch addresses a real bug but the fix may be papering over a deeper design issue in the error handling. Need more context to determine the correct fix.

---
Generated by Claude Code Patch Reviewer