[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