Re: [PATCH RFC 13/13] drm/msm: add lpac ring to devcoredump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked error pointer causes kernel panic during devcoredump
--

commit d796b03f3d2b7a2a3e2dcafef659304fe3dd3b3e
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>

drm/msm: add lpac ring to devcoredump

This commit adds the LPAC ring state to the devcoredump to aid in debugging
GPU issues. It updates the loop conditions in the adreno GPU state functions
to include the LPAC ringbuffer data if it is present on the device.

> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 94dbec9464a4d..030bb1aeb7ee1 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -782,25 +782,26 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
>  
>  	ktime_get_real_ts64(&state->time);
>  
> -	for (i = 0; i < gpu->nr_rings; i++) {
> +	for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
> +		struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;

[Severity: High]
This isn't a bug introduced by this patch, but this devcoredump modification
exposes a pre-existing unhandled error pointer.

In msm_gpu_init(), if the allocation of gpu->lpac_rb fails (e.g., due to memory
pressure), it returns an error pointer. Due to a pre-existing bug, the error
check incorrectly evaluates IS_ERR(gpu->rb[i]) instead of IS_ERR(gpu->lpac_rb):

drivers/gpu/drm/msm/msm_gpu.c:msm_gpu_init() {
    ...
	if (IS_ERR(gpu->rb[i])) {
    ...
}

Since an error pointer evaluates to true for !!gpu->lpac_rb, will this loop
include the error pointer and cause a kernel panic when it attempts to
dereference ring->memptrs->fence below?

>  		int size = 0, j;
>  
> -		state->ring[i].fence = gpu->rb[i]->memptrs->fence;
> -		state->ring[i].iova = gpu->rb[i]->iova;
> -		state->ring[i].seqno = gpu->rb[i]->fctx->last_fence;
> -		state->ring[i].rptr = get_rptr(adreno_gpu, gpu->rb[i]);
> -		state->ring[i].wptr = get_wptr(gpu->rb[i]);
> +		state->ring[i].fence = ring->memptrs->fence;
> +		state->ring[i].iova = ring->iova;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705-descriptive-name-lpac-upstream-v1-0-01d50c3e0c99@gmail.com?part=13