* [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
@ 2025-03-17 10:03 Qasim Ijaz
0 siblings, 0 replies; 6+ messages in thread
From: Qasim Ijaz @ 2025-03-17 10:03 UTC (permalink / raw)
To: robdclark, quic_abhinavk, dmitry.baryshkov, sean, marijn.suijten,
airlied, simona, quic_jesszhan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
_dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
before the sanity checks which can lead to a NULL pointer dereference if
phys is NULL.
Fix this by reordering the dereference after the sanity checks.
Fixes: 8144d17a81d9 ("drm/msm/dpu: Skip trigger flush and start for CWB")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
v2:
- Moved Signed-off tag below Fixes tag
- Moved dpu_enc declaration to the top and initialisation below sanity checks
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0eed93a4d056..0bd1f2bfaaff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1667,7 +1667,7 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
*/
static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(phys->parent);
+ struct dpu_encoder_virt *dpu_enc;
if (!phys) {
DPU_ERROR("invalid argument(s)\n");
@@ -1678,6 +1678,8 @@ static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
DPU_ERROR("invalid pingpong hw\n");
return;
}
+
+ dpu_enc = to_dpu_encoder_virt(phys->parent);
if (phys->parent->encoder_type == DRM_MODE_ENCODER_VIRTUAL &&
dpu_enc->cwb_mask) {
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
@ 2025-03-29 11:35 Qasim Ijaz
2025-03-30 9:06 ` Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Qasim Ijaz @ 2025-03-29 11:35 UTC (permalink / raw)
To: robdclark, quic_abhinavk, lumag, sean, marijn.suijten, airlied,
simona, quic_jesszhan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Dmitry Baryshkov
_dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
before the sanity checks which can lead to a NULL pointer dereference if
phys is NULL.
Fix this by reordering the dereference after the sanity checks.
Fixes: 8144d17a81d9 ("drm/msm/dpu: Skip trigger flush and start for CWB")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
v2:
- Moved Signed-off tag below Fixes tag
- Moved dpu_enc declaration to the top and initialisation below sanity checks
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0eed93a4d056..0bd1f2bfaaff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1667,7 +1667,7 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
*/
static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(phys->parent);
+ struct dpu_encoder_virt *dpu_enc;
if (!phys) {
DPU_ERROR("invalid argument(s)\n");
@@ -1678,6 +1678,8 @@ static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
DPU_ERROR("invalid pingpong hw\n");
return;
}
+
+ dpu_enc = to_dpu_encoder_virt(phys->parent);
if (phys->parent->encoder_type == DRM_MODE_ENCODER_VIRTUAL &&
dpu_enc->cwb_mask) {
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
2025-03-29 11:35 Qasim Ijaz
@ 2025-03-30 9:06 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2025-03-30 9:06 UTC (permalink / raw)
To: Qasim Ijaz, freedreno, dri-devel, linux-arm-msm, Abhinav Kumar,
David Airlie, Dmitry Baryshkov, Jessica Zhang, Marijn Suijten,
Rob Clark, Sean Paul, Simona Vetter
Cc: LKML, Dmitry Baryshkov
…
> Fix this by reordering the dereference after the sanity checks.
Can my previous patch review contribution trigger more desirable collateral evolution
also for this development topic?
https://lore.kernel.org/all/6f01f71b-284b-4841-bda9-a3934cb4efc8@web.de/
https://lkml.org/lkml/2025/3/10/696
Is any automated source code analysis involved here?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
@ 2025-04-08 17:22 Qasim Ijaz
2025-04-08 17:28 ` Jessica Zhang
2025-04-08 18:21 ` Abhinav Kumar
0 siblings, 2 replies; 6+ messages in thread
From: Qasim Ijaz @ 2025-04-08 17:22 UTC (permalink / raw)
To: robdclark, quic_abhinavk, lumag, sean, marijn.suijten, airlied,
simona, quic_jesszhan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Dmitry Baryshkov
_dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
before the sanity checks which can lead to a NULL pointer dereference if
phys is NULL.
Fix this by reordering the dereference after the sanity checks.
Fixes: 8144d17a81d9 ("drm/msm/dpu: Skip trigger flush and start for CWB")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
v2:
- Moved Signed-off tag below Fixes tag
- Moved dpu_enc declaration to the top and initialisation below sanity checks
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0eed93a4d056..0bd1f2bfaaff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1667,7 +1667,7 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
*/
static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(phys->parent);
+ struct dpu_encoder_virt *dpu_enc;
if (!phys) {
DPU_ERROR("invalid argument(s)\n");
@@ -1678,6 +1678,8 @@ static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
DPU_ERROR("invalid pingpong hw\n");
return;
}
+
+ dpu_enc = to_dpu_encoder_virt(phys->parent);
if (phys->parent->encoder_type == DRM_MODE_ENCODER_VIRTUAL &&
dpu_enc->cwb_mask) {
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
2025-04-08 17:22 Qasim Ijaz
@ 2025-04-08 17:28 ` Jessica Zhang
2025-04-08 18:21 ` Abhinav Kumar
1 sibling, 0 replies; 6+ messages in thread
From: Jessica Zhang @ 2025-04-08 17:28 UTC (permalink / raw)
To: Qasim Ijaz, robdclark, quic_abhinavk, lumag, sean, marijn.suijten,
airlied, simona
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Dmitry Baryshkov
On 4/8/2025 10:22 AM, Qasim Ijaz wrote:
> _dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
> before the sanity checks which can lead to a NULL pointer dereference if
> phys is NULL.
>
> Fix this by reordering the dereference after the sanity checks.
>
> Fixes: 8144d17a81d9 ("drm/msm/dpu: Skip trigger flush and start for CWB")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> v2:
> - Moved Signed-off tag below Fixes tag
> - Moved dpu_enc declaration to the top and initialisation below sanity checks
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0eed93a4d056..0bd1f2bfaaff 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1667,7 +1667,7 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
> */
> static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
> {
> - struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(phys->parent);
> + struct dpu_encoder_virt *dpu_enc;
>
> if (!phys) {
> DPU_ERROR("invalid argument(s)\n");
> @@ -1678,6 +1678,8 @@ static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
> DPU_ERROR("invalid pingpong hw\n");
> return;
> }
> +
> + dpu_enc = to_dpu_encoder_virt(phys->parent);
>
> if (phys->parent->encoder_type == DRM_MODE_ENCODER_VIRTUAL &&
> dpu_enc->cwb_mask) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
2025-04-08 17:22 Qasim Ijaz
2025-04-08 17:28 ` Jessica Zhang
@ 2025-04-08 18:21 ` Abhinav Kumar
1 sibling, 0 replies; 6+ messages in thread
From: Abhinav Kumar @ 2025-04-08 18:21 UTC (permalink / raw)
To: Qasim Ijaz, robdclark, lumag, sean, marijn.suijten, airlied,
simona, quic_jesszhan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Dmitry Baryshkov
On 4/8/2025 10:22 AM, Qasim Ijaz wrote:
> _dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
> before the sanity checks which can lead to a NULL pointer dereference if
> phys is NULL.
>
> Fix this by reordering the dereference after the sanity checks.
>
> Fixes: 8144d17a81d9 ("drm/msm/dpu: Skip trigger flush and start for CWB")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> ---
> v2:
> - Moved Signed-off tag below Fixes tag
> - Moved dpu_enc declaration to the top and initialisation below sanity checks
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-08 18:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-17 10:03 [PATCH v2 RESEND] drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref Qasim Ijaz
-- strict thread matches above, loose matches on Subject: below --
2025-03-29 11:35 Qasim Ijaz
2025-03-30 9:06 ` Markus Elfring
2025-04-08 17:22 Qasim Ijaz
2025-04-08 17:28 ` Jessica Zhang
2025-04-08 18:21 ` Abhinav Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox