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

[Gustavo A. R. Silva]

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);           \
+})
 #endif /* _LINUX_NOSPEC_H */

Thanks
--
Gustavo