LKML Archive on
help / color / mirror / Atom feed
From: Kees Cook <>
To: Matthew Wilcox <>
Cc: Matthew Wilcox <>,
	Linux-MM <>,
	LKML <>,
	Rasmus Villemoes <>
Subject: Re: *alloc API changes
Date: Fri, 4 May 2018 21:24:56 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, May 4, 2018 at 8:46 PM, Matthew Wilcox <> wrote:
> and if you're counting f2fs_*alloc, there's a metric tonne of *alloc
> wrappers out there.

Yeah. *sob*

> That's a little revisionist ;-)  We had kmalloc before we had the slab
> allocator (kernel 1.2, I think?).  But I see your point, and that's
> certainly how it's implemented these days.

Okay, yes, that's true. I did think of that briefly. :)

> I got shot down for proposing adding
> #define malloc(x) kmalloc(x, GFP_KERNEL)
> on the grounds that driver writers will then use malloc in interrupt
> context.  So I think our base version has to be foo_alloc(size, gfp_t).

Okay, fair enough.

> Right, I was thinking:
> static inline size_t mul_ab(size_t a, size_t b)
> {
>         unsigned long c;
>         if (__builtin_mul_overflow(a, b, &c))
>                 return SIZE_MAX;
>         return c;
> #else
>         if (b != 0 && a >= SIZE_MAX / b)
>                 return SIZE_MAX;
>         return a * b;
> #endif
> }

Rasmus, what do you think of a saturating version of your helpers?

The only fear I have with the saturating helpers is that we'll end up
using them in places that don't recognize SIZE_MAX. Like, say:

size = mul(a, b) + 1;

then *poof* size == 0. Now, I'd hope that code would use add(mul(a,
b), 1), but still... it makes me nervous.

> You don't need the size check here.  We have the size check buried deep in
> alloc_pages (look for MAX_ORDER), so kmalloc and then alloc_pages will try
> a bunch of paths all of which fail before returning NULL.

Good point. Though it does kind of creep me out to let a known-bad
size float around in the allocator until it decides to reject it. I
would think an early:

if (unlikely(size == SIZE_MAX))
    return NULL;

would have virtually no cycle count difference...

> I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest
> calls to mult().

Agreed. I think having exactly those would cover almost everything,
and the two places where a 4-factor product is needed could just nest
them. (bikeshed: the very common mul_ab() should just be mul(), IMO.)

> Nono, Linus had the better proposal, struct_size(p, member, n).

Oh, yes! I totally missed that in the threads.

> Ooh, we could instantiate classes and ... yeah, no, not C++.  We *could*
> abuse the C preprocessor to autogenerate every variant, but I hate that
> because you can't grep for it.

Right, no. I think if we can ditch *calloc() and _array() by using
saturating helpers, we'll have the API in a much better form:

kmalloc(foo * bar, GFP_KERNEL);
kmalloc_array(foo, bar, GFP_KERNEL);
kmalloc(mul(foo, bar), GFP_KERNEL);


kmalloc(foo * bar, GFP_KERNEL | __GFP_ZERO);
kzalloc(foo * bar, GFP_KERNEL);
kcalloc(foo, bar, GFP_KERNEL);
kzalloc(mul(foo, bar), GFP_KERNEL);

and the fun

kzalloc(sizeof(*header) + count * sizeof(*header->element), GFP_KERNEL);
kzalloc(struct_size(header, element, count), GFP_KERNEL);

modulo all *alloc* families...



Kees Cook
Pixel Security

  reply	other threads:[~2018-05-05  4:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-05  1:08 Kees Cook
2018-05-05  3:46 ` Matthew Wilcox
2018-05-05  4:24   ` Kees Cook [this message]
2018-05-07 11:39     ` Matthew Wilcox
2018-05-07 16:03       ` Kees Cook
2018-05-07 20:19         ` Matthew Wilcox
2018-05-07 20:27           ` Kees Cook
2018-05-07 20:49             ` Matthew Wilcox
2018-05-07 21:15               ` Kees Cook
2018-05-07 21:48             ` John Johansen
2018-05-07 21:41     ` Rasmus Villemoes
2018-05-07 22:56       ` Kees Cook
2018-05-05  4:30   ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \
    --subject='Re: *alloc API changes' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).