| Message ID | 20230821062159.198691-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ast: Avoid possible NULL dereference | expand |
Le 21/08/2023 à 08:22, Su Hui a écrit : > smatch error: > drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error: > we previously assumed 'ast->dp501_fw' could be null (see line 223) > > when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true, > there will be a NULL dereference of 'ast->dp501_fw'. > > Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when required") > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > drivers/gpu/drm/ast/ast_dp501.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c > index 1bc35a992369..d9f3a0786a6f 100644 > --- a/drivers/gpu/drm/ast/ast_dp501.c > +++ b/drivers/gpu/drm/ast/ast_dp501.c > @@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev) > ast_load_dp501_microcode(dev) < 0) > return false; > > - fw_addr = (u8 *)ast->dp501_fw->data; > - len = ast->dp501_fw->size; > + if (ast->dp501_fw) { > + fw_addr = (u8 *)ast->dp501_fw->data; > + len = ast->dp501_fw->size; > + } > } > /* Get BootAddress */ > ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8); Hi, this looks like a false positive. If "!ast->dp501_fw", and ast_load_dp501_microcode()>=0, then ast_load_dp501_microcode() will set a valid value in ast->dp501_fw. CJ
On 2023/8/21 15:04, Christophe JAILLET wrote: > Le 21/08/2023 à 08:22, Su Hui a écrit : >> smatch error: >> drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error: >> we previously assumed 'ast->dp501_fw' could be null (see line 223) >> >> when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true, >> there will be a NULL dereference of 'ast->dp501_fw'. >> >> Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when >> required") >> Signed-off-by: Su Hui <suhui@nfschina.com> >> --- >> drivers/gpu/drm/ast/ast_dp501.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/ast/ast_dp501.c >> b/drivers/gpu/drm/ast/ast_dp501.c >> index 1bc35a992369..d9f3a0786a6f 100644 >> --- a/drivers/gpu/drm/ast/ast_dp501.c >> +++ b/drivers/gpu/drm/ast/ast_dp501.c >> @@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev) >> ast_load_dp501_microcode(dev) < 0) >> return false; >> - fw_addr = (u8 *)ast->dp501_fw->data; >> - len = ast->dp501_fw->size; >> + if (ast->dp501_fw) { >> + fw_addr = (u8 *)ast->dp501_fw->data; >> + len = ast->dp501_fw->size; >> + } >> } >> /* Get BootAddress */ >> ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8); > > Hi, > > this looks like a false positive. > > If "!ast->dp501_fw", and ast_load_dp501_microcode()>=0, then > ast_load_dp501_microcode() will set a valid value in ast->dp501_fw. Hi, Sorry for the noise, you are right, this is a false positive. I will take more careful next time. Su Hui > > CJ
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c index 1bc35a992369..d9f3a0786a6f 100644 --- a/drivers/gpu/drm/ast/ast_dp501.c +++ b/drivers/gpu/drm/ast/ast_dp501.c @@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev) ast_load_dp501_microcode(dev) < 0) return false; - fw_addr = (u8 *)ast->dp501_fw->data; - len = ast->dp501_fw->size; + if (ast->dp501_fw) { + fw_addr = (u8 *)ast->dp501_fw->data; + len = ast->dp501_fw->size; + } } /* Get BootAddress */ ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
smatch error: drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error: we previously assumed 'ast->dp501_fw' could be null (see line 223) when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true, there will be a NULL dereference of 'ast->dp501_fw'. Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when required") Signed-off-by: Su Hui <suhui@nfschina.com> --- drivers/gpu/drm/ast/ast_dp501.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)