lkml.org 
[lkml]   [2018]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1
    On Tue, Apr 24, 2018 at 12:35:00PM +0300, Dan Carpenter wrote:
    > On Mon, Apr 23, 2018 at 03:24:55PM -0300, Mauro Carvalho Chehab wrote:
    > > Em Mon, 23 Apr 2018 12:38:03 -0500
    > > "Gustavo A. R. Silva" <gustavo@embeddedor.com> escreveu:

    > > > @@ -875,6 +876,7 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
    > > > if (f->index >= ARRAY_SIZE(format))
    > > > return -EINVAL;
    > > >
    > > > + f->index = array_index_nospec(f->index, ARRAY_SIZE(format));
    > >
    > > Please enlighten me: how do you think this could be exploited?

    TL;DR: read the papers [1] & [2]

    I suspect you didn't get the gist of Spectre V1 [1], let me explain:

    Suppose userspace provides f->index > ARRAY_SIZE(format), and we predict
    the branch to -EINVAL to not be taken.

    Then the CPU _WILL_ load (out of bounds) format[f->index] into
    f->pixelformat and continue onwards to use this bogus value, all the way
    until it figures out the branch was mis-predicted.

    Once it figures out the mispredict, it will throw away the state and
    start over at the condition site. So far, so basic.

    The thing is, is will not (and cannot) throw away all state. Suppose our
    speculation continues into v4l_fill_fmtdesc() and that switch there is
    compiled as another array lookup, it will then feed our f->pixelformat
    (which contains random kernel memory) into that array to find the
    requested descr pointer.

    Now, imagine userspace having flushed cache on the descr pointer array,
    having trained the branch predictor to mis-predict the branch (see
    branchscope paper [2]) and doing that out-of-bounds ioctl().

    It can then speculative do the out-of-bounds array access, followed by
    the desc array load, then figure out it was wrong and redo.

    Then usespace probes which part of the descr[] array is now in cache and
    from that it can infer the initial out-of-bound value.

    So while format[] is static and bound, it can read random kernel memory
    up to format+4g, including your crypto keys.

    As far as V1 goes, this is actually a fairly solid exploit candidate. No
    false positive about it.

    Now kernel policy is to kill any and all speculation on user controlled
    array indexing such that we don't have to go look for subsequent side
    channels (the above cache side channel is the one described in the
    Spectre paper and by far the easiest, but there are other possible side
    channels) and we simply don't want to worry about it.

    So even from that pov, the proposed patch is good.


    [1] https://spectreattack.com/spectre.pdf
    [2] www.cs.ucr.edu/~nael/pubs/asplos18.pdf

    \
     
     \ /
      Last update: 2018-04-24 12:37    [W:2.402 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site