[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4gzbB-xESmmmpQ9+rt0swA+5y5pEOVNOYvroYQaP1kCfQ@mail.gmail.com>
Date: Sun, 7 Jan 2018 11:37:33 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-arch@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Netdev <netdev@...r.kernel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Thomas Gleixner <tglx@...utronix.de>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Elena Reshetova <elena.reshetova@...el.com>,
"Linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
dsj@...com
Subject: Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via
speculative execution
On Sun, Jan 7, 2018 at 1:09 AM, Greg KH <gregkh@...uxfoundation.org> wrote:
[..]
> Sorry for the confusion, no, I don't mean the "taint tracking", I mean
> the generic pattern of "speculative out of bounds access" that we are
> fixing here.
>
> Yes, as you mentioned before, there are tons of false-positives in the
> tree, as to find the real problems you have to show that userspace
> controls the access index. But if we have a generic pattern that can
> rewrite that type of logic into one where it does not matter at all
> (i.e. like the ebpf proposed changes), then it would not be an issue if
> they are false or not, we just rewrite them all to be safe.
>
> We need to find some way not only to fix these issues now (like you are
> doing with this series), but to prevent them from every coming back into
> the codebase again. It's that second part that we need to keep in the
> back of our minds here, while doing the first portion of this work.
I understand the goal, but I'm not sure any of our current annotation
mechanisms are suitable. We have:
__attribute__((noderef, address_space(x)))
...for the '__user' annotation and other pointers that must not be
de-referenced without a specific accessor. We also have:
__attribute__((bitwise))
...for values that should not be consumed directly without a specific
conversion like endian swapping.
The problem is that we need to see if a value derived from a userspace
controlled input is used to trigger a chain of dependent reads. As far
as I can see the annotation would need to be guided by taint analysis
to be useful, at which point we can just "annotate" the problem spot
with nospec_array_ptr(). Otherwise it seems the scope of a
"__nospec_array_index" annotation would have a low signal to noise
ratio.
Stopping speculation past a uacess_begin() boundary appears to handle
a wide swath of potential problems, and the rest likely needs taint
analysis, at least for now.
All that to say, yes, we need better tooling and infrastructure going forward.
Powered by blists - more mailing lists