Messages in this thread | | | Date | Mon, 24 Jan 2022 13:13:20 -0800 | From | Kees Cook <> | 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
| |