kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Laura Abbott <labbott@redhat.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>
Subject: Re: [RFC][PATCH 00/13] Provide saturating helpers for allocation
Date: Wed, 9 May 2018 10:01:41 -0700	[thread overview]
Message-ID: <CAGXu5jKXq7--CYgp8Q+k00RjjGJY+o71RMr51NPuWS1eM0KX1w@mail.gmail.com> (raw)
In-Reply-To: <4baffc55-510e-96d3-3487-5ea09f993a0c@redhat.com>

On Wed, May 9, 2018 at 9:08 AM, Laura Abbott <labbott@redhat.com> wrote:
> On 05/08/2018 05:42 PM, Kees Cook wrote:
>>
>> This is a stab at providing three new helpers for allocation size
>> calculation:
>>
>> struct_size(), array_size(), and array3_size().
>>
>> These are implemented on top of Rasmus's overflow checking functions,
>> and the last 8 patches are all treewide conversions of open-coded
>> multiplications into the various combinations of the helper functions.
>>
>> -Kees
>>
>>
> Obvious question (that might indicate this deserves documentation?)
>
> What's the difference between
>
> kmalloc_array(cnt, sizeof(struct blah), GFP_KERNEL);
>
> and
>
> kmalloc(array_size(cnt, struct blah), GFP_KERNEL);
>
>
> and when would you use one over the other?

If I'm understanding the intentions here, the next set of treewide
changes would be to remove *calloc() and *_array() in favor of using
the array_size() helper. (i.e. reducing proliferation of allocator
helpers in favor of using the *_size() helpers.

There are, however, some cases that don't map well to
{struct,array,array3}_size(), specifically cases of additions in
finding a count. For example, stuff like:

kmalloc(sizeof(header) + sizeof(trailing_array) * (count + SOMETHING), gfp...)

This gets currently mapped to:

kmalloc(struct_size(header, trailing_array, (count + SOMETHING), gfp...)

But we run the risk in some cases of having even the addition
overflow. I think we need to have a "saturating add" too. Something
like:

kmalloc(struct_size(header, trailing_array, sat_add(count, SOMETHING), gfp...)

It's a bit ugly, but it would cover nearly all the remaining cases...

-Kees

-- 
Kees Cook
Pixel Security

      reply	other threads:[~2018-05-09 17:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09  0:42 [RFC][PATCH 00/13] Provide saturating helpers for allocation Kees Cook
2018-05-09  0:42 ` [PATCH 01/13] compiler.h: enable builtin overflow checkers and add fallback code Kees Cook
2018-05-09  0:42 ` [PATCH 02/13] lib: add runtime test of check_*_overflow functions Kees Cook
2018-05-09  0:42 ` [PATCH 03/13] overflow.h: Add allocation size calculation helpers Kees Cook
2018-05-09 18:27   ` Rasmus Villemoes
2018-05-09 18:49     ` Kees Cook
2018-05-09  0:42 ` [PATCH 04/13] mm: Use array_size() helpers for kmalloc() Kees Cook
2018-05-09 11:34   ` Matthew Wilcox
2018-05-09 17:58     ` Kees Cook
2018-05-09 18:00     ` Rasmus Villemoes
2018-05-09 18:07       ` Kees Cook
2018-05-09 18:39         ` Rasmus Villemoes
2018-05-09  0:42 ` [PATCH 05/13] mm: Use array_size() helpers for kvmalloc() Kees Cook
2018-05-09  0:42 ` [PATCH 06/13] treewide: Use struct_size() for kmalloc()-family Kees Cook
2018-05-09  0:42 ` [PATCH 07/13] treewide: Use struct_size() for vmalloc()-family Kees Cook
2018-05-09  0:42 ` [PATCH 08/13] treewide: Use struct_size() for devm_kmalloc() and friends Kees Cook
2018-05-09  0:42 ` [PATCH 09/13] treewide: Use array_size() for kmalloc()-family Kees Cook
2018-05-09  0:42 ` [PATCH 10/13] treewide: Use array_size() for kmalloc()-family, leftovers Kees Cook
2018-05-09  0:42 ` [PATCH 11/13] treewide: Use array_size() for vmalloc() Kees Cook
2018-05-09  0:42 ` [PATCH 12/13] treewide: Use array_size() for devm_*alloc()-like Kees Cook
2018-05-09  0:42 ` [PATCH 13/13] treewide: Use array_size() for devm_*alloc()-like, leftovers Kees Cook
2018-05-09 16:08 ` [RFC][PATCH 00/13] Provide saturating helpers for allocation Laura Abbott
2018-05-09 17:01   ` Kees Cook [this message]

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=CAGXu5jKXq7--CYgp8Q+k00RjjGJY+o71RMr51NPuWS1eM0KX1w@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mawilcox@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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).