Messages in this thread | | | From | Kees Cook <> | Date | Mon, 30 Apr 2018 12:02:14 -0700 | Subject | Re: [PATCH 2/2] mm: Add kvmalloc_ab_c and kvzalloc_struct |
| |
On Sun, Apr 29, 2018 at 1:30 PM, Matthew Wilcox <willy@infradead.org> wrote: > On Sun, Apr 29, 2018 at 09:59:27AM -0700, Kees Cook wrote: >> Did this ever happen? > > Not yet. I brought it up at LSFMM, and I'll repost the patches soon. > >> I'd also like to see kmalloc_array_3d() or >> something that takes three size arguments. We have a lot of this >> pattern too: >> >> kmalloc(sizeof(foo) * A * B, gfp...) >> >> And we could turn that into: >> >> kmalloc_array_3d(sizeof(foo), A, B, gfp...) > > Are either of A or B constant? Because if so, we could just use > kmalloc_array. If not, then kmalloc_array_3d becomes a little more > expensive than kmalloc_array because we have to do a divide at runtime > instead of compile-time. that's still better than allocating too few > bytes, of course.
Yeah, getting the order of the division is nice. Some thoughts below...
> > I'm wondering how far down the abc + ab + ac + bc + d rabbit-hole we're > going to end up going. As far as we have to, I guess.
Well, the common patterns I've seen so far are:
a ab abc a + bc ab + cd
For any longer multiplications, I've only found[1]:
drivers/staging/rtl8188eu/os_dep/osdep_service.c: void **a = kzalloc(h * sizeof(void *) + h * w * size, GFP_KERNEL);
At the end of the day, though, I don't really like having all these different names...
kmalloc(), kmalloc_array(), kmalloc_ab_c(), kmalloc_array_3d()
with their "matching" zeroing function:
kzalloc(), kcalloc(), kzalloc_ab_c(), kmalloc_array_3d(..., gfp | __GFP_ZERO)
For the multiplication cases, I wonder if we could just have:
kmalloc_multN(gfp, a, b, c, ...) kzalloc_multN(gfp, a, b, c, ...)
and we can replace all kcalloc() users with kzalloc_mult2(), all kmalloc_array() users with kmalloc_mult2(), the abc uses with kmalloc_mult3().
That said, I *do* like kmalloc_struct() as it's a very common pattern...
Or maybe, just leave the pattern in the name? kmalloc_ab(), kmalloc_abc(), kmalloc_ab_c(), kmalloc_ab_cd() ?
Getting the constant ordering right could be part of the macro definition, maybe? i.e.:
static inline void *kmalloc_ab(size_t a, size_t b, gfp_t flags) { if (__builtin_constant_p(a) && a != 0 && \ b > SIZE_MAX / a) return NULL; else if (__builtin_constant_p(b) && b != 0 && \ a > SIZE_MAX / b) return NULL;
return kmalloc(a * b, flags); }
(I just wish C had a sensible way to catch overflow...)
-Kees
[1] git grep -E 'alloc\([^,]+[^(]\*[^)][^,]+[^(]\*[^)][^,]+[^(]\*[^)][^,]+,'
-- Kees Cook Pixel Security
| |