Claude review: drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**The bug is real and the fix prevents the crash.** The NULL dereference is userspace-triggerable via the `MSM_GET_PARAM` ioctl, making this a local DoS / kernel oops from unprivileged userspace. The `Fixes:` tag is appropriate.

**Design concern: returning fabricated defaults vs returning -EINVAL.**

The patch returns hardcoded values (`13` for `HIGHEST_BANK_BIT`, `0` for the others) when `ubwc_config` is NULL:

```c
	case MSM_PARAM_HIGHEST_BANK_BIT:
		if (!adreno_gpu->ubwc_config)
			*value = 13;
		else
			*value = adreno_gpu->ubwc_config->highest_bank_bit;
```

While `13` is a reasonable default for `highest_bank_bit` (it matches the value used in `a5xx_gpu.c:838` where `BUG_ON(adreno_gpu->ubwc_config->highest_bank_bit < 13)` shows 13 is the baseline), returning fabricated UBWC parameters for hardware that **doesn't support UBWC at all** is semantically questionable. These params are meaningless on a2xx-a4xx.

An alternative would be to return `-EINVAL` for these three params when `ubwc_config` is NULL, similar to how `MSM_PARAM_TIMESTAMP` returns `-EINVAL` when the GPU doesn't support timestamps. This would clearly communicate to userspace "this GPU does not have UBWC" rather than silently returning plausible-looking but meaningless values. If existing userspace (e.g., freedreno in Mesa) depends on getting a success return from these params without checking the GPU generation first, that would justify the current approach — but a comment explaining why defaults were chosen over `-EINVAL` would be helpful.

**Minor style observation: repetitive NULL checks.**

The three cases repeat the same `if (!adreno_gpu->ubwc_config)` pattern. A small refactor could consolidate:

```c
	case MSM_PARAM_HIGHEST_BANK_BIT:
	case MSM_PARAM_UBWC_SWIZZLE:
	case MSM_PARAM_MACROTILE_MODE:
		if (!adreno_gpu->ubwc_config) {
			*value = (param == MSM_PARAM_HIGHEST_BANK_BIT) ? 13 : 0;
			return 0;
		}
		/* fall through to individual handling */
```

But this is a matter of taste for a simple bugfix, and the current form is perfectly readable.

**Missing Cc: stable.** Given this is a userspace-triggerable NULL pointer dereference, this should probably carry `Cc: stable@vger.kernel.org` to get backported to affected stable kernels (those carrying `a452510aad53`).

**Overall verdict:** The patch fixes a real crash. The only substantive question is whether returning `-EINVAL` would be more appropriate than returning dummy values for hardware that has no UBWC support. If the maintainers are happy with the default-value approach (and Mesa handles it correctly), the patch is ready to merge with the possible addition of a `Cc: stable` tag.

---
Generated by Claude Code Patch Reviewer