lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Aug 2022 11:42:19 -0700 From: Kees Cook <keescook@...omium.org> To: Andrzej Hajda <andrzej.hajda@...el.com> Cc: Rasmus Villemoes <linux@...musvillemoes.dk>, Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, linux-hardening@...r.kernel.org, Daniel Latypov <dlatypov@...gle.com>, Vitor Massaru Iha <vitor@...saru.org>, linux-kernel@...r.kernel.org Subject: Re: [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@...musvillemoes.dk> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@...el.com> > > Cc: Andrzej Hajda <andrzej.hajda@...el.com> > > Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org> > > Cc: Nick Desaulniers <ndesaulniers@...gle.com> > > Cc: linux-hardening@...r.kernel.org > > Signed-off-by: Kees Cook <keescook@...omium.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
Powered by blists - more mailing lists