LKML Archive on
 help / Atom feed
From: Kees Cook <>
To: "Uecker, Martin" <>
Cc: "" <>,
	"" <>
Subject: Re: VLAs and security
Date: Sun, 2 Sep 2018 10:40:13 -0700
Message-ID: <> (raw)
In-Reply-To: <>

On Sun, Sep 2, 2018 at 1:08 AM, Uecker, Martin
<> wrote:
> I do not agree that VLAs are generally bad for security.
> I think the opposite is true. A VLA with the right size
> allows the compiler to automatically perform or insert
> meaningful bounds checks, while a fixed upper bound does not.

While I see what you mean, the trouble is that the compiler has no
idea what the upper bounds of the _available_ stack is. This means
that a large VLA might allow code to read/write beyond the stack
allocation, which also bypasses the "standard" stack buffer overflow
checks. Additionally, VLAs bypass the existing stack-size checks we've
added to the kernel.

> For example:
> char buf[N];
> buf[n] = 1;
> Here, a compiler / analysis tool can for  n < N  using
> static analysis or insert a run-time check.
> Replacing this with
> char buf[MAX_SIZE]
> hides the information about the true upper bound
> from automatic tools.

While this may be true for some tools, I don't agree VLAs are better
in general. For example, the compiler actually knows the upper bound
at build time now, and things like the printf format size checks and
CONFIG_FORTIFY_SOURCE are now able to produce compile-time warnings
(since "sizeof(buf)" isn't a runtime value). With a VLA, this is
hidden from those tools, and detection depends on runtime analysis.

It should be noted that VLAs are also slow[1], so removing them not
only improves robustness but also improves performance.

> Limiting the stack usage can also be achieved in
> the following way:
> assert(N <= MAX_SIZE)
> char buf[N];

If you look through the various VLA removals that have been landing,
there is a common pattern of performing these checks where it might be
possible for an "n" to be larger than the fixed size. (Many removals
can be compile-time checked as callers are usually just in a specific
range -- it's not really a "runtime" size that was changing, since all
callers used different but hard-coded sizes.)

char buf[N];
if (WARN_ON(n > N))
    return -EINVAL;

> Of course, having predictable stack usage might be more
> important in the kernel and might be a good argument
> to still prefer the constant bound.

Between improved compile-time checking, faster runtime performance,
and improved robustness against stack exhaustion, I strongly believe
the kernel to be better off with VLAs entirely removed. And we are
close: only 6 remain (out of the 115 I counted in v4.15).

> But loosing the tighter bounds is clearly a disadvantage
> with respect to security that one should keep it mind.

Yes: without VLAs, stack array usage is reduced to "standard" stack
buffer overflow concerns. Removing the VLA doesn't introduce a new
risk: we already had to worry about fixed-size arrays. Removing VLAs
means we don't have to worry about the VLA-specific risks anymore.



Kees Cook
Pixel Security

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-02  8:08 Uecker, Martin
2018-09-02 17:40 ` Kees Cook [this message]
2018-09-03  7:39   ` Uecker, Martin
2018-09-03 21:28     ` Linus Torvalds
2018-09-04  6:27       ` Uecker, Martin
2018-09-04  8:00         ` Dmitry Vyukov
2018-09-04 18:22           ` Uecker, Martin
2018-09-05  7:35             ` Dmitry Vyukov

Reply instructions:

You may reply publically 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 \ \ \ \ \ \

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

LKML Archive on

Archives are clonable:
	git clone --mirror lkml/git/0.git
	git clone --mirror lkml/git/1.git
	git clone --mirror lkml/git/2.git
	git clone --mirror lkml/git/3.git
	git clone --mirror lkml/git/4.git
	git clone --mirror lkml/git/5.git
	git clone --mirror lkml/git/6.git
	git clone --mirror lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ \
	public-inbox-index lkml

Newsgroup available over NNTP:

AGPL code for this site: git clone public-inbox