| Message ID | 20200219102122.1607365-40-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm_device managed resources | expand |
Hi Daniel, On 2/19/20 11:21 AM, Daniel Vetter wrote: > It's right above the drm_dev_put(). > > Aside: Another driver with a bit much devm_kzalloc, which should > probably use drmm_kzalloc instead ... > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Yannick Fertre <yannick.fertre@st.com> > Cc: Philippe Cornu <philippe.cornu@st.com> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> > Cc: Vincent Abriou <vincent.abriou@st.com> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > Cc: Alexandre Torgue <alexandre.torgue@st.com> > Cc: linux-stm32@st-md-mailman.stormreply.com > Cc: linux-arm-kernel@lists.infradead.org > --- > drivers/gpu/drm/stm/drv.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c > index ea9fcbdc68b3..5b374531dd8c 100644 > --- a/drivers/gpu/drm/stm/drv.c > +++ b/drivers/gpu/drm/stm/drv.c > @@ -88,7 +88,9 @@ static int drv_load(struct drm_device *ddev) > > ddev->dev_private = (void *)ldev; > > - drm_mode_config_init(ddev); > + ret = drm_mode_config_init(ddev); > + if (ret) > + return ret; > > /* > * set max width and height as default value. > @@ -103,7 +105,7 @@ static int drv_load(struct drm_device *ddev) > > ret = ltdc_load(ddev); > if (ret) > - goto err; > + return ret; > > drm_mode_config_reset(ddev); > drm_kms_helper_poll_init(ddev); > @@ -111,9 +113,6 @@ static int drv_load(struct drm_device *ddev) > platform_set_drvdata(pdev, ddev); > > return 0; > -err: > - drm_mode_config_cleanup(ddev); > - return ret; > } > > static void drv_unload(struct drm_device *ddev) > @@ -122,7 +121,6 @@ static void drv_unload(struct drm_device *ddev) > > drm_kms_helper_poll_fini(ddev); > ltdc_unload(ddev); > - drm_mode_config_cleanup(ddev); > } > > static __maybe_unused int drv_suspend(struct device *dev) > Thank you for your patch, For this stm part, Acked-by: Philippe Cornu <philippe.cornu@st.com> note: we will handle devm_kzalloc() asap, thanks. Philippe :-)
On Thu, Feb 20, 2020 at 3:19 PM Philippe CORNU <philippe.cornu@st.com> wrote: > > Hi Daniel, > > On 2/19/20 11:21 AM, Daniel Vetter wrote: > > It's right above the drm_dev_put(). > > > > Aside: Another driver with a bit much devm_kzalloc, which should > > probably use drmm_kzalloc instead ... > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Yannick Fertre <yannick.fertre@st.com> > > Cc: Philippe Cornu <philippe.cornu@st.com> > > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> > > Cc: Vincent Abriou <vincent.abriou@st.com> > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > > Cc: Alexandre Torgue <alexandre.torgue@st.com> > > Cc: linux-stm32@st-md-mailman.stormreply.com > > Cc: linux-arm-kernel@lists.infradead.org > > --- > > drivers/gpu/drm/stm/drv.c | 10 ++++------ > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c > > index ea9fcbdc68b3..5b374531dd8c 100644 > > --- a/drivers/gpu/drm/stm/drv.c > > +++ b/drivers/gpu/drm/stm/drv.c > > @@ -88,7 +88,9 @@ static int drv_load(struct drm_device *ddev) > > > > ddev->dev_private = (void *)ldev; > > > > - drm_mode_config_init(ddev); > > + ret = drm_mode_config_init(ddev); > > + if (ret) > > + return ret; > > > > /* > > * set max width and height as default value. > > @@ -103,7 +105,7 @@ static int drv_load(struct drm_device *ddev) > > > > ret = ltdc_load(ddev); > > if (ret) > > - goto err; > > + return ret; > > > > drm_mode_config_reset(ddev); > > drm_kms_helper_poll_init(ddev); > > @@ -111,9 +113,6 @@ static int drv_load(struct drm_device *ddev) > > platform_set_drvdata(pdev, ddev); > > > > return 0; > > -err: > > - drm_mode_config_cleanup(ddev); > > - return ret; > > } > > > > static void drv_unload(struct drm_device *ddev) > > @@ -122,7 +121,6 @@ static void drv_unload(struct drm_device *ddev) > > > > drm_kms_helper_poll_fini(ddev); > > ltdc_unload(ddev); > > - drm_mode_config_cleanup(ddev); > > } > > > > static __maybe_unused int drv_suspend(struct device *dev) > > > > Thank you for your patch, > For this stm part, > Acked-by: Philippe Cornu <philippe.cornu@st.com> > > note: we will handle devm_kzalloc() asap, thanks. Note that as-is you can't just blindly switch devm_kzalloc over to drmm_kzalloc for the structures containing a drm_* object, or you'll just replace one type of use-after free with another one (and probably worse, since the new one will hit you on normal driver unload too). There's a bit more work needed in this area, this here is just the first steps and a heads up. And removing the devm_kzalloc would result in lots of code added for a bunch of kfree() all over, not so great option either. I'd say wait for the next round :-) -Daniel
diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index ea9fcbdc68b3..5b374531dd8c 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -88,7 +88,9 @@ static int drv_load(struct drm_device *ddev) ddev->dev_private = (void *)ldev; - drm_mode_config_init(ddev); + ret = drm_mode_config_init(ddev); + if (ret) + return ret; /* * set max width and height as default value. @@ -103,7 +105,7 @@ static int drv_load(struct drm_device *ddev) ret = ltdc_load(ddev); if (ret) - goto err; + return ret; drm_mode_config_reset(ddev); drm_kms_helper_poll_init(ddev); @@ -111,9 +113,6 @@ static int drv_load(struct drm_device *ddev) platform_set_drvdata(pdev, ddev); return 0; -err: - drm_mode_config_cleanup(ddev); - return ret; } static void drv_unload(struct drm_device *ddev) @@ -122,7 +121,6 @@ static void drv_unload(struct drm_device *ddev) drm_kms_helper_poll_fini(ddev); ltdc_unload(ddev); - drm_mode_config_cleanup(ddev); } static __maybe_unused int drv_suspend(struct device *dev)
It's right above the drm_dev_put(). Aside: Another driver with a bit much devm_kzalloc, which should probably use drmm_kzalloc instead ... Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Yannick Fertre <yannick.fertre@st.com> Cc: Philippe Cornu <philippe.cornu@st.com> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> Cc: Vincent Abriou <vincent.abriou@st.com> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: Alexandre Torgue <alexandre.torgue@st.com> Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/stm/drv.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)