Re: [PATCH 2/2] drm/msm/a6xx: Fix NULL dereference during crashstate capture

[Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On Mon, Dec 10, 2018 at 05:34:22PM +0530, Sharat Masetty wrote:
> The gpu crashstate's base objects registers pointer can be NULL if the
> target implementation decides to capture the register dump on its own.
> This patch simply checks for NULL before dereferencing.
> 
> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 40bcf32..a39cebc 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -415,6 +415,9 @@ void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
>  		}
>  	}
>  
> +	if (!adreno_gpu->registers)
> +		return;
> +

This looks good - we should get it in the 4.21 pull.

>  	/* Count the number of registers */
>  	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
>  		count += adreno_gpu->registers[i + 1] -
> @@ -550,12 +553,14 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
>  		}
>  	}
>  
> -	drm_puts(p, "registers:\n");
> +	if (state->nr_registers > 0) {
> +		drm_puts(p, "registers:\n");
>  
> -	for (i = 0; i < state->nr_registers; i++) {
> -		drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
> -			state->registers[i * 2] << 2,
> -			state->registers[(i * 2) + 1]);
> +		for (i = 0; i < state->nr_registers; i++) {
> +			drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
> +					state->registers[i * 2] << 2,
> +					state->registers[(i * 2) + 1]);
> +		}

I don't think we need the extra indentation here - something like

for (i = 0; i < state->nr_registers; i++) {
+	if (i == 0)
+		drm_puts(p, "Registers:\n");
	drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n",

would suffice since we won't go into the loop if state->nr_registers == 0.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno