linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "Van De Ven, Arjan" <arjan.van.de.ven@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arch <linux-arch@vger.kernel.org>,
	Cyril Novikov <cnovikov@lynx.com>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	X86 ML <x86@kernel.org>, Will Deacon <will.deacon@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Alan Cox <alan@linux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references
Date: Wed, 31 Jan 2018 15:21:38 +0100	[thread overview]
Message-ID: <20180131142138.GA9233@kroah.com> (raw)
In-Reply-To: <0575AF4FD06DD142AD198903C74E1CC87A605256@ORSMSX103.amr.corp.intel.com>

On Wed, Jan 31, 2018 at 02:13:45PM +0000, Van De Ven, Arjan wrote:
> > > short term there was some extremely rudimentary static analysis done. clearly
> > > that has extreme limitations both in insane rate of false positives, and missing
> > > cases.
> > 
> > What was the output roughly, how many suspect places that need
> > array_idx_nospec()
> > handling: a few, a few dozen, a few hundred or a few thousand?
> > 
> > I'm guessing 'static tool emitted hundreds of sites with many false positives
> > included, but the real sites are probably a few dozen' - but that's really just a
> > very, very wild guess.
> 
> your guess is pretty accurate; we ended up with some 15 or so places
> (the first patch kit Dan mailed out)

But in looking at that first patch set, I don't see many, if any, that
could be solved with your proposal of copy_from_user_struct().  The two
networking patches couldn't, the scsi one was just bizarre (seriously,
you had to trust the input from the hardware, if not, there's worse
things happening), and the networking drivers were dealing with other
data streams I think.

So while I love the idea of copy_from_user_struct(), I don't see it as
any type of "this will solve the issues we are trying to address here"
solution :(

I've been emailing the Coverity people recently about this, and they
say they are close to having a ruleset/test that can identify this data
pattern better than the original tool that Intel and others came up
with.  But as I haven't seen the output of it yet, I have no idea if
that's true or not.  Hopefully they will release it in a few days so we
can get an idea of if this is even going to be possible to automatically
scan for at all with their tool.

Other than Coverity, I don't know of any other tool that is trying to
even identify this pattern :(

> > Also, IMHO any tooling should very much be open source: this isn't the kind of
> > bug
> > that can be identified via code review, so there's no fall-back detection method
> > like we have for buffer overflows.
> 
> we absolutely need some good open source tooling; my personal
> preference will be a compiler plugin; that way you can use all the
> fancy logic inside the compilers for analysis, and you can make a "I
> don't care just fix it" option in addition to flagging for human
> inspection as the kernel would.

I thought gcc plugins were going to enable this type of analysis, or
maybe clang plugins, but I have yet to hear of anyone working on this.

thanks,

greg k-h

  reply	other threads:[~2018-01-31 14:21 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-27  7:55 [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Dan Williams
2018-01-27  7:55 ` Dan Williams
2018-01-27  7:55 ` [PATCH v5 01/12] Documentation: document array_idx Dan Williams
2018-01-27  7:55 ` [PATCH v5 02/12] array_idx: sanitize speculative array de-references Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  8:55   ` Ingo Molnar
2018-01-28 11:36     ` Thomas Gleixner
2018-01-28 11:36       ` Thomas Gleixner
2018-01-28 16:28     ` Dan Williams
2018-01-28 18:33       ` Ingo Molnar
2018-01-29 16:45         ` Dan Williams
2018-01-29 16:45           ` Dan Williams
2018-01-28 18:36       ` Thomas Gleixner
2018-01-28 18:36         ` Thomas Gleixner
2018-01-30  6:29         ` Dan Williams
2018-01-30  6:29           ` Dan Williams
2018-01-30 19:38           ` Linus Torvalds
2018-01-30 20:13             ` Dan Williams
2018-01-30 20:27               ` Van De Ven, Arjan
2018-01-31  8:03                 ` Ingo Molnar
2018-01-31 14:13                   ` Van De Ven, Arjan
2018-01-31 14:21                     ` Greg KH [this message]
2018-01-27  7:55 ` [PATCH v5 03/12] x86: implement array_idx_mask Dan Williams
2018-01-28  9:02   ` Ingo Molnar
2018-01-27  7:55 ` [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence Dan Williams
2018-01-28  9:06   ` Ingo Molnar
2018-01-28  9:14   ` Ingo Molnar
2018-01-29 20:41     ` Dan Williams
2018-01-29 20:41       ` Dan Williams
2018-01-30  6:56       ` Ingo Molnar
2018-01-27  7:55 ` [PATCH v5 05/12] x86, __get_user: use __uaccess_begin_nospec Dan Williams
2018-01-28  9:19   ` Ingo Molnar
2018-01-28  9:19     ` Ingo Molnar
2018-01-27  7:55 ` [PATCH v5 06/12] x86, get_user: use pointer masking to limit speculation Dan Williams
2018-01-27  7:55   ` Dan Williams
2018-01-28  9:25   ` Ingo Molnar
2018-01-28  9:25     ` Ingo Molnar
2018-01-27  7:55 ` [PATCH v5 07/12] x86: remove the syscall_64 fast-path Dan Williams
2018-01-28  9:29   ` Ingo Molnar
2018-01-28  9:29     ` Ingo Molnar
2018-01-28 15:22     ` Andy Lutomirski
2018-01-28 15:22       ` Andy Lutomirski
2018-01-27  7:55 ` [PATCH v5 08/12] x86: sanitize sycall table de-references under speculation Dan Williams
2018-01-28  9:36   ` Ingo Molnar
2018-01-27  7:56 ` [PATCH v5 09/12] vfs, fdtable: prevent bounds-check bypass via speculative execution Dan Williams
2018-01-27  7:56 ` [PATCH v5 10/12] kvm, x86: update spectre-v1 mitigation Dan Williams
     [not found] ` <151703971300.26578.1185595719337719486.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-01-27  7:56   ` [PATCH v5 11/12] nl80211: sanitize array index in parse_txq_params Dan Williams
2018-01-27  7:56     ` Dan Williams
2018-01-27  7:56 ` [PATCH v5 12/12] x86/spectre: report get_user mitigation for spectre_v1 Dan Williams
2018-01-28  9:50   ` Ingo Molnar
2018-01-29 22:05     ` Dan Williams
2018-01-31  8:07       ` Ingo Molnar
2018-02-01 20:23         ` Dan Williams
2018-02-01 20:23           ` Dan Williams
2018-01-27 18:52 ` [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Linus Torvalds
2018-01-27 18:52   ` Linus Torvalds
2018-01-27 19:26 ` Dan Williams
2018-01-27 19:26   ` Dan Williams

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=20180131142138.GA9233@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=alan@linux.intel.com \
    --cc=arjan.van.de.ven@intel.com \
    --cc=catalin.marinas@arm.com \
    --cc=cnovikov@lynx.com \
    --cc=dan.j.williams@intel.com \
    --cc=hpa@zytor.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=x86@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).