lkml.org 
[lkml]   [2022]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] overflow: Allow mixed type arguments
On Tue, Aug 30, 2022 at 08:40:25AM +0200, Andrzej Hajda wrote:
>
>
> On 29.08.2022 23:45, Kees Cook wrote:
> > When the check_[op]_overflow() helpers were introduced, all arguments were
> > required to be the same type to make the fallback macros simpler. However,
> > once the fallback macros were removed[1], it is fine to allow mixed
> > types, which makes using the helpers much more useful, as they can be
> > used to test for type-based overflows (e.g. adding two large ints but
> > storing into a u8), as would be handy in the drm core[2].
> >
> > Remove the restriction, and add additional self-tests that exercise some
> > of the mixed-type overflow cases.
> >
> > [1] https://git.kernel.org/linus/4eb6bd55cfb22ffc20652732340c4962f3ac9a91
> > [2] https://lore.kernel.org/lkml/20220824084514.2261614-2-gwan-gyeong.mun@intel.com
> >
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: linux-hardening@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > v2: change names to "type1_type2__output-type" for better readability (Rasmus)
> > v1: https://lore.kernel.org/lkml/20220829204729.3409270-1-keescook@chromium.org
> > ---
> > include/linux/overflow.h | 6 ----
> > lib/overflow_kunit.c | 77 +++++++++++++++++++++++++++++-----------
> > 2 files changed, 57 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> > index 0eb3b192f07a..ad692fb11bf3 100644
> > --- a/include/linux/overflow.h
> > +++ b/include/linux/overflow.h
> > @@ -63,8 +63,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
> > typeof(a) __a = (a); \
> > typeof(b) __b = (b); \
> > typeof(d) __d = (d); \
> > - (void) (&__a == &__b); \
> > - (void) (&__a == __d); \
> > __builtin_add_overflow(__a, __b, __d); \
> > }))
>
> Is there a reason to keep all local vars?
> Why not:
> #define check_add_overflow(a, b, d)
> __must_check_overflow(__builtin_add_overflow((a), (b), (d)))

Excellent point! Those were there to avoid side-effects when doing the
type comparisons in the macro, which are being removed too. :P

--
Kees Cook

\
 
 \ /
  Last update: 2022-08-30 20:42    [W:0.068 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site