<div dir="ltr">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.<div><br></div><div>backlight: pcf50633: pdata may be a null pointer, null pointer dereference causes crash</div><div>pdata has been checked at line 120 before dereference. However, it is used without check at line 130. So just add the check,</div><div><br></div><div> Signed-off-by: Wenjia Zhao <<a href="mailto:driverfuzzing@gmail.com" target="_blank">driverfuzzing@gmail.com</a>><br>---<br> drivers/video/backlight/pcf50633-backlight.c | 3 ++-<br> 1 file changed, 2 insertions(+), 1 deletion(-)<br><br>diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c<br>index 540dd338..43267af 100644<br>--- a/drivers/video/backlight/pcf50633-backlight.c<br>+++ b/drivers/video/backlight/pcf50633-backlight.c<br>@@ -127,7 +127,8 @@ static int pcf50633_bl_probe(struct platform_device *pdev)<br><br> platform_set_drvdata(pdev, pcf_bl);<br><br>- pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);<br>+ if (pdata)<br>+ pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);<br><br> /*<br> * Should be different from bl_props.brightness, so we do not exit<font color="#888888"><br>--</font><br></div><div><br></div><div> 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.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Daniel Thompson <<a href="mailto:daniel.thompson@linaro.org" target="_blank">daniel.thompson@linaro.org</a>> 于2021年2月2日周二 上午3:25写道:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Feb 01, 2021 at 08:41:38AM -0600, Wenjia Zhao wrote:<br>
> Signed-off-by: Wenjia Zhao <<a href="mailto:driverfuzzing@gmail.com" target="_blank">driverfuzzing@gmail.com</a>><br>
<br>
There should be a patch description here explaining why the patch<br>
is needed and how it works.<br>
<br>
<br>
> ---<br>
> drivers/video/backlight/pcf50633-backlight.c | 3 ++-<br>
> 1 file changed, 2 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c<br>
> index 540dd338..43267af 100644<br>
> --- a/drivers/video/backlight/pcf50633-backlight.c<br>
> +++ b/drivers/video/backlight/pcf50633-backlight.c<br>
> @@ -127,7 +127,8 @@ static int pcf50633_bl_probe(struct platform_device *pdev)<br>
> <br>
> platform_set_drvdata(pdev, pcf_bl);<br>
> <br>
> - pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);<br>
> + if (pdata)<br>
> + pcf50633_reg_write(pcf_bl->pcf, PCF50633_REG_LEDDIM, pdata->ramp_time);<br>
<br>
Assuming you found this issue using a static analyzer then I think it<br>
might be better to if an "if (!pdata) return -EINVAL" further up the<br>
file instead.<br>
<br>
In other words it is better to "document" (via the return code) that the<br>
code does not support pdata == NULL than to add another untested code<br>
path.<br>
<br>
<br>
Daniel.<br>
</blockquote></div></div>