Re: [PATCH] kernel/sys.c: fix possible spectre-v1 in do_prlimit()

[Cyrill Gorcunov]

On Thu, May 30, 2019 at 01:45:16PM +0800, Dianzhang Chen wrote:
>
> Though syscall `getrlimit` , it seems not works after check_prlimit_permission.
> 
> And the speculation windows are large, as said[1]:
> >> Can the speculation proceed past the task_lock()?  Or is the policy to
> >> ignore such happy happenstances even if they are available?
> >
> > Locks are not in the way of speculation. Speculation has almost no limits
> > except serializing instructions. At least they respect the magic AND
> > limitation in array_index_nospec().
> 
> [1] https://do-db2.lkml.org/lkml/2018/5/15/1056

Please, stop top-posting, it trashes conversation context. You miss the point:
before bpu hits misprediction we've a number of branches, second in case of
misprediction the kernel's stack value is cached, so I'm not convinced at all
that teaching bpu and read the cache is easy here (or possible at all). Thus,
the final solution is up to maintainers. Another reason why I complaining about
the patch -- it is not the patch body, as I said I'm fine with it, but the patch
title: it implies the fix should go in stable kernels, that's what I dont agree
with. But again, I'm not a maintainer and might be wrong.

> 
> On Wed, May 29, 2019 at 8:18 PM Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote:
> >
> > On Wed, May 29, 2019 at 10:39:52AM +0800, Dianzhang Chen wrote:
> > > Hi,
> > >
> > > Although when detect it is misprediction and drop the execution, but
> > > it can not drop all the effects of speculative execution, like the
> > > cache state. During the speculative execution, the:
> > >
> > >
> > > rlim = tsk->signal->rlim + resource;    // use resource as index
> > >
> > > ...
> > >
> > >             *old_rlim = *rlim;
> > >
> > >
> > > may read some secret data into cache.
> > >
> > > and then the attacker can use side-channel attack to find out what the
> > > secret data is.
> >
> > This code works after check_prlimit_permission call, which means you already
> > should have a permission granted. And you implies that misprediction gonna
> > be that deep which involves a number of calls/read/writes/jumps/locks-rb-wb-flushes
> > and a bunch or other instructions, moreover all conditions are "mispredicted".
> > This is very bold and actually unproved claim!
> >
> > Note that I pointed the patch is fine in cleanup context but seriously I
> > don't see how this all can be exploitable in sense of spectre.
> 

	Cyrill