| Message ID | 1541410354-19090-5-git-send-email-wni@nvidia.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Fixes for Tegra soctherm | expand |
On Mon, Nov 05, 2018 at 05:32:34PM +0800, Wei Ni wrote: > Fix dereference dev before null check. > > Signed-off-by: Wei Ni <wni@nvidia.com> > --- > drivers/thermal/tegra/soctherm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c > index 3042837364e8..96527df91f2a 100644 > --- a/drivers/thermal/tegra/soctherm.c > +++ b/drivers/thermal/tegra/soctherm.c > @@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev, > struct soctherm_throt_cfg *stc, > int trip_temp) > { > - struct tegra_soctherm *ts = dev_get_drvdata(dev); > + struct tegra_soctherm *ts; > int temp, cpu_throt, gpu_throt; > unsigned int throt; > u32 r, reg_off; > @@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev, > if (!sg || !stc || !stc->init) > return -EINVAL; > > + ts = dev_get_drvdata(dev); I think coverity is wrong. How is dev ever going to be NULL in this case? We allocate all of these struct tegra_thermctl_zone structures in tegra_soctherm_probe() and assign zone->dev = &pdev->dev, which can never be NULL. And even if it could, the code would've crashed earlier in tegra_soctherm_probe() already. Furthermore, I fail to see how your patch would fix the defect. None of the checks in the conditional above actually check the dev value. Thierry
On 8/11/2018 8:37 PM, Thierry Reding wrote: > On Mon, Nov 05, 2018 at 05:32:34PM +0800, Wei Ni wrote: >> Fix dereference dev before null check. >> >> Signed-off-by: Wei Ni <wni@nvidia.com> >> --- >> drivers/thermal/tegra/soctherm.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c >> index 3042837364e8..96527df91f2a 100644 >> --- a/drivers/thermal/tegra/soctherm.c >> +++ b/drivers/thermal/tegra/soctherm.c >> @@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev, >> struct soctherm_throt_cfg *stc, >> int trip_temp) >> { >> - struct tegra_soctherm *ts = dev_get_drvdata(dev); >> + struct tegra_soctherm *ts; >> int temp, cpu_throt, gpu_throt; >> unsigned int throt; >> u32 r, reg_off; >> @@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev, >> if (!sg || !stc || !stc->init) >> return -EINVAL; >> >> + ts = dev_get_drvdata(dev); > > I think coverity is wrong. How is dev ever going to be NULL in this > case? We allocate all of these struct tegra_thermctl_zone structures in > tegra_soctherm_probe() and assign zone->dev = &pdev->dev, which can > never be NULL. > > And even if it could, the code would've crashed earlier in > tegra_soctherm_probe() already. > > Furthermore, I fail to see how your patch would fix the defect. None of > the checks in the conditional above actually check the dev value. > Yes, you are right, we doesn't need this change. The driver would not pass null dev in any case. And this driver already had a change "1fba81cc09bd thermal: tegra: remove null check for dev pointer" which remove this "dev" checking. Thank. Wei. > Thierry >
diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c index 3042837364e8..96527df91f2a 100644 --- a/drivers/thermal/tegra/soctherm.c +++ b/drivers/thermal/tegra/soctherm.c @@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev, struct soctherm_throt_cfg *stc, int trip_temp) { - struct tegra_soctherm *ts = dev_get_drvdata(dev); + struct tegra_soctherm *ts; int temp, cpu_throt, gpu_throt; unsigned int throt; u32 r, reg_off; @@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev, if (!sg || !stc || !stc->init) return -EINVAL; + ts = dev_get_drvdata(dev); + temp = enforce_temp_range(dev, trip_temp) / ts->soc->thresh_grain; /* Hardcode LIGHT on LEVEL1 and HEAVY on LEVEL2 */
Fix dereference dev before null check. Signed-off-by: Wei Ni <wni@nvidia.com> --- drivers/thermal/tegra/soctherm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)