linux-kernel - Re: [PATCH] fs: fix possible Spectre V1 indexing in __close

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Sep 2018 20:39:11 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Greg Hackmann <ghackmann@...roid.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Omer Tripp <trippo@...gle.com>, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, Greg Hackmann <ghackmann@...gle.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] fs: fix possible Spectre V1 indexing in __close_fd()

On Mon, Sep 24, 2018 at 11:15:00AM -0700, Greg Hackmann wrote:
> __close_fd() is reachable via the close() syscall with a
> userspace-controlled fd.  Ensure that it can't be used to speculatively
> access past the end of current->fdt.
> 
> Reported-by: Omer Tripp <trippo@...gle.com>
> Cc: stable@...r.kernel.org
> Signed-off-by: Greg Hackmann <ghackmann@...gle.com>
> ---
>  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];

Don't you need 2 "halfs" of a gadget in order to make it work?  This is
one half, where is the second half?

Or am I reading this code wrong here somehow?

We don't want to play whack-a-mole with only 1/2 spectre gadgets,
otherwise the 700+ patches that Red Hat added to their kernel would have
been merged already.

Which reminds me, did the Red Hat tooling catch this one as well?  If
not, someone need to go fix it :)

thanks,

greg k-h