On Mon, Jul 25, 2022 at 9:46 PM <quic_ddhamara@xxxxxxxxxxx> wrote:
>
> From: Akhil P Oommen <quic_akhilpo@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Devi prasad Dhamarasingi <quic_ddhamara@xxxxxxxxxxx>
> ---
>
> 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
>
