Hi,
This patch series converts the driver to use drm managed resources to prevent
potential use-after-free issues on driver unbind/rebind and to get rid of the
usage of deprecated APIs.
Danilo Krummrich (8):
drm/fsl-dcu: use drmm_* to allocate driver structures
drm/fsl-dcu: replace drm->dev_private with drm_to_fsl_dcu_drm_dev()
drm/fsl-dcu: crtc: use drmm_crtc_init_with_planes()
drm/fsl-dcu: plane: use drm managed resources
drm/fsl-dcu: use drm_dev_unplug()
drm/fsl-dcu: plane: protect device resources after removal
drm/fsl-dcu: crtc: protect device resources after removal
drm/fsl-dcu: remove trailing return statements
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 64 ++++++++++++++++-----
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 43 ++++++--------
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 4 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 19 +++---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 48 ++++++++--------
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 8 +--
6 files changed, 108 insertions(+), 78 deletions(-)
base-commit: 961bcdf956a4645745407a5d919be8757549b062
--
2.37.3
Remove the trailing return statements at the end of void functions.
Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 1 -
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index c77df9b7893f..23687551c831 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -147,7 +147,6 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
DCU_THRESHOLD_OUT_BUF_LOW(BUF_MIN_VAL));
drm_dev_exit(idx);
- return;
}
static const struct drm_crtc_helper_funcs fsl_dcu_drm_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 1be3062a95df..d0a14b5b506e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -172,7 +172,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
}
drm_dev_exit(idx);
- return;
}
static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
--
2.37.3
When the driver is unbound, there might still be users in userspace
having an open fd and are calling into the driver.
While this is fine for drm managed resources, it is not for resources
bound to the device/driver lifecycle, e.g. clocks or MMIO mappings.
To prevent use-after-free issues we need to protect those resources with
drm_dev_enter() and drm_dev_exit(). This does only work if we indicate
that the drm device was unplugged, hence use drm_dev_unplug() instead of
drm_dev_unregister().
Protecting the particular resources with drm_dev_enter()/drm_dev_exit()
is handled by subsequent patches.
Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 4139f674c5de..3ac57516c3fe 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -340,7 +340,7 @@ static int fsl_dcu_drm_remove(struct platform_device *pdev)
struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev);
struct drm_device *drm = &fsl_dev->base;
- drm_dev_unregister(drm);
+ drm_dev_unplug(drm);
clk_disable_unprepare(fsl_dev->clk);
clk_unregister(fsl_dev->pix_clk);
--
2.37.3
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 23ff285da477..1be3062a95df 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -10,6 +10,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_crtc.h>
+#include <drm/drm_drv.h>
#include <drm/drm_fb_dma_helper.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
@@ -65,7 +66,10 @@ static void fsl_dcu_drm_plane_atomic_disable(struct drm_plane *plane,
{
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(plane->dev);
unsigned int value;
- int index;
+ int index, idx;
+
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
@@ -74,6 +78,8 @@ static void fsl_dcu_drm_plane_atomic_disable(struct drm_plane *plane,
regmap_read(fsl_dev->regmap, DCU_CTRLDESCLN(index, 4), &value);
value &= ~DCU_LAYER_EN;
regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(index, 4), value);
+
+ drm_dev_exit(idx);
}
static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
@@ -86,7 +92,10 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
struct drm_framebuffer *fb = plane->state->fb;
struct drm_gem_dma_object *gem;
unsigned int alpha = DCU_LAYER_AB_NONE, bpp;
- int index;
+ int index, idx;
+
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
if (!fb)
return;
@@ -162,6 +171,7 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
DCU_LAYER_PRE_SKIP(0));
}
+ drm_dev_exit(idx);
return;
}
--
2.37.3
Use drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
to get rid of the explicit destroy hook in struct drm_plane_funcs.
Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index e05311e2b0e0..0b70624260fc 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -159,7 +159,6 @@ static void fsl_dcu_drm_crtc_disable_vblank(struct drm_crtc *crtc)
static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = {
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
- .destroy = drm_crtc_cleanup,
.page_flip = drm_atomic_helper_page_flip,
.reset = drm_atomic_helper_crtc_reset,
.set_config = drm_atomic_helper_set_config,
@@ -180,8 +179,8 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
if (!primary)
return -ENOMEM;
- ret = drm_crtc_init_with_planes(drm, crtc, primary, NULL,
- &fsl_dcu_drm_crtc_funcs, NULL);
+ ret = drmm_crtc_init_with_planes(drm, crtc, primary, NULL,
+ &fsl_dcu_drm_crtc_funcs, NULL);
if (ret) {
primary->funcs->destroy(primary);
return ret;
--
2.37.3