Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Tue, 9 Jan 2018, Josh Poimboeuf wrote: > On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote: > > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: > > > On Fri, 5 Jan 2018, Dan Williams wrote: > > > > > > [ ... snip ... ] > > >> Andi Kleen (1): > > >> x86, barrier: stop speculation for failed access_ok > > >> > > >> Dan Williams (13): > > >> x86: implement nospec_barrier() > > >> [media] uvcvideo: prevent bounds-check bypass via speculative > > >> execution > > >> carl9170: prevent bounds-check bypass via speculative execution > > >> p54: prevent bounds-check bypass via speculative execution > > >> qla2xxx: prevent bounds-check bypass via speculative execution > > >> cw1200: prevent bounds-check bypass via speculative execution > > >> Thermal/int340x: prevent bounds-check bypass via speculative > > >> execution > > >> ipv6: prevent bounds-check bypass via speculative execution > > >> ipv4: prevent bounds-check bypass via speculative execution > > >> vfs, fdtable: prevent bounds-check bypass via speculative execution > > >> net: mpls: prevent bounds-check bypass via speculative execution > > >> udf: prevent bounds-check bypass via speculative execution > > >> userns: prevent bounds-check bypass via speculative execution > > >> > > >> Mark Rutland (4): > > >> asm-generic/barrier: add generic nospec helpers > > >> Documentation: document nospec helpers > > >> arm64: implement nospec_ptr() > > >> arm: implement nospec_ptr() > > > > > > So considering the recent publication of [1], how come we all of a sudden > > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and > > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? > > > > > > Is this going to be handled in eBPF in some other way? > > > > > > Without that in place, and considering Jann Horn's paper, it would seem > > > like PTI doesn't really lock it down fully, right? > > > > Here is the latest (v3) bpf fix: > > > > https://patchwork.ozlabs.org/patch/856645/ > > > > I currently have v2 on my 'nospec' branch and will move that to v3 for > > the next update, unless it goes upstream before then. Daniel, I guess you're planning to send this still for 4.15? > That patch seems specific to CONFIG_BPF_SYSCALL. Is the bpf() syscall > the only attack vector? Or are there other ways to run bpf programs > that we should be worried about? Seems like Alexei is probably the only person in the whole universe who isn't CCed here ... let's fix that. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Thu, Jan 11, 2018 at 1:54 AM, Jiri Kosina wrote: > On Tue, 9 Jan 2018, Josh Poimboeuf wrote: > >> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote: >> > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: >> > > On Fri, 5 Jan 2018, Dan Williams wrote: >> > > >> > > [ ... snip ... ] >> > >> Andi Kleen (1): >> > >> x86, barrier: stop speculation for failed access_ok >> > >> >> > >> Dan Williams (13): >> > >> x86: implement nospec_barrier() >> > >> [media] uvcvideo: prevent bounds-check bypass via speculative >> > >> execution >> > >> carl9170: prevent bounds-check bypass via speculative execution >> > >> p54: prevent bounds-check bypass via speculative execution >> > >> qla2xxx: prevent bounds-check bypass via speculative execution >> > >> cw1200: prevent bounds-check bypass via speculative execution >> > >> Thermal/int340x: prevent bounds-check bypass via speculative >> > >> execution >> > >> ipv6: prevent bounds-check bypass via speculative execution >> > >> ipv4: prevent bounds-check bypass via speculative execution >> > >> vfs, fdtable: prevent bounds-check bypass via speculative >> > >> execution >> > >> net: mpls: prevent bounds-check bypass via speculative execution >> > >> udf: prevent bounds-check bypass via speculative execution >> > >> userns: prevent bounds-check bypass via speculative execution >> > >> >> > >> Mark Rutland (4): >> > >> asm-generic/barrier: add generic nospec helpers >> > >> Documentation: document nospec helpers >> > >> arm64: implement nospec_ptr() >> > >> arm: implement nospec_ptr() >> > > >> > > So considering the recent publication of [1], how come we all of a sudden >> > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and >> > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? >> > > >> > > Is this going to be handled in eBPF in some other way? >> > > >> > > Without that in place, and considering Jann Horn's paper, it would seem >> > > like PTI doesn't really lock it down fully, right? >> > >> > Here is the latest (v3) bpf fix: >> > >> > https://patchwork.ozlabs.org/patch/856645/ >> > >> > I currently have v2 on my 'nospec' branch and will move that to v3 for >> > the next update, unless it goes upstream before then. > > Daniel, I guess you're planning to send this still for 4.15? It's pending in the bpf.git tree: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc9 >> That patch seems specific to CONFIG_BPF_SYSCALL. Is the bpf() syscall >> the only attack vector? Or are there other ways to run bpf programs >> that we should be worried about? > > Seems like Alexei is probably the only person in the whole universe who > isn't CCed here ... let's fix that. He will be cc'd on v2 of this series which will be available later today.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On 01/11/2018 04:58 PM, Dan Williams wrote: > On Thu, Jan 11, 2018 at 1:54 AM, Jiri Kosina wrote: >> On Tue, 9 Jan 2018, Josh Poimboeuf wrote: >>> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote: On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: > On Fri, 5 Jan 2018, Dan Williams wrote: > > [ ... snip ... ] >> Andi Kleen (1): >> x86, barrier: stop speculation for failed access_ok >> >> Dan Williams (13): >> x86: implement nospec_barrier() >> [media] uvcvideo: prevent bounds-check bypass via speculative >> execution >> carl9170: prevent bounds-check bypass via speculative execution >> p54: prevent bounds-check bypass via speculative execution >> qla2xxx: prevent bounds-check bypass via speculative execution >> cw1200: prevent bounds-check bypass via speculative execution >> Thermal/int340x: prevent bounds-check bypass via speculative >> execution >> ipv6: prevent bounds-check bypass via speculative execution >> ipv4: prevent bounds-check bypass via speculative execution >> vfs, fdtable: prevent bounds-check bypass via speculative execution >> net: mpls: prevent bounds-check bypass via speculative execution >> udf: prevent bounds-check bypass via speculative execution >> userns: prevent bounds-check bypass via speculative execution >> >> Mark Rutland (4): >> asm-generic/barrier: add generic nospec helpers >> Documentation: document nospec helpers >> arm64: implement nospec_ptr() >> arm: implement nospec_ptr() > > So considering the recent publication of [1], how come we all of a sudden > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? > > Is this going to be handled in eBPF in some other way? > > Without that in place, and considering Jann Horn's paper, it would seem > like PTI doesn't really lock it down fully, right? Here is the latest (v3) bpf fix: https://patchwork.ozlabs.org/patch/856645/ I currently have v2 on my 'nospec' branch and will move that to v3 for the next update, unless it goes upstream before then. >> >> Daniel, I guess you're planning to send this still for 4.15? > > It's pending in the bpf.git tree: > > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc9 Sorry for the delay, just noticed the question now since not on Cc either: It made it into in DaveM's tree already and part of his latest pull-req to Linus. >>> That patch seems specific to CONFIG_BPF_SYSCALL. Is the bpf() syscall >>> the only attack vector? Or are there other ways to run bpf programs >>> that we should be worried about? >> >> Seems like Alexei is probably the only person in the whole universe who >> isn't CCed here ... let's fix that. > > He will be cc'd on v2 of this series which will be available later today.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
Dan Williams writes:
> Quoting Mark's original RFC:
>
> "Recently, Google Project Zero discovered several classes of attack
> against speculative execution. One of these, known as variant-1, allows
> explicit bounds checks to be bypassed under speculation, providing an
> arbitrary read gadget. Further details can be found on the GPZ blog [1]
> and the Documentation patch in this series."
>
> This series incorporates Mark Rutland's latest api and adds the x86
> specific implementation of nospec_barrier. The
> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
> wide analysis performed by Elena Reshetova to address static analysis
> reports where speculative execution on a userspace controlled value
> could bypass a bounds check. The patches address a precondition for the
> attack discussed in the Spectre paper [2].
Please expand this.
It is not clear what the static analysis is looking for. Have a clear
description of what is being fixed is crucial for allowing any of these
changes.
For the details given in the change description what I read is magic
changes because a magic process says this code is vunlerable.
Given the similarities in the code that is being patched to many other
places in the kernel it is not at all clear that this small set of
changes is sufficient for any purpose.
> A consideration worth noting for reviewing these patches is to weigh the
> dramatic cost of being wrong about whether a given report is exploitable
> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
> lets make the bar for applying these patches be "can you prove that the
> bounds check bypass is *not* exploitable". Consider that the Spectre
> paper reports one example of a speculation window being ~180 cycles.
> Note that there is also a proposal from Linus, array_access [3], that
> attempts to quash speculative execution past a bounds check without
> introducing an lfence instruction. That may be a future optimization
> possibility that is compatible with this api, but it would appear to
> need guarantees from the compiler that it is not clear the kernel can
> rely on at this point. It is also not clear that it would be a
> significant performance win vs lfence.
It is also not clear that these changes fix anything, or are in any
sense correct for the problem they are trying to fix as the problem
is not clearly described.
In at least one place (mpls) you are patching a fast path. Compile out
or don't load mpls by all means. But it is not acceptable to change the
fast path without even considering performance.
So because the description sucks, and the due diligence is not there.
Nacked-by: "Eric W. Biederman"
to the series.
>
> These patches also will also be available via the 'nospec' git branch
> here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
>
> [1]:
> https://googleprojectzero.blogspot.co.uk/2018/01/reading-privileged-memory-with-side.html
> [2]: https://spectreattack.com/spectre.pdf
> [3]: https://marc.info/?l=linux-kernel&m=151510446027625&w=2
>
> ---
>
> Andi Kleen (1):
> x86, barrier: stop speculation for failed access_ok
>
> Dan Williams (13):
> x86: implement nospec_barrier()
> [media] uvcvideo: prevent bounds-check bypass via speculative execution
> carl9170: prevent bounds-check bypass via speculative execution
> p54: prevent bounds-check bypass via speculative execution
> qla2xxx: prevent bounds-check bypass via speculative execution
> cw1200: prevent bounds-check bypass via speculative execution
> Thermal/int340x: prevent bounds-check bypass via speculative execution
> ipv6: prevent bounds-check bypass via speculative execution
> ipv4: prevent bounds-check bypass via speculative execution
> vfs, fdtable: prevent bounds-check bypass via speculative execution
> net: mpls: prevent bounds-check bypass via speculative execution
> udf: prevent bounds-check bypass via speculative execution
> userns: prevent bounds-check bypass via speculative execution
>
> Mark Rutland (4):
> asm-generic/barrier: add generic nospec helpers
> Documentation: document nospec helpers
> arm64: implement nospec_ptr()
> arm: implement nospec_ptr()
>
> Documentation/speculation.txt | 166
>
> arch/arm/include/asm/barrier.h | 75 +
> arch/arm64/include/asm/barrier.h | 55 +++
> arch/x86/include/asm/barrier.h |6 +
> arch/x86/include/asm/uaccess.h | 17 ++
> drivers/media/usb/uvc/uvc_v4l2.c |7 +
> drivers/net/wireless/ath/carl9170/main.c |6 -
> drivers/net/wireless/intersil/p54/main.c |8 +
> drivers/net/wireless/st/cw1200/sta.c | 10 +
> drivers/net/wireless/st/cw1200/wsm.h |4
> drivers/scsi/qla2xxx/qla_mr.c
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman wrote:
> Dan Williams writes:
>
>> Quoting Mark's original RFC:
>>
>> "Recently, Google Project Zero discovered several classes of attack
>> against speculative execution. One of these, known as variant-1, allows
>> explicit bounds checks to be bypassed under speculation, providing an
>> arbitrary read gadget. Further details can be found on the GPZ blog [1]
>> and the Documentation patch in this series."
>>
>> This series incorporates Mark Rutland's latest api and adds the x86
>> specific implementation of nospec_barrier. The
>> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
>> wide analysis performed by Elena Reshetova to address static analysis
>> reports where speculative execution on a userspace controlled value
>> could bypass a bounds check. The patches address a precondition for the
>> attack discussed in the Spectre paper [2].
>
> Please expand this.
>
> It is not clear what the static analysis is looking for. Have a clear
> description of what is being fixed is crucial for allowing any of these
> changes.
>
> For the details given in the change description what I read is magic
> changes because a magic process says this code is vunlerable.
Yes, that was my first reaction to the patches as well, I try below to
add some more background and guidance, but in the end these are static
analysis reports across a wide swath of sub-systems. It's going to
take some iteration with domain experts to improve the patch
descriptions, and that's the point of this series, to get the better
trained eyes from the actual sub-system owners to take a look at these
reports.
For example, I'm looking for feedback like what Srinivas gave where he
identified that the report is bogus, the branch condition can not be
seeded with bad values in that path. Be like Srinivas.
> Given the similarities in the code that is being patched to many other
> places in the kernel it is not at all clear that this small set of
> changes is sufficient for any purpose.
I find this assertion absurd, when in the past have we as kernel
developers ever been handed a static analysis report and then
questioned why the static analysis did not flag other call sites
before first reviewing the ones it did find?
>> A consideration worth noting for reviewing these patches is to weigh the
>> dramatic cost of being wrong about whether a given report is exploitable
>> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
>> lets make the bar for applying these patches be "can you prove that the
>> bounds check bypass is *not* exploitable". Consider that the Spectre
>> paper reports one example of a speculation window being ~180 cycles.
>
>
>> Note that there is also a proposal from Linus, array_access [3], that
>> attempts to quash speculative execution past a bounds check without
>> introducing an lfence instruction. That may be a future optimization
>> possibility that is compatible with this api, but it would appear to
>> need guarantees from the compiler that it is not clear the kernel can
>> rely on at this point. It is also not clear that it would be a
>> significant performance win vs lfence.
>
> It is also not clear that these changes fix anything, or are in any
> sense correct for the problem they are trying to fix as the problem
> is not clearly described.
I'll try my best. I don't have first hand knowledge of how the static
analyzer is doing this job, and I don't think it matters for
evaluating these reports. I'll give you my thoughts on how I would
handle one of these reports if it flagged one of the sub-systems I
maintain.
Start with the example from the Spectre paper:
if (x < array1_size)
y = array2[array1[x] * 256];
In all the patches 'x' and 'array1' are called out explicitly. For example:
net: mpls: prevent bounds-check bypass via speculative execution
Static analysis reports that 'index' may be a user controlled value that
is used as a data dependency reading 'rt' from the 'platform_label'
array...
So the first thing to review is whether the analyzer got it wrong and
'x' is not arbitrarily controllable by userspace to cause speculation
outside of the checked bounds. Be like Srinivas. The next step is to
ask whether the code can be refactored so that 'x' is sanitized
earlier in the call stack, especially if the nospec_array_ptr() lands
in a hot path. The next aspect that I expect most would be tempted to
go check is whether 'array2[array1[x]]' occurs later in the code
stream, but with speculation windows being architecture dependent and
potentially large (~180 cycles in one case says the paper) I submit
that we should err on the side of caution and not guess if that second
dependent read has been emitted somewhere in the instruction stream.
> In at least one place (mpls) you are patching a fast path. Compile out
> or don't load mpls by all means. But it is not acceptable to change the
> fast path without even
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
Le 01/05/18 à 17:09, Dan Williams a écrit :
> Quoting Mark's original RFC:
>
> "Recently, Google Project Zero discovered several classes of attack
> against speculative execution. One of these, known as variant-1, allows
> explicit bounds checks to be bypassed under speculation, providing an
> arbitrary read gadget. Further details can be found on the GPZ blog [1]
> and the Documentation patch in this series."
>
> This series incorporates Mark Rutland's latest api and adds the x86
> specific implementation of nospec_barrier. The
> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
> wide analysis performed by Elena Reshetova to address static analysis
> reports where speculative execution on a userspace controlled value
> could bypass a bounds check. The patches address a precondition for the
> attack discussed in the Spectre paper [2].
>
> A consideration worth noting for reviewing these patches is to weigh the
> dramatic cost of being wrong about whether a given report is exploitable
> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
> lets make the bar for applying these patches be "can you prove that the
> bounds check bypass is *not* exploitable". Consider that the Spectre
> paper reports one example of a speculation window being ~180 cycles.
>
> Note that there is also a proposal from Linus, array_access [3], that
> attempts to quash speculative execution past a bounds check without
> introducing an lfence instruction. That may be a future optimization
> possibility that is compatible with this api, but it would appear to
> need guarantees from the compiler that it is not clear the kernel can
> rely on at this point. It is also not clear that it would be a
> significant performance win vs lfence.
>
> These patches also will also be available via the 'nospec' git branch
> here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
Although I suppose -stable and distribution maintainers will keep a
close eye on these patches, is there a particular reason why they don't
include the relevant CVE number in their commit messages?
It sounds like Coverity was used to produce these patches? If so, is
there a plan to have smatch (hey Dan) or other open source static
analysis tool be possibly enhanced to do a similar type of work?
Thanks!
--
Florian
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
It sounds like Coverity was used to produce these patches? If so, is there a plan to have smatch (hey Dan) or other open source static analysis tool be possibly enhanced to do a similar type of work? I'd love for that to happen; the tricky part is being able to have even a sort of sensible concept of "trusted" vs "untrusted" value... if you look at a very small window of code, that does not work well; you likely need to even look (as tool) across .c file boundaries
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams wrote:
> Quoting Mark's original RFC:
>
> "Recently, Google Project Zero discovered several classes of attack
> against speculative execution. One of these, known as variant-1, allows
> explicit bounds checks to be bypassed under speculation, providing an
> arbitrary read gadget. Further details can be found on the GPZ blog [1]
> and the Documentation patch in this series."
>
> This series incorporates Mark Rutland's latest api and adds the x86
> specific implementation of nospec_barrier. The
> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
> wide analysis performed by Elena Reshetova to address static analysis
> reports where speculative execution on a userspace controlled value
> could bypass a bounds check. The patches address a precondition for the
> attack discussed in the Spectre paper [2].
>
> A consideration worth noting for reviewing these patches is to weigh the
> dramatic cost of being wrong about whether a given report is exploitable
> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
> lets make the bar for applying these patches be "can you prove that the
> bounds check bypass is *not* exploitable". Consider that the Spectre
> paper reports one example of a speculation window being ~180 cycles.
>
> Note that there is also a proposal from Linus, array_access [3], that
> attempts to quash speculative execution past a bounds check without
> introducing an lfence instruction. That may be a future optimization
> possibility that is compatible with this api, but it would appear to
> need guarantees from the compiler that it is not clear the kernel can
> rely on at this point. It is also not clear that it would be a
> significant performance win vs lfence.
>
> These patches also will also be available via the 'nospec' git branch
> here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
It appears that git.kernel.org has not mirrored out the new branch. In
the meantime here's an alternative location:
https://github.com/djbw/linux.git nospec
If there are updates to these patches they will appear in nospec-v2,
nospec-v3, etc... branches.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Sat, Jan 6, 2018 at 11:37 AM, Dan Williams wrote:
> On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams wrote:
>> Quoting Mark's original RFC:
>>
>> "Recently, Google Project Zero discovered several classes of attack
>> against speculative execution. One of these, known as variant-1, allows
>> explicit bounds checks to be bypassed under speculation, providing an
>> arbitrary read gadget. Further details can be found on the GPZ blog [1]
>> and the Documentation patch in this series."
>>
>> This series incorporates Mark Rutland's latest api and adds the x86
>> specific implementation of nospec_barrier. The
>> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
>> wide analysis performed by Elena Reshetova to address static analysis
>> reports where speculative execution on a userspace controlled value
>> could bypass a bounds check. The patches address a precondition for the
>> attack discussed in the Spectre paper [2].
>>
>> A consideration worth noting for reviewing these patches is to weigh the
>> dramatic cost of being wrong about whether a given report is exploitable
>> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
>> lets make the bar for applying these patches be "can you prove that the
>> bounds check bypass is *not* exploitable". Consider that the Spectre
>> paper reports one example of a speculation window being ~180 cycles.
>>
>> Note that there is also a proposal from Linus, array_access [3], that
>> attempts to quash speculative execution past a bounds check without
>> introducing an lfence instruction. That may be a future optimization
>> possibility that is compatible with this api, but it would appear to
>> need guarantees from the compiler that it is not clear the kernel can
>> rely on at this point. It is also not clear that it would be a
>> significant performance win vs lfence.
>>
>> These patches also will also be available via the 'nospec' git branch
>> here:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
>
> It appears that git.kernel.org has not mirrored out the new branch. In
> the meantime here's an alternative location:
>
> https://github.com/djbw/linux.git nospec
>
> If there are updates to these patches they will appear in nospec-v2,
> nospec-v3, etc... branches.
For completeness I appended the bpf fix [1] to the git branch.
https://lwn.net/Articles/743288/
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote: > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman > wrote: > > In at least one place (mpls) you are patching a fast path. Compile out > > or don't load mpls by all means. But it is not acceptable to change the > > fast path without even considering performance. > > Performance matters greatly, but I need help to identify a workload > that is representative for this fast path to see what, if any, impact > is incurred. Even better is a review that says "nope, 'index' is not > subject to arbitrary userspace control at this point, drop the patch." I think we're focussing a little too much on pure userspace. That is, we should be saying under the attackers control. Inbound network packets could equally be under the attackers control. Sure, userspace is the most direct and highest bandwidth one, but I think we should treat all (kernel) external values with the same paranoia.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Mon, 8 Jan 2018 11:08:36 +0100 Peter Zijlstra wrote: > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote: > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman > > wrote: > > > In at least one place (mpls) you are patching a fast path. Compile out > > > or don't load mpls by all means. But it is not acceptable to change the > > > fast path without even considering performance. > > > > Performance matters greatly, but I need help to identify a workload > > that is representative for this fast path to see what, if any, impact > > is incurred. Even better is a review that says "nope, 'index' is not > > subject to arbitrary userspace control at this point, drop the patch." > > I think we're focussing a little too much on pure userspace. That is, we > should be saying under the attackers control. Inbound network packets > could equally be under the attackers control. Inbound network packets don't come with a facility to read back and do cache timimg. For the more general case, timing attacks on network activity are not exactly new, and you have to mitigate them in user space because most of them are about how many instructions you execute on each path. The ancient classic being telling if a user exists by seeing if the password was actually checked. Alan
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Mon, Jan 08, 2018 at 11:43:42AM +, Alan Cox wrote: > On Mon, 8 Jan 2018 11:08:36 +0100 > Peter Zijlstra wrote: > > > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote: > > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman > > > wrote: > > > > In at least one place (mpls) you are patching a fast path. Compile out > > > > or don't load mpls by all means. But it is not acceptable to change the > > > > fast path without even considering performance. > > > > > > Performance matters greatly, but I need help to identify a workload > > > that is representative for this fast path to see what, if any, impact > > > is incurred. Even better is a review that says "nope, 'index' is not > > > subject to arbitrary userspace control at this point, drop the patch." > > > > I think we're focussing a little too much on pure userspace. That is, we > > should be saying under the attackers control. Inbound network packets > > could equally be under the attackers control. > > Inbound network packets don't come with a facility to read back and do > cache timimg. But could they not be used in conjunction with a local task to prime the stuff?
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On 01/05/18 22:30, Dan Williams wrote: On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman wrote: Please expand this. It is not clear what the static analysis is looking for. Have a clear description of what is being fixed is crucial for allowing any of these changes. For the details given in the change description what I read is magic changes because a magic process says this code is vulnerable. Yes, that was my first reaction to the patches as well, I try below to add some more background and guidance, but in the end these are static analysis reports across a wide swath of sub-systems. It's going to take some iteration with domain experts to improve the patch descriptions, and that's the point of this series, to get the better trained eyes from the actual sub-system owners to take a look at these reports. More information about what the static analysis is looking for would definitely be welcome. Additionally, since the analysis tool is not publicly available, how are authors of new kernel code assumed to verify whether or not their code needs to use nospec_array_ptr()? How are reviewers of kernel code assumed to verify whether or not nospec_array_ptr() is missing where it should be used? Since this patch series only modifies the upstream kernel, how will out-of-tree drivers be fixed, e.g. the nVidia driver and the Android drivers? Thanks, Bart.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
* Alan Cox wrote: > On Mon, 8 Jan 2018 11:08:36 +0100 > Peter Zijlstra wrote: > > > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote: > > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman > > > wrote: > > > > In at least one place (mpls) you are patching a fast path. Compile out > > > > or don't load mpls by all means. But it is not acceptable to change the > > > > fast path without even considering performance. > > > > > > Performance matters greatly, but I need help to identify a workload > > > that is representative for this fast path to see what, if any, impact > > > is incurred. Even better is a review that says "nope, 'index' is not > > > subject to arbitrary userspace control at this point, drop the patch." > > > > I think we're focussing a little too much on pure userspace. That is, we > > should be saying under the attackers control. Inbound network packets > > could equally be under the attackers control. > > Inbound network packets don't come with a facility to read back and do > cache timimg. [...] But the reply packets can be measured on the sending side, and the total delay timing would thus carry the timing information. Yes, a lot of noise gets added that way if we think 'packet goes through the Internet' - but with gigabit local network access or even through localhost access a lot of noise can be removed as well. It's not as dangerous as a near instantaneous local attack, but 'needs a day of runtime to brute-force through localhost or 10GigE' is still worrying in many real-world security contexts. So I concur with Peter that we should generally consider making all of our responses to external data (maybe with the exception of pigeon post messages) Spectre-safe. Thanks, Ingo
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: > On Fri, 5 Jan 2018, Dan Williams wrote: > > [ ... snip ... ] >> Andi Kleen (1): >> x86, barrier: stop speculation for failed access_ok >> >> Dan Williams (13): >> x86: implement nospec_barrier() >> [media] uvcvideo: prevent bounds-check bypass via speculative execution >> carl9170: prevent bounds-check bypass via speculative execution >> p54: prevent bounds-check bypass via speculative execution >> qla2xxx: prevent bounds-check bypass via speculative execution >> cw1200: prevent bounds-check bypass via speculative execution >> Thermal/int340x: prevent bounds-check bypass via speculative execution >> ipv6: prevent bounds-check bypass via speculative execution >> ipv4: prevent bounds-check bypass via speculative execution >> vfs, fdtable: prevent bounds-check bypass via speculative execution >> net: mpls: prevent bounds-check bypass via speculative execution >> udf: prevent bounds-check bypass via speculative execution >> userns: prevent bounds-check bypass via speculative execution >> >> Mark Rutland (4): >> asm-generic/barrier: add generic nospec helpers >> Documentation: document nospec helpers >> arm64: implement nospec_ptr() >> arm: implement nospec_ptr() > > So considering the recent publication of [1], how come we all of a sudden > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? > > Is this going to be handled in eBPF in some other way? > > Without that in place, and considering Jann Horn's paper, it would seem > like PTI doesn't really lock it down fully, right? Here is the latest (v3) bpf fix: https://patchwork.ozlabs.org/patch/856645/ I currently have v2 on my 'nospec' branch and will move that to v3 for the next update, unless it goes upstream before then.
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Fri, 5 Jan 2018, Dan Williams wrote: [ ... snip ... ] > Andi Kleen (1): > x86, barrier: stop speculation for failed access_ok > > Dan Williams (13): > x86: implement nospec_barrier() > [media] uvcvideo: prevent bounds-check bypass via speculative execution > carl9170: prevent bounds-check bypass via speculative execution > p54: prevent bounds-check bypass via speculative execution > qla2xxx: prevent bounds-check bypass via speculative execution > cw1200: prevent bounds-check bypass via speculative execution > Thermal/int340x: prevent bounds-check bypass via speculative execution > ipv6: prevent bounds-check bypass via speculative execution > ipv4: prevent bounds-check bypass via speculative execution > vfs, fdtable: prevent bounds-check bypass via speculative execution > net: mpls: prevent bounds-check bypass via speculative execution > udf: prevent bounds-check bypass via speculative execution > userns: prevent bounds-check bypass via speculative execution > > Mark Rutland (4): > asm-generic/barrier: add generic nospec helpers > Documentation: document nospec helpers > arm64: implement nospec_ptr() > arm: implement nospec_ptr() So considering the recent publication of [1], how come we all of a sudden don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? Is this going to be handled in eBPF in some other way? Without that in place, and considering Jann Horn's paper, it would seem like PTI doesn't really lock it down fully, right? [1] https://bugs.chromium.org/p/project-zero/issues/attachmentText?aid=287305 -- Jiri Kosina SUSE Labs
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote: > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: > > On Fri, 5 Jan 2018, Dan Williams wrote: > > > > [ ... snip ... ] > >> Andi Kleen (1): > >> x86, barrier: stop speculation for failed access_ok > >> > >> Dan Williams (13): > >> x86: implement nospec_barrier() > >> [media] uvcvideo: prevent bounds-check bypass via speculative > >> execution > >> carl9170: prevent bounds-check bypass via speculative execution > >> p54: prevent bounds-check bypass via speculative execution > >> qla2xxx: prevent bounds-check bypass via speculative execution > >> cw1200: prevent bounds-check bypass via speculative execution > >> Thermal/int340x: prevent bounds-check bypass via speculative > >> execution > >> ipv6: prevent bounds-check bypass via speculative execution > >> ipv4: prevent bounds-check bypass via speculative execution > >> vfs, fdtable: prevent bounds-check bypass via speculative execution > >> net: mpls: prevent bounds-check bypass via speculative execution > >> udf: prevent bounds-check bypass via speculative execution > >> userns: prevent bounds-check bypass via speculative execution > >> > >> Mark Rutland (4): > >> asm-generic/barrier: add generic nospec helpers > >> Documentation: document nospec helpers > >> arm64: implement nospec_ptr() > >> arm: implement nospec_ptr() > > > > So considering the recent publication of [1], how come we all of a sudden > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? > > > > Is this going to be handled in eBPF in some other way? > > > > Without that in place, and considering Jann Horn's paper, it would seem > > like PTI doesn't really lock it down fully, right? > > Here is the latest (v3) bpf fix: > > https://patchwork.ozlabs.org/patch/856645/ > > I currently have v2 on my 'nospec' branch and will move that to v3 for > the next update, unless it goes upstream before then. That patch seems specific to CONFIG_BPF_SYSCALL. Is the bpf() syscall the only attack vector? Or are there other ways to run bpf programs that we should be worried about? -- Josh
