All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Daniel Vetter <daniel.vetter@intel.com>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Maxime Ripard <maxime@cerno.tech>,
	Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Eric Anholt <eric@anholt.net>
Cc: devicetree@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com,
	Tim Gover <tim.gover@raspberrypi.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	Phil Elwell <phil@raspberrypi.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH v2 2/7] drm: Document use-after-free gotcha with private objects
Date: Fri,  4 Dec 2020 16:11:33 +0100	[thread overview]
Message-ID: <20201204151138.1739736-3-maxime@cerno.tech> (raw)
In-Reply-To: <20201204151138.1739736-1-maxime@cerno.tech>

The private objects have a gotcha that could result in a use-after-free,
make sure it's properly documented.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 include/drm/drm_atomic.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d07c851d255b..5d34c1df03f3 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -248,6 +248,26 @@ struct drm_private_state_funcs {
  *    drm_dev_register()
  * 2/ all calls to drm_atomic_private_obj_fini() must be done after calling
  *    drm_dev_unregister()
+ *
+ * If that private object is used to store a state shared by multiple
+ * CRTCs, proper care must be taken to ensure that non-blocking commits are
+ * properly ordered to avoid a use-after-free issue.
+ *
+ * Indeed, assuming a sequence of two non-blocking &drm_atomic_commit on two
+ * different &drm_crtc using different &drm_plane and &drm_connector, so with no
+ * resources shared, there's no guarantee on which commit is going to happen
+ * first. However, the second &drm_atomic_commit will consider the first
+ * &drm_private_obj its old state, and will be in charge of freeing it whenever
+ * the second &drm_atomic_commit is done.
+ *
+ * If the first &drm_atomic_commit happens after it, it will consider its
+ * &drm_private_obj the new state and will be likely to access it, resulting in
+ * an access to a freed memory region. Drivers should store (and get a reference
+ * to) the &drm_crtc_commit structure in our private state in
+ * &drm_mode_config_helper_funcs.atomic_commit_setup, and then wait for that
+ * commit to complete as the first step of
+ * &drm_mode_config_helper_funcs.atomic_commit_tail, similar to
+ * drm_atomic_helper_wait_for_dependencies().
  */
 struct drm_private_obj {
 	/**
-- 
2.28.0


WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Daniel Vetter <daniel.vetter@intel.com>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Maxime Ripard <maxime@cerno.tech>,
	Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Eric Anholt <eric@anholt.net>
Cc: devicetree@vger.kernel.org, Tim Gover <tim.gover@raspberrypi.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	dri-devel@lists.freedesktop.org,
	bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org,
	Phil Elwell <phil@raspberrypi.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/7] drm: Document use-after-free gotcha with private objects
Date: Fri,  4 Dec 2020 16:11:33 +0100	[thread overview]
Message-ID: <20201204151138.1739736-3-maxime@cerno.tech> (raw)
In-Reply-To: <20201204151138.1739736-1-maxime@cerno.tech>

The private objects have a gotcha that could result in a use-after-free,
make sure it's properly documented.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 include/drm/drm_atomic.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d07c851d255b..5d34c1df03f3 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -248,6 +248,26 @@ struct drm_private_state_funcs {
  *    drm_dev_register()
  * 2/ all calls to drm_atomic_private_obj_fini() must be done after calling
  *    drm_dev_unregister()
+ *
+ * If that private object is used to store a state shared by multiple
+ * CRTCs, proper care must be taken to ensure that non-blocking commits are
+ * properly ordered to avoid a use-after-free issue.
+ *
+ * Indeed, assuming a sequence of two non-blocking &drm_atomic_commit on two
+ * different &drm_crtc using different &drm_plane and &drm_connector, so with no
+ * resources shared, there's no guarantee on which commit is going to happen
+ * first. However, the second &drm_atomic_commit will consider the first
+ * &drm_private_obj its old state, and will be in charge of freeing it whenever
+ * the second &drm_atomic_commit is done.
+ *
+ * If the first &drm_atomic_commit happens after it, it will consider its
+ * &drm_private_obj the new state and will be likely to access it, resulting in
+ * an access to a freed memory region. Drivers should store (and get a reference
+ * to) the &drm_crtc_commit structure in our private state in
+ * &drm_mode_config_helper_funcs.atomic_commit_setup, and then wait for that
+ * commit to complete as the first step of
+ * &drm_mode_config_helper_funcs.atomic_commit_tail, similar to
+ * drm_atomic_helper_wait_for_dependencies().
  */
 struct drm_private_obj {
 	/**
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Daniel Vetter <daniel.vetter@intel.com>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Maxime Ripard <maxime@cerno.tech>,
	Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Eric Anholt <eric@anholt.net>
Cc: devicetree@vger.kernel.org, Tim Gover <tim.gover@raspberrypi.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	dri-devel@lists.freedesktop.org,
	bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org,
	Phil Elwell <phil@raspberrypi.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/7] drm: Document use-after-free gotcha with private objects
Date: Fri,  4 Dec 2020 16:11:33 +0100	[thread overview]
Message-ID: <20201204151138.1739736-3-maxime@cerno.tech> (raw)
In-Reply-To: <20201204151138.1739736-1-maxime@cerno.tech>

The private objects have a gotcha that could result in a use-after-free,
make sure it's properly documented.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 include/drm/drm_atomic.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d07c851d255b..5d34c1df03f3 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -248,6 +248,26 @@ struct drm_private_state_funcs {
  *    drm_dev_register()
  * 2/ all calls to drm_atomic_private_obj_fini() must be done after calling
  *    drm_dev_unregister()
+ *
+ * If that private object is used to store a state shared by multiple
+ * CRTCs, proper care must be taken to ensure that non-blocking commits are
+ * properly ordered to avoid a use-after-free issue.
+ *
+ * Indeed, assuming a sequence of two non-blocking &drm_atomic_commit on two
+ * different &drm_crtc using different &drm_plane and &drm_connector, so with no
+ * resources shared, there's no guarantee on which commit is going to happen
+ * first. However, the second &drm_atomic_commit will consider the first
+ * &drm_private_obj its old state, and will be in charge of freeing it whenever
+ * the second &drm_atomic_commit is done.
+ *
+ * If the first &drm_atomic_commit happens after it, it will consider its
+ * &drm_private_obj the new state and will be likely to access it, resulting in
+ * an access to a freed memory region. Drivers should store (and get a reference
+ * to) the &drm_crtc_commit structure in our private state in
+ * &drm_mode_config_helper_funcs.atomic_commit_setup, and then wait for that
+ * commit to complete as the first step of
+ * &drm_mode_config_helper_funcs.atomic_commit_tail, similar to
+ * drm_atomic_helper_wait_for_dependencies().
  */
 struct drm_private_obj {
 	/**
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-12-04 15:12 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04 15:11 [PATCH v2 0/7] vc4: Convert to drm_atomic_helper_commit Maxime Ripard
2020-12-04 15:11 ` Maxime Ripard
2020-12-04 15:11 ` Maxime Ripard
2020-12-04 15:11 ` [PATCH v2 1/7] drm: Introduce an atomic_commit_setup function Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 16:04   ` Daniel Vetter
2020-12-04 16:04     ` Daniel Vetter
2020-12-04 16:04     ` Daniel Vetter
2020-12-04 15:11 ` Maxime Ripard [this message]
2020-12-04 15:11   ` [PATCH v2 2/7] drm: Document use-after-free gotcha with private objects Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11 ` [PATCH v2 3/7] drm/vc4: Simplify a bit the global atomic_check Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11 ` [PATCH v2 4/7] drm/vc4: kms: Wait on previous FIFO users before a commit Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-09  0:28   ` Daniel Vetter
2020-12-09  0:28     ` Daniel Vetter
2020-12-09  0:28     ` Daniel Vetter
2020-12-04 15:11 ` [PATCH v2 5/7] drm/vc4: kms: Remove unassigned_channels from the HVS state Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 18:33   ` kernel test robot
2020-12-04 18:33     ` kernel test robot
2020-12-10 14:36   ` Maxime Ripard
2020-12-10 14:36     ` Maxime Ripard
2020-12-10 14:36     ` Maxime Ripard
2020-12-11 10:11   ` Thomas Zimmermann
2020-12-11 10:11     ` Thomas Zimmermann
2020-12-11 10:11     ` Thomas Zimmermann
2020-12-04 15:11 ` [PATCH v2 6/7] drm/vc4: kms: Remove async modeset semaphore Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11 ` [PATCH v2 7/7] drm/vc4: kms: Convert to atomic helpers Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-04 15:11   ` Maxime Ripard
2020-12-15 10:41 ` [PATCH v2 0/7] vc4: Convert to drm_atomic_helper_commit Maxime Ripard
2020-12-15 10:41   ` Maxime Ripard
2020-12-15 10:41   ` Maxime Ripard

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=20201204151138.1739736-3-maxime@cerno.tech \
    --to=maxime@cerno.tech \
    --cc=airlied@linux.ie \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=frowand.list@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mark.rutland@arm.com \
    --cc=phil@raspberrypi.com \
    --cc=robh+dt@kernel.org \
    --cc=tim.gover@raspberrypi.com \
    --cc=tzimmermann@suse.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.