* [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
@ 2026-04-06 22:14 Dmitry Baryshkov
2026-04-06 22:24 ` Rob Clark
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Dmitry Baryshkov @ 2026-04-06 22:14 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Konrad Dybcio, Akhil P Oommen,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Marijn Suijten,
David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Before a5xx Adreno driver will not try fetching UBWC params (because
those generations didn't support UBWC anyway), however it's still
possible to query UBWC-related params from the userspace, triggering
possible NULL pointer dereference. Check for UBWC config in
adreno_get_param() and return sane defaults if there is none.
Fixes: a452510aad53 ("drm/msm/adreno: Switch to the common UBWC config struct")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index d5fe6f6f0dec..7dc95c0a17f7 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -426,16 +426,25 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
*value = vm->mm_range;
return 0;
case MSM_PARAM_HIGHEST_BANK_BIT:
- *value = adreno_gpu->ubwc_config->highest_bank_bit;
+ if (!adreno_gpu->ubwc_config)
+ *value = 13;
+ else
+ *value = adreno_gpu->ubwc_config->highest_bank_bit;
return 0;
case MSM_PARAM_RAYTRACING:
*value = adreno_gpu->has_ray_tracing;
return 0;
case MSM_PARAM_UBWC_SWIZZLE:
- *value = adreno_gpu->ubwc_config->ubwc_swizzle;
+ if (!adreno_gpu->ubwc_config)
+ *value = 0;
+ else
+ *value = adreno_gpu->ubwc_config->ubwc_swizzle;
return 0;
case MSM_PARAM_MACROTILE_MODE:
- *value = adreno_gpu->ubwc_config->macrotile_mode;
+ if (!adreno_gpu->ubwc_config)
+ *value = 0;
+ else
+ *value = adreno_gpu->ubwc_config->macrotile_mode;
return 0;
case MSM_PARAM_UCHE_TRAP_BASE:
*value = adreno_gpu->uche_trap_base;
---
base-commit: 36ece9697e89016181e5ae87510e40fb31d86f2b
change-id: 20260407-adreno-fix-ubwc-6a2564710e21
Best regards,
--
With best wishes
Dmitry
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
2026-04-06 22:14 [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx Dmitry Baryshkov
@ 2026-04-06 22:24 ` Rob Clark
2026-04-06 23:43 ` Dmitry Baryshkov
2026-04-12 4:17 ` Claude review: " Claude Code Review Bot
2026-04-12 4:17 ` Claude Code Review Bot
2 siblings, 1 reply; 5+ messages in thread
From: Rob Clark @ 2026-04-06 22:24 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Sean Paul, Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
Simona Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel
On Mon, Apr 6, 2026 at 3:14 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> Before a5xx Adreno driver will not try fetching UBWC params (because
> those generations didn't support UBWC anyway), however it's still
> possible to query UBWC-related params from the userspace, triggering
> possible NULL pointer dereference. Check for UBWC config in
> adreno_get_param() and return sane defaults if there is none.
>
> Fixes: a452510aad53 ("drm/msm/adreno: Switch to the common UBWC config struct")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index d5fe6f6f0dec..7dc95c0a17f7 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -426,16 +426,25 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
> *value = vm->mm_range;
> return 0;
> case MSM_PARAM_HIGHEST_BANK_BIT:
> - *value = adreno_gpu->ubwc_config->highest_bank_bit;
> + if (!adreno_gpu->ubwc_config)
> + *value = 13;
> + else
> + *value = adreno_gpu->ubwc_config->highest_bank_bit;
These three params are only used by userspace on a6xx, so tbh it would
be ok to return -ENOENT for platforms that have no ubwc config. That
might be better than returning imaginary values.
BR,
-R
> return 0;
> case MSM_PARAM_RAYTRACING:
> *value = adreno_gpu->has_ray_tracing;
> return 0;
> case MSM_PARAM_UBWC_SWIZZLE:
> - *value = adreno_gpu->ubwc_config->ubwc_swizzle;
> + if (!adreno_gpu->ubwc_config)
> + *value = 0;
> + else
> + *value = adreno_gpu->ubwc_config->ubwc_swizzle;
> return 0;
> case MSM_PARAM_MACROTILE_MODE:
> - *value = adreno_gpu->ubwc_config->macrotile_mode;
> + if (!adreno_gpu->ubwc_config)
> + *value = 0;
> + else
> + *value = adreno_gpu->ubwc_config->macrotile_mode;
> return 0;
> case MSM_PARAM_UCHE_TRAP_BASE:
> *value = adreno_gpu->uche_trap_base;
>
> ---
> base-commit: 36ece9697e89016181e5ae87510e40fb31d86f2b
> change-id: 20260407-adreno-fix-ubwc-6a2564710e21
>
> Best regards,
> --
> With best wishes
> Dmitry
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
2026-04-06 22:24 ` Rob Clark
@ 2026-04-06 23:43 ` Dmitry Baryshkov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Baryshkov @ 2026-04-06 23:43 UTC (permalink / raw)
To: Rob Clark
Cc: Sean Paul, Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
Simona Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel
On Mon, Apr 06, 2026 at 03:24:43PM -0700, Rob Clark wrote:
> On Mon, Apr 6, 2026 at 3:14 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >
> > Before a5xx Adreno driver will not try fetching UBWC params (because
> > those generations didn't support UBWC anyway), however it's still
> > possible to query UBWC-related params from the userspace, triggering
> > possible NULL pointer dereference. Check for UBWC config in
> > adreno_get_param() and return sane defaults if there is none.
> >
> > Fixes: a452510aad53 ("drm/msm/adreno: Switch to the common UBWC config struct")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index d5fe6f6f0dec..7dc95c0a17f7 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -426,16 +426,25 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
> > *value = vm->mm_range;
> > return 0;
> > case MSM_PARAM_HIGHEST_BANK_BIT:
> > - *value = adreno_gpu->ubwc_config->highest_bank_bit;
> > + if (!adreno_gpu->ubwc_config)
> > + *value = 13;
> > + else
> > + *value = adreno_gpu->ubwc_config->highest_bank_bit;
>
> These three params are only used by userspace on a6xx, so tbh it would
> be ok to return -ENOENT for platforms that have no ubwc config. That
> might be better than returning imaginary values.
I'd say, those are defaults. But I agree, -ENOENT might be better.
>
> BR,
> -R
>
> > return 0;
> > case MSM_PARAM_RAYTRACING:
> > *value = adreno_gpu->has_ray_tracing;
> > return 0;
> > case MSM_PARAM_UBWC_SWIZZLE:
> > - *value = adreno_gpu->ubwc_config->ubwc_swizzle;
> > + if (!adreno_gpu->ubwc_config)
> > + *value = 0;
> > + else
> > + *value = adreno_gpu->ubwc_config->ubwc_swizzle;
> > return 0;
> > case MSM_PARAM_MACROTILE_MODE:
> > - *value = adreno_gpu->ubwc_config->macrotile_mode;
> > + if (!adreno_gpu->ubwc_config)
> > + *value = 0;
> > + else
> > + *value = adreno_gpu->ubwc_config->macrotile_mode;
> > return 0;
> > case MSM_PARAM_UCHE_TRAP_BASE:
> > *value = adreno_gpu->uche_trap_base;
> >
> > ---
> > base-commit: 36ece9697e89016181e5ae87510e40fb31d86f2b
> > change-id: 20260407-adreno-fix-ubwc-6a2564710e21
> >
> > Best regards,
> > --
> > With best wishes
> > Dmitry
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
2026-04-06 22:14 [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx Dmitry Baryshkov
2026-04-06 22:24 ` Rob Clark
@ 2026-04-12 4:17 ` Claude Code Review Bot
2026-04-12 4:17 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 4:17 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Patches: 3
Reviewed: 2026-04-12T14:17:19.494675
---
This is a single patch fixing a real NULL pointer dereference bug in the Adreno GPU driver. The problem is genuine: the commit `a452510aad53` ("drm/msm/adreno: Switch to the common UBWC config struct") introduced unconditional dereferences of `adreno_gpu->ubwc_config` in `adreno_get_param()`, but on a2xx through a4xx hardware, `ubwc_config` is never initialized (confirmed: none of a2xx_gpu.c, a3xx_gpu.c, or a4xx_gpu.c ever set `ubwc_config`). Any userspace process can trigger this crash by issuing an `MSM_PARAM_HIGHEST_BANK_BIT`, `MSM_PARAM_UBWC_SWIZZLE`, or `MSM_PARAM_MACROTILE_MODE` ioctl query on those older GPUs.
The fix is correct in that it prevents the crash, but the approach is debatable.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx
2026-04-06 22:14 [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx Dmitry Baryshkov
2026-04-06 22:24 ` Rob Clark
2026-04-12 4:17 ` Claude review: " Claude Code Review Bot
@ 2026-04-12 4:17 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 4:17 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-12 4:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-06 22:14 [PATCH] drm/msm/adreno: fix userspace-triggered crash on a2xx-a4xx Dmitry Baryshkov
2026-04-06 22:24 ` Rob Clark
2026-04-06 23:43 ` Dmitry Baryshkov
2026-04-12 4:17 ` Claude review: " Claude Code Review Bot
2026-04-12 4:17 ` 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