dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org,
	shawnguo@kernel.org, kernel@pengutronix.de,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] drm/imx: fix use after free
Date: Thu, 11 Jun 2020 14:01:45 +0100	[thread overview]
Message-ID: <20200611130145.GX1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200611124332.20819-1-m.felsch@pengutronix.de>

On Thu, Jun 11, 2020 at 02:43:31PM +0200, Marco Felsch wrote:
> From: Philipp Zabel <p.zabel@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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 503kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-06-11 13:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-11 12:43 [PATCH 1/2] drm/imx: fix use after free Marco Felsch
2020-06-11 12:43 ` [PATCH 2/2] drm/imx: tve: fix regulator_disable error path Marco Felsch
2020-06-11 13:01 ` Russell King - ARM Linux admin [this message]
2020-06-11 14:51   ` [PATCH 1/2] drm/imx: fix use after free Philipp Zabel
2020-07-20 13:22 ` Philipp Zabel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200611130145.GX1551@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=m.felsch@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).