Re: [PATCH] kernel: sys: fix potential Spectre v1

[Gustavo A. R. Silva]

On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote:


Oops, it seems I sent the wrong patch. The function would look like 
this:

#ifndef sanitize_index_nospec
inline bool sanitize_index_nospec(unsigned long *index,
                                   unsigned long size)
{
         if (*index >= size)
                 return false;
         *index = array_index_nospec(*index, size);

         return true;
}
#endif

I think this is fine in concept, we already do something similar in
mpls_label_ok(). Perhaps call it validate_index_nospec() since
validation is something that can fail, but sanitization in theory is
something that can always succeed.


OK. I got it.

However, the problem is the data type of the index. I expect you would
need to do this in a macro and use typeof() if you wanted this to be
generally useful, and also watch out for multiple usage of a macro
argument. Is it still worth it at that point?


Yeah. I think it is worth it. I'll work on this during the weekend and 
send a proper patch for review.

Thanks for the feedback.

BTW, I'm analyzing other cases, like the following:

bool foo(int x)
{
         if(!validate_index_nospec(&x))
                 return false;

         [...]

         return true;
}

int vulnerable(int x)
{
         if (!foo(x))
                 return -1;

         temp = array[x];

         [...]

}

Basically my doubt is how deep this barrier can be placed into the call 
chain in order to continue working.

Thanks
--
Gustavo