[PATCH] powerpc/kernel: Fix potential spectre v1 in syscall

[Nathan Lynch]

Michael Ellerman <mpe at ellerman.id.au> writes:
> Nathan Lynch <nathanl at linux.ibm.com> writes:
>>
>> 1. The patch sanitizes 'nargs' immediately before the call to memset(),
>>    but shouldn't that happen before 'nargs' is used as an input to
>>    copy_from_user()?
>
> I think the reasoning is that there's no way to exploit an out of bounds
> value using copy_from_user(). But it's much easier to reason about if we
> just do the sanitisation up front.
>
>> 2. If 'nargs' needs this treatment, then why wouldn't the user-supplied
>>    'nret' and 'token' need them as well? 'nret' is used to index the
>>    same array as 'nargs'. And at least conceptually, 'token' is used to
>>    index a data structure (xarray) with array-like semantics (to be
>>    fair, this is a relatively recent development and was not the case
>>    when this change was submitted).
>     
> I don't know exactly what smatch looks for when trying to detect these,
> but I suspect it's a plain array access. Not sure why it doesn't
> complain about nret, but I think it would be good to sanitise it as
> well.

Agreed. I'm sending a new patch that does this.

> token is different, at least in the above code, because it's not bounds
> checked, so there's no bounds check to bypass.

Right.

> Though maybe there is one inside the rtas lookup code that should be
> masked.

In rtas_function_token()? I think it's OK... the handles passed to it
are always build-time constants that are supposed to be within the
bounds of the function table.