| Message ID | 5e6e8882-478a-46c0-9119-b643d524cc0c@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: it6505: Fix potential NULL dereference | expand |
On Sat, Jun 08, 2024 at 05:21:08PM +0300, Dan Carpenter wrote: > Smatch complains correctly that the NULL checking isn't consistent: > > drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron() > error: we previously assumed 'pdata->pwr18' could be null > (see line 2569) > > Add a NULL check to prevent a NULL dereference on the error path. > > Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/gpu/drm/bridge/ite-it6505.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Sat, Jun 08, 2024 at 05:21:08PM +0300, Dan Carpenter wrote: > Smatch complains correctly that the NULL checking isn't consistent: > > drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron() > error: we previously assumed 'pdata->pwr18' could be null > (see line 2569) > > Add a NULL check to prevent a NULL dereference on the error path. > > Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/gpu/drm/bridge/ite-it6505.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c > index 3f68c82888c2..4f01fadaec0f 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -2580,7 +2580,8 @@ static int it6505_poweron(struct it6505 *it6505) > usleep_range(1000, 2000); > err = regulator_enable(pdata->ovdd); > if (err) { > - regulator_disable(pdata->pwr18); > + if (pdata->pwr18) > + regulator_disable(pdata->pwr18); Wait... I wat too quick to R-B it. The driver uses devm_regulator_get(), which always returns non-NULL result. So all `if (pdata->pwr18)` and `if (pdata->ovdd)` checks in the driver are useless. Could you please send a patch, removing them? > return err; > } > } > -- > 2.43.0 >
On Sun, Jun 09, 2024 at 10:38:39PM +0300, Dmitry Baryshkov wrote: > On Sat, Jun 08, 2024 at 05:21:08PM +0300, Dan Carpenter wrote: > > Smatch complains correctly that the NULL checking isn't consistent: > > > > drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron() > > error: we previously assumed 'pdata->pwr18' could be null > > (see line 2569) > > > > Add a NULL check to prevent a NULL dereference on the error path. > > > > Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > --- > > drivers/gpu/drm/bridge/ite-it6505.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c > > index 3f68c82888c2..4f01fadaec0f 100644 > > --- a/drivers/gpu/drm/bridge/ite-it6505.c > > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > > @@ -2580,7 +2580,8 @@ static int it6505_poweron(struct it6505 *it6505) > > usleep_range(1000, 2000); > > err = regulator_enable(pdata->ovdd); > > if (err) { > > - regulator_disable(pdata->pwr18); > > + if (pdata->pwr18) > > + regulator_disable(pdata->pwr18); > > Wait... I wat too quick to R-B it. The driver uses devm_regulator_get(), > which always returns non-NULL result. So all `if (pdata->pwr18)` and > `if (pdata->ovdd)` checks in the driver are useless. Could you please > send a patch, removing them? > Sure. Will do. regards, dan carpenter
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c index 3f68c82888c2..4f01fadaec0f 100644 --- a/drivers/gpu/drm/bridge/ite-it6505.c +++ b/drivers/gpu/drm/bridge/ite-it6505.c @@ -2580,7 +2580,8 @@ static int it6505_poweron(struct it6505 *it6505) usleep_range(1000, 2000); err = regulator_enable(pdata->ovdd); if (err) { - regulator_disable(pdata->pwr18); + if (pdata->pwr18) + regulator_disable(pdata->pwr18); return err; } }
Smatch complains correctly that the NULL checking isn't consistent: drivers/gpu/drm/bridge/ite-it6505.c:2583 it6505_poweron() error: we previously assumed 'pdata->pwr18' could be null (see line 2569) Add a NULL check to prevent a NULL dereference on the error path. Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/gpu/drm/bridge/ite-it6505.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)