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: Mon, 24 Jan 2022 13:13:20 -0800 From: Kees Cook <keescook@...omium.org> To: Rasmus Villemoes <linux@...musvillemoes.dk> Cc: "Gustavo A . R . Silva" <gustavoars@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>, Nick Desaulniers <ndesaulniers@...gle.com>, Leon Romanovsky <leon@...nel.org>, Keith Busch <kbusch@...nel.org>, Len Baker <len.baker@....com>, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH 1/2] overflow: Implement size_t saturating arithmetic helpers *thread necromancy* On Tue, Sep 21, 2021 at 08:51:53AM +0200, Rasmus Villemoes wrote: > Not that I can see that the __must_check matters much for these anyway; > if anybody does > > size_mul(foo, bar); > > that's just a statement with no side effects, so probably the compiler > would warn anyway, or at least nobody can then go on to do anything > "wrong". Unlike the check_*_overflow(), which have the (possibly > wrapped) result in a output-pointer and the "did it overflow" as the > return value, so you can do > > check_mul_overflow(a, b, &d); > do_stuff_with(d); > > were it not for the __must_check wrapper. > > [Reminder: __must_check is a bit of a misnomer, the attribute is really > warn_unused_result, and there's no requirement that the result is part > of the controlling expression of an if() or while() - just passing the > result on directly to some other function counts as a "use", which is > indeed what we do with the size wrappers.] What I'd really like is a "store this in a size_t" check to catch dumb storage size problems (or related overflows). In other words: size_t big1 = 2147483647; size_t big2 = 2147483647; /* Doesn't overflow, but 4611686014132420609 becomes a 1 for int */ int size = size_mul(big1, big2); ... ptr = kmalloc(size, GFP_KERNEL); /* Allocates a 1 instead... */ I could solve this but removing the assignment, but then I can't compose calls: static inline size_t __size_mul(size_t f1, size_t f2) { size_t out; if (check_mul_overflow(f1, f2, &out)) out = SIZE_MAX; return out; } #define size_mul(f1, f2, out) do { \ BUILD_BUG_ON(!__same_type(out, size_t)); \ out = __size_mul(f1, f2); \ } while (0) i.e. now I can't do size_mul(size_add(...), size_add(...)) Better would be to build the entire kernel with -Wconversion. :) -- Kees Cook
Powered by blists - more mailing lists