Sorry about the missing description, I have a description at my local commit. But the commit description disappeared when I used git send-email to submit this patch.
backlight: pcf50633: pdata may be a null pointer, null pointer dereference causes crash
pdata has been checked at line 120 before dereference. However, it is used without check at line 130. So just add the check,
Signed-off-by: Wenjia Zhao <driverfuzzing@xxxxxxxxx>
---
drivers/video/backlight/pcf50633-backlight.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
index 540dd338..43267af 100644
--- a/drivers/video/backlight/pcf50633-backlight.c
+++ b/drivers/video/backlight/pcf50633-backlight.c
@@ -127,7 +127,8 @@ static int pcf50633_bl_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcf_bl);
- pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
+ if (pdata)
+ pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
/*
* Should be different from bl_props.brightness, so we do not exit
--
---
drivers/video/backlight/pcf50633-backlight.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
index 540dd338..43267af 100644
--- a/drivers/video/backlight/pcf50633-backlight.c
+++ b/drivers/video/backlight/pcf50633-backlight.c
@@ -127,7 +127,8 @@ static int pcf50633_bl_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcf_bl);
- pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
+ if (pdata)
+ pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
/*
* Should be different from bl_props.brightness, so we do not exit
--
It is better to write a default value to the register if the ramp_time has a default value. Then it does not need to return -EINVAL. It will keep consistent with the behavior at line 120.
Daniel Thompson <daniel.thompson@xxxxxxxxxx> 于2021年2月2日周二 上午3:25写道：
On Mon, Feb 01, 2021 at 08:41:38AM -0600, Wenjia Zhao wrote:
> Signed-off-by: Wenjia Zhao <driverfuzzing@xxxxxxxxx>
There should be a patch description here explaining why the patch
is needed and how it works.
> ---
> drivers/video/backlight/pcf50633-backlight.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
> index 540dd338..43267af 100644
> --- a/drivers/video/backlight/pcf50633-backlight.c
> +++ b/drivers/video/backlight/pcf50633-backlight.c
> @@ -127,7 +127,8 @@ static int pcf50633_bl_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, pcf_bl);
>
> - pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
> + if (pdata)
> + pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);
Assuming you found this issue using a static analyzer then I think it
might be better to if an "if (!pdata) return -EINVAL" further up the
file instead.
In other words it is better to "document" (via the return code) that the
code does not support pdata == NULL than to add another untested code
path.
Daniel.
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
