On Sun, May 20, 2018 at 7:00 PM, Gustavo A. R. Silva
<gust...@embeddedor.com> wrote:
>
>
> On 05/20/2018 07:50 PM, Gustavo A. R. Silva wrote:
>>
>>
>>
>> On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote:
>>>>>
>>>>>
>>>>> #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.
>>>
>>
>> Dan,
>>
>> What do you think about this first draft:
>>
>> diff --git a/include/linux/nospec.h b/include/linux/nospec.h
>> index e791ebc..6154183 100644
>> --- a/include/linux/nospec.h
>> +++ b/include/linux/nospec.h
>> @@ -55,4 +55,16 @@ static inline unsigned long
>> array_index_mask_nospec(unsigned long index,
>> \
>> (typeof(_i)) (_i & _mask); \
>> })
>> +
>> +#define validate_index_nospec(index, size) \
>> +({ \
>> + typeof(index) *ptr = &(index); \
>> + typeof(size) _s = (size); \
>> + \
>> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \
>> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
>> + \
>> + *ptr >= _s ? false : \
>> + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \
>
>
> This actually should be:
>
> ((*ptr = array_index_nospec(*ptr, _s)) ? true : true);
>
Let's not use ternary conditionals at all to make this more readable.
