All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Hackmann <ghackmann@android.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Omer Tripp <trippo@google.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	gregkh@linuxfoundation.org, Greg Hackmann <ghackmann@android.com>,
	stable@vger.kernel.org
Subject: [PATCH v2] fs: fix possible Spectre V1 indexing in __close_fd()
Date: Mon, 24 Dec 2018 06:26:42 -0800	[thread overview]
Message-ID: <20181224142642.7385-1-ghackmann@android.com> (raw)

Omer Tripp's analysis of a Spectre V1 gadget in __close_fd():

"1.  __close_fd() is reachable via the close() syscall with a
     user-controlled fd.

 2.  If said bounds check is mispredicted, then a user-controlled
     address fdt->fd[fd] is obtained then dereferenced, and the value of
     a user-controlled address is loaded into the local variable file.

 3.  file is then passed as an argument to filp_close, where the cache
     lines secret + offsetof(f_op) and secret + offsetof(f_mode) are hot
     and vulnerable to a timing channel attack."

Address this by using array_index_nospec() to prevent speculation past
the end of current->fdt.

Reported-by: Omer Tripp <trippo@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Hackmann <ghackmann@android.com>
---
v2: include Omer Tripp's analysis in commit message, and update my email
    address

 fs/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/file.c b/fs/file.c
index 7ffd6e9d103d..a80cf82be96b 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -18,6 +18,7 @@
 #include <linux/bitops.h>
 #include <linux/spinlock.h>
 #include <linux/rcupdate.h>
+#include <linux/nospec.h>
 
 unsigned int sysctl_nr_open __read_mostly = 1024*1024;
 unsigned int sysctl_nr_open_min = BITS_PER_LONG;
@@ -626,6 +627,7 @@ int __close_fd(struct files_struct *files, unsigned fd)
 	fdt = files_fdtable(files);
 	if (fd >= fdt->max_fds)
 		goto out_unlock;
+	fd = array_index_nospec(fd, fdt->max_fds);
 	file = fdt->fd[fd];
 	if (!file)
 		goto out_unlock;
-- 
2.19.1

             reply	other threads:[~2018-12-24 14:26 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-24 14:26 Greg Hackmann [this message]
2018-12-24 14:40 ` [PATCH v2] fs: fix possible Spectre V1 indexing in __close_fd() Matthew Wilcox

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=20181224142642.7385-1-ghackmann@android.com \
    --to=ghackmann@android.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=trippo@google.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.