linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: Keith Packard <keithp@keithp.com>,
	Dave Airlie <airlied@redhat.com>, David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, jannh@google.com
Cc: linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>
Subject: [PATCH] drm: fix use-after-free read in drm_mode_create_lease_ioctl()
Date: Mon,  1 Oct 2018 17:31:17 +0200	[thread overview]
Message-ID: <20181001153117.216923-1-jannh@google.com> (raw)

fd_install() moves the reference given to it into the file descriptor table
of the current process. If the current process is multithreaded, then
immediately after fd_install(), another thread can close() the file
descriptor and cause the file's resources to be cleaned up.

Since the reference to "lessee" is held by the file, we must not access
"lessee" after the fd_install() call.

As far as I can tell, to reach this codepath, the caller must have an open
file descriptor to a DRI device in master mode. I'm not sure what the
requirements for that are.

Signed-off-by: Jann Horn <jannh@google.com>
Fixes: 62884cd386b8 ("drm: Add four ioctls for managing drm mode object leases [v7]")
Cc: stable@vger.kernel.org
---
I'm not sure how to actually use this ioctl, so I have neither verified
the bug experimentally nor experimentally verified the fix. I would
appreciate it if someone could confirm my analysis.

There have been a number of fd_install() bugs over time; I think it's
probably time to rename fd_install() to fd_install_dropref(), or
something like that.

 drivers/gpu/drm/drm_lease.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index b54fb78a283c..b82da96ded5c 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -566,14 +566,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
 	lessee_priv->is_master = 1;
 	lessee_priv->authenticated = 1;
 
-	/* Hook up the fd */
-	fd_install(fd, lessee_file);
-
 	/* Pass fd back to userspace */
 	DRM_DEBUG_LEASE("Returning fd %d id %d\n", fd, lessee->lessee_id);
 	cl->fd = fd;
 	cl->lessee_id = lessee->lessee_id;
 
+	/* Hook up the fd */
+	fd_install(fd, lessee_file);
+
 	DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
 	return 0;
 
-- 
2.19.0.605.g01d371f741-goog


             reply	other threads:[~2018-10-01 15:31 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-01 15:31 Jann Horn [this message]
2018-10-02  8:24 ` [PATCH] drm: fix use-after-free read in drm_mode_create_lease_ioctl() Daniel Vetter

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=20181001153117.216923-1-jannh@google.com \
    --to=jannh@google.com \
    --cc=airlied@linux.ie \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=keescook@chromium.org \
    --cc=keithp@keithp.com \
    --cc=linux-kernel@vger.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).