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

Michael Ellerman [off-list ref] writes:

Nathan Lynch [off-list ref] writes:

quoted

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.

quoted

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.