| Message ID | 20210618052800.89007-1-jjcao20@fudan.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/meson: fix potential NULL pointer exception in meson_drv_unbind() | expand |
Hello, first of all: thanks for your patch and sorry for being late with my review question. On Fri, Jun 18, 2021 at 7:28 AM Jiajun Cao <jjcao20@fudan.edu.cn> wrote: > > Fix a potential NULL pointer exception when meson_drv_unbind() > attempts to operate on the driver_data priv which may be NULL. > Add a null pointer check on the priv struct to avoid the NULL > pointer dereference after calling dev_get_drvdata(), just like > the null pointer checks done on the struct priv in the function > meson_drv_shutdown(), meson_drv_pm_suspend() and meson_drv_pm_resume(). I am trying to review Amlogic Meson related patches in the DRM subsystem so I can help Neil with this. However, I am still new to this so please help me educate on this topic. [...] > static void meson_drv_unbind(struct device *dev) > { > struct meson_drm *priv = dev_get_drvdata(dev); > - struct drm_device *drm = priv->drm; > + struct drm_device *drm; > + > + if (!priv) > + return; My understanding of the component framework is that meson_drv_unbind() is only called if previously meson_drv_bind() was called (and did not return any error). This is different from meson_drv_shutdown() (for example) because that can be called if meson_drv_probe() returns 0 (success) in case the "count" variable was 0 (then the probe function does nothing). As I mentioned before: I am still learning about the DRM subsystem in the Linux kernel. So it would be great if you could help me understand for which scenarios this newly added if-condition is needed. Thank you! Best regards, Martin
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 07fcd12dca16..f544fba8c44e 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -380,7 +380,10 @@ static int meson_drv_bind(struct device *dev) static void meson_drv_unbind(struct device *dev) { struct meson_drm *priv = dev_get_drvdata(dev); - struct drm_device *drm = priv->drm; + struct drm_device *drm; + + if (!priv) + return; if (priv->canvas) { meson_canvas_free(priv->canvas, priv->canvas_id_osd1); @@ -389,6 +392,7 @@ static void meson_drv_unbind(struct device *dev) meson_canvas_free(priv->canvas, priv->canvas_id_vd1_2); } + drm = priv->drm; drm_dev_unregister(drm); drm_kms_helper_poll_fini(drm); drm_atomic_helper_shutdown(drm);