diff mbox series

drm/msm/dpu: Fix pointer dereferenced before checking

Message ID 1653896005-25168-1-git-send-email-baihaowen@meizu.com (mailing list archive)
State Not Applicable
Headers show
Series drm/msm/dpu: Fix pointer dereferenced before checking | expand

Commit Message

baihaowen May 30, 2022, 7:33 a.m. UTC
The ctx->hw is dereferencing before null checking, so move
it after checking.

Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Dmitry Baryshkov May 30, 2022, 9:49 p.m. UTC | #1
A nit: patchwork thinks that two patches from the same author with the
same subject are two versions of the same patch. In future, could you
please send such patches with distinct names? No need to send v2 now
unless review shows other issues with the patches.

On Mon, 30 May 2022 at 10:33, Haowen Bai <baihaowen@meizu.com> wrote:
>
> The ctx->hw is dereferencing before null checking, so move
> it after checking.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Abhinav Kumar May 31, 2022, 12:36 a.m. UTC | #2
On 5/30/2022 12:33 AM, Haowen Bai wrote:
> The ctx->hw is dereferencing before null checking, so move
> it after checking.
> 
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>

Agree with Dmitry's comment. Adjust the patch subject to a different one 
otherwise PW thinks they are same patches.

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> index bcccce292937..e59680cdd0ce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> @@ -155,11 +155,13 @@ static void dpu_hw_wb_roi(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *wb)
>   static void dpu_hw_wb_setup_qos_lut(struct dpu_hw_wb *ctx,
>   		struct dpu_hw_wb_qos_cfg *cfg)
>   {
> -	struct dpu_hw_blk_reg_map *c = &ctx->hw;
> +	struct dpu_hw_blk_reg_map *c;
>   	u32 qos_ctrl = 0;
>   
>   	if (!ctx || !cfg)
>   		return;
> +	
> +	c = &ctx->hw;
>   
>   	DPU_REG_WRITE(c, WB_DANGER_LUT, cfg->danger_lut);
>   	DPU_REG_WRITE(c, WB_SAFE_LUT, cfg->safe_lut);
baihaowen May 31, 2022, 1:34 a.m. UTC | #3
在 2022/5/30 下午3:33, Haowen Bai 写道:
> The ctx->hw is dereferencing before null checking, so move
> it after checking.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> index bcccce292937..e59680cdd0ce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> @@ -155,11 +155,13 @@ static void dpu_hw_wb_roi(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *wb)
>  static void dpu_hw_wb_setup_qos_lut(struct dpu_hw_wb *ctx,
>  		struct dpu_hw_wb_qos_cfg *cfg)
>  {
> -	struct dpu_hw_blk_reg_map *c = &ctx->hw;
> +	struct dpu_hw_blk_reg_map *c;
>  	u32 qos_ctrl = 0;
>  
>  	if (!ctx || !cfg)
>  		return;
> +	
> +	c = &ctx->hw;
>  
>  	DPU_REG_WRITE(c, WB_DANGER_LUT, cfg->danger_lut);
>  	DPU_REG_WRITE(c, WB_SAFE_LUT, cfg->safe_lut);
Sorry, plz ignore this patch.

ctx->hw is dereferenced, &ctx->hw is just a pointer math for pointer address offset, so it would not cause a bug(dereferencing null pointer).
baihaowen May 31, 2022, 1:37 a.m. UTC | #4
在 2022/5/31 上午8:36, Abhinav Kumar 写道:
>
>
> On 5/30/2022 12:33 AM, Haowen Bai wrote:
>> The ctx->hw is dereferencing before null checking, so move
>> it after checking.
>>
>> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
>
> Agree with Dmitry's comment. Adjust the patch subject to a different one otherwise PW thinks they are same patches.
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> index bcccce292937..e59680cdd0ce 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> @@ -155,11 +155,13 @@ static void dpu_hw_wb_roi(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *wb)
>>   static void dpu_hw_wb_setup_qos_lut(struct dpu_hw_wb *ctx,
>>           struct dpu_hw_wb_qos_cfg *cfg)
>>   {
>> -    struct dpu_hw_blk_reg_map *c = &ctx->hw;
>> +    struct dpu_hw_blk_reg_map *c;
>>       u32 qos_ctrl = 0;
>>         if (!ctx || !cfg)
>>           return;
>> +   
>> +    c = &ctx->hw;
>>         DPU_REG_WRITE(c, WB_DANGER_LUT, cfg->danger_lut);
>>       DPU_REG_WRITE(c, WB_SAFE_LUT, cfg->safe_lut);
Sorry, plz ignore this patch.

ctx->hw is dereferenced, &ctx->hw is just a pointer math for pointer address offset, so it would not cause a bug(dereferencing null pointer).
Rob Clark May 31, 2022, 8:34 p.m. UTC | #5
On Mon, May 30, 2022 at 12:34 AM Haowen Bai <baihaowen@meizu.com> wrote:
>
> The ctx->hw is dereferencing before null checking, so move
> it after checking.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> index bcccce292937..e59680cdd0ce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> @@ -155,11 +155,13 @@ static void dpu_hw_wb_roi(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *wb)
>  static void dpu_hw_wb_setup_qos_lut(struct dpu_hw_wb *ctx,
>                 struct dpu_hw_wb_qos_cfg *cfg)
>  {
> -       struct dpu_hw_blk_reg_map *c = &ctx->hw;
> +       struct dpu_hw_blk_reg_map *c;
>         u32 qos_ctrl = 0;
>
>         if (!ctx || !cfg)
>                 return;
> +
> +       c = &ctx->hw;

tbh, we should just drop both of these null checks.. there is no
codepath that can reach this with potential for either param to be
NULL

BR,
-R

>
>         DPU_REG_WRITE(c, WB_DANGER_LUT, cfg->danger_lut);
>         DPU_REG_WRITE(c, WB_SAFE_LUT, cfg->safe_lut);
> --
> 2.7.4
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
index bcccce292937..e59680cdd0ce 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -155,11 +155,13 @@  static void dpu_hw_wb_roi(struct dpu_hw_wb *ctx, struct dpu_hw_wb_cfg *wb)
 static void dpu_hw_wb_setup_qos_lut(struct dpu_hw_wb *ctx,
 		struct dpu_hw_wb_qos_cfg *cfg)
 {
-	struct dpu_hw_blk_reg_map *c = &ctx->hw;
+	struct dpu_hw_blk_reg_map *c;
 	u32 qos_ctrl = 0;
 
 	if (!ctx || !cfg)
 		return;
+	
+	c = &ctx->hw;
 
 	DPU_REG_WRITE(c, WB_DANGER_LUT, cfg->danger_lut);
 	DPU_REG_WRITE(c, WB_SAFE_LUT, cfg->safe_lut);