[PATCH 1/2] drm/imx: fix use after free
Philipp Zabel
p.zabel at pengutronix.de
Thu Jun 11 14:51:57 UTC 2020
Hi Russell,
On Thu, 2020-06-11 at 14:01 +0100, Russell King - ARM Linux admin wrote:
> On Thu, Jun 11, 2020 at 02:43:31PM +0200, Marco Felsch wrote:
> > From: Philipp Zabel <p.zabel at pengutronix.de>
> >
> > Component driver structures allocated with devm_kmalloc() in bind() are
> > freed automatically after unbind(). Since the contained drm structures
> > are accessed afterwards in drm_mode_config_cleanup(), move the
> > allocation into probe() to extend the driver structure's lifetime to the
> > lifetime of the device. This should eventually be changed to use drm
> > resource managed allocations with lifetime of the drm device.
>
> You need to be extremely careful doing this. If the allocation is
> in the probe function, it's lifetime is not just until unbind, but
> potentitally to the _next_ bind, unbind, bind, unbind. In other
> words, it's lifetime is from the point that the component is probed
> to the point that it is later removed.
>
> If the driver relies on initialisation of that structure, then that
> must be _very_ carefully handled - any state in that structure will
> remain.
>
> So, you need to think long and hard about changes like this, and do
> a thorough review of the lifetime of every structure member.
Thank you for the warning, I've tried to make sure that no driver relies
on prior initialization by explicitly replacing each
x = devm_kzalloc(dev, sizeof(*x), GFP_KERNEL);
in .bind() with a
memset(x, 0, sizeof(*x));
The patch still requires the lifetime of embedded connector and encoder
structures to end somewhere between .unbind() and the next .bind(), but
that should be guaranteed by calling drm_mode_config_cleanup() after
component_unbind_all().
I'd like to replace this with devm_drm_dev_alloc() afterwards, but doing
this first would allow to fix stable kernels as well.
regards
Philipp
More information about the dri-devel
mailing list