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

[Sharat Masetty]

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 <***@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;
+
/* 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]);
+ }
}
}
#endif

[Jordan Crouse]

Post by Sharat Masetty
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.
---
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.

Post by Sharat Masetty
/* 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

[Jordan Crouse]

We are not really checking the state of the adreno_gpu_state_get()
function at the callers and in addition the state capture is mostly a
best effort service, so make the function return void.
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +---
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 1ca4bea..40bcf32 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -380,7 +380,7 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
return false;
}
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
int i, count = 0;
@@ -437,8 +437,6 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
state->nr_registers = count;
}
-
- return 0;
}
void adreno_gpu_state_destroy(struct msm_gpu_state *state)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 4973c8c..d4834b3 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -235,7 +235,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
void adreno_gpu_state_destroy(struct msm_gpu_state *state);
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
int adreno_gpu_state_put(struct msm_gpu_state *state);
/* ringbuffer helpers (the parts that are adreno specific) */
--
1.9.1