From: Greg KH <gregkh@linuxfoundation.org>
To: Steve Cohen <cohens@codeaurora.org>
Cc: dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
linux-arm-msm@vger.kernel.org, sam@ravnborg.org, daniel@ffwll.ch,
seanpaul@chromium.org, pdhaval@codeaurora.org,
jsanka@codeaurora.org, adelva@google.com
Subject: Re: [PATCH V2] drm: hold gem reference until object is no longer accessed
Date: Mon, 27 Jul 2020 21:55:07 +0200 [thread overview]
Message-ID: <20200727195507.GA240123@kroah.com> (raw)
In-Reply-To: <1595284250-31580-1-git-send-email-cohens@codeaurora.org>
On Mon, Jul 20, 2020 at 06:30:50PM -0400, Steve Cohen wrote:
> A use-after-free in drm_gem_open_ioctl can happen if the
> GEM object handle is closed between the idr lookup and
> retrieving the size from said object since a local reference
> is not being held at that point. Hold the local reference
> while the object can still be accessed to fix this and
> plug the potential security hole.
>
> Signed-off-by: Steve Cohen <cohens@codeaurora.org>
> ---
> drivers/gpu/drm/drm_gem.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 7bf628e..ee2058a 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -871,9 +871,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
> * @file_priv: drm file-private structure
> *
> * Open an object using the global name, returning a handle and the size.
> - *
> - * This handle (of course) holds a reference to the object, so the object
> - * will not go away until the handle is deleted.
> */
> int
> drm_gem_open_ioctl(struct drm_device *dev, void *data,
> @@ -898,14 +895,15 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
>
> /* drm_gem_handle_create_tail unlocks dev->object_name_lock. */
> ret = drm_gem_handle_create_tail(file_priv, obj, &handle);
> - drm_gem_object_put_unlocked(obj);
> if (ret)
> - return ret;
> + goto err;
>
> args->handle = handle;
> args->size = obj->size;
>
> - return 0;
> +err:
> + drm_gem_object_put_unlocked(obj);
> + return ret;
> }
>
> /**
As this seems to fix an important issue, any reason it wasn't cc: stable
on it so that it gets backported properly?
How about a "Fixes:" tag so that we know what commit id it fixes so we
know how far back to backport things?
And a hint to the maintainers that "this is an issue that needs to get
into 5.8-final, it shouldn't wait around longer please" would have also
been nice to see :)
And what chagned from v1, aren't you supposed to list that somewhere in
the changelog or below the --- line (never remember what DRM drivers
want here...)
Care to send a v3?
thanks,
greg k-h
next prev parent reply other threads:[~2020-07-27 19:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-20 22:30 [PATCH V2] drm: hold gem reference until object is no longer accessed Steve Cohen
2020-07-27 19:55 ` Greg KH [this message]
2020-07-27 20:11 ` daniel
2020-07-27 21:54 ` [Freedreno] " cohens
2020-07-28 8:52 ` daniel
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=20200727195507.GA240123@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=adelva@google.com \
--cc=cohens@codeaurora.org \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jsanka@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=pdhaval@codeaurora.org \
--cc=sam@ravnborg.org \
--cc=seanpaul@chromium.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).