[Freedreno] [PATCH v2 1/1] drm/msm/a6xx: Fix null pointer access in a6xx_get_indexed_registers

[Rob Clark]

On Mon, Jul 25, 2022 at 9:46 PM <quic_ddhamara at quicinc.com> wrote:
>
> From: Akhil P Oommen <quic_akhilpo at quicinc.com>
>
> Fix a null pointer access when memory allocation fails in
> a6xx_get_indexed_registers().
>
> Change-Id: I33e13745cd8e5841d2f377f48a199af98be2ed02
> Signed-off-by: Akhil P Oommen <quic_akhilpo at quicinc.com>
> Signed-off-by: Devi prasad Dhamarasingi <quic_ddhamara at quicinc.com>
> ---
>
> Changes in v2:
> - Corrected the signoff name and email id.
>
>  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index 55f443328d8e..507074f6222c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -952,6 +952,12 @@ static void a6xx_get_indexed_registers(struct msm_gpu *gpu,
>         a6xx_get_indexed_regs(gpu, a6xx_state, &a6xx_cp_mempool_indexed,
>                 &a6xx_state->indexed_regs[i]);
>
> +       if (!a6xx_state->indexed_regs[i].data) {
> +               gpu_write(gpu, REG_A6XX_CP_MEM_POOL_SIZE, mempool_size);
> +               a6xx_state->nr_indexed_regs = count - 1;
> +               return;
> +       }

Hmm, I don't see us adjusting nr_indexed_regs if any of the earlier
sections fails, so I don't think we need to do that here either.  So I
think you could just:

if (a6xx_state->indexed_regs[i].data)
   a6xx_state->indexed_regs[i].data[0x2000] = mempool_size;

And I kinda expect if there was an allocation failure we'd just end up
dereferencing a null ptr later in the show path.

But, I think in general you can assume small GFP_KERNEL allocations
will never fail.  If necessary they will block for reclaim/shrinker to
free up some memory or evict some pages to swap.  If you've gotten to
the point where even that isn't possible, then a null ptr deref is
really the least of your problems ;-)

BR,
-R

> +
>         /*
>          * Offset 0x2000 in the mempool is the size - copy the saved size over
>          * so the data is consistent
> --
> 2.37.0
>