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/arm/malidp: use drmm_* to allocate driver structures
drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
drm/arm/malidp: plane: use drm managed resources
drm/arm/malidp: use drm_dev_unplug()
drm/arm/malidp: plane: protect device resources after removal
drm/arm/malidp: crtc: protect device resources after removal
drm/arm/malidp: drv: protect device resources after removal
drivers/gpu/drm/arm/malidp_crtc.c | 48 +++++++++++++++++++++---
drivers/gpu/drm/arm/malidp_drv.c | 58 ++++++++++++++---------------
drivers/gpu/drm/arm/malidp_drv.h | 2 +
drivers/gpu/drm/arm/malidp_hw.c | 10 ++---
drivers/gpu/drm/arm/malidp_mw.c | 6 +--
drivers/gpu/drm/arm/malidp_planes.c | 45 +++++++++++-----------
6 files changed, 100 insertions(+), 69 deletions(-)
base-commit: 8fe444eb326869823f3788a4b4da5dca03339d10
--
2.37.2
(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/arm/malidp_planes.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 34547edf1ee3..d2ea60549454 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -790,9 +790,12 @@ static void malidp_de_plane_update(struct drm_plane *plane,
u16 pixel_alpha = new_state->pixel_blend_mode;
u8 plane_alpha = new_state->alpha >> 8;
u32 src_w, src_h, dest_w, dest_h, val;
- int i;
+ int i, idx;
struct drm_framebuffer *fb = plane->state->fb;
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
+
mp = to_malidp_plane(plane);
/*
@@ -897,16 +900,24 @@ static void malidp_de_plane_update(struct drm_plane *plane,
malidp_hw_write(mp->hwdev, val,
mp->layer->base + MALIDP_LAYER_CONTROL);
+
+ drm_dev_exit(idx);
}
static void malidp_de_plane_disable(struct drm_plane *plane,
struct drm_atomic_state *state)
{
struct malidp_plane *mp = to_malidp_plane(plane);
+ int idx;
+
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
malidp_hw_clearbits(mp->hwdev,
LAYER_ENABLE | LAYER_FLOWCFG(LAYER_FLOWCFG_MASK),
mp->layer->base + MALIDP_LAYER_CONTROL);
+
+ drm_dev_exit(idx);
}
static const struct drm_plane_helper_funcs malidp_de_plane_helper_funcs = {
--
2.37.2
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/arm/malidp_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 678c5b0d8014..aedd30f5f451 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -893,7 +893,7 @@ static void malidp_unbind(struct device *dev)
struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
- drm_dev_unregister(drm);
+ drm_dev_unplug(drm);
drm_kms_helper_poll_fini(drm);
pm_runtime_get_sync(dev);
drm_atomic_helper_shutdown(drm);
--
2.37.2
On Mon, Sep 05, 2022 at 05:19:02PM +0200, Danilo Krummrich wrote:
> Hi,
Hi Danilo,
>
> 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.
Appologies for the extended silence, I was on holiday for 3 weeks and stayed away
from mailing lists. Will review the two series this week.
Best regards,
Liviu
>
> Danilo Krummrich (8):
> drm/arm/malidp: use drmm_* to allocate driver structures
> drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
> drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
> drm/arm/malidp: plane: use drm managed resources
> drm/arm/malidp: use drm_dev_unplug()
> drm/arm/malidp: plane: protect device resources after removal
> drm/arm/malidp: crtc: protect device resources after removal
> drm/arm/malidp: drv: protect device resources after removal
>
> drivers/gpu/drm/arm/malidp_crtc.c | 48 +++++++++++++++++++++---
> drivers/gpu/drm/arm/malidp_drv.c | 58 ++++++++++++++---------------
> drivers/gpu/drm/arm/malidp_drv.h | 2 +
> drivers/gpu/drm/arm/malidp_hw.c | 10 ++---
> drivers/gpu/drm/arm/malidp_mw.c | 6 +--
> drivers/gpu/drm/arm/malidp_planes.c | 45 +++++++++++-----------
> 6 files changed, 100 insertions(+), 69 deletions(-)
>
>
> base-commit: 8fe444eb326869823f3788a4b4da5dca03339d10
> --
> 2.37.2
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯