From: Daniel Vetter <daniel@ffwll.ch>
To: Michel Thierry <michel.thierry@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: fix use-after-free in i915_drop_caches_set
Date: Mon, 8 Sep 2014 09:54:34 +0200 [thread overview]
Message-ID: <20140908075434.GS15520@phenom.ffwll.local> (raw)
In-Reply-To: <1409936003-9341-1-git-send-email-michel.thierry@intel.com>
On Fri, Sep 05, 2014 at 05:53:23PM +0100, Michel Thierry wrote:
> With the new vma/ppgtt lifetime rules, the ppgtt (vm) could be removed
> after i915_vma_unbind.
>
> Use list_for_each_entry_safe() to prevent this use-after-free.
>
> Found with gem_persistent_relocs and gem_evict_everything igt tests.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
I've merged Chris' version of this already, which additional also grabs a
temporary refcount to make sure the bo doesn't disappear untimely. Which
can happen if unbind ends up dropping the last ref when dropping the
active reference.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index dd736c0..4b05cd8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3820,8 +3820,8 @@ i915_drop_caches_set(void *data, u64 val)
> struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_gem_object *obj, *next;
> - struct i915_address_space *vm;
> - struct i915_vma *vma, *x;
> + struct i915_address_space *vm, *x;
> + struct i915_vma *vma, *y;
> int ret;
>
> DRM_DEBUG("Dropping caches: 0x%08llx\n", val);
> @@ -3842,8 +3842,9 @@ i915_drop_caches_set(void *data, u64 val)
> i915_gem_retire_requests(dev);
>
> if (val & DROP_BOUND) {
> - list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
> - list_for_each_entry_safe(vma, x, &vm->inactive_list,
> + list_for_each_entry_safe(vm, x, &dev_priv->vm_list,
> + global_link) {
> + list_for_each_entry_safe(vma, y, &vm->inactive_list,
> mm_list) {
> if (vma->pin_count)
> continue;
> --
> 2.0.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
prev parent reply other threads:[~2014-09-08 7:54 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-05 16:53 [PATCH] drm/i915: fix use-after-free in i915_drop_caches_set Michel Thierry
2014-09-08 7:54 ` Daniel Vetter [this message]
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=20140908075434.GS15520@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=michel.thierry@intel.com \
/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