From: Kees Cook <keescook@chromium.org>
To: "Uecker, Martin" <Martin.Uecker@med.uni-goettingen.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>
Subject: Re: VLAs and security
Date: Sun, 2 Sep 2018 10:40:13 -0700 [thread overview]
Message-ID: <CAGXu5jJ7kGm-DGUB8uwwhd-Hv7i4WEcSKumTf2MBsj_-yzWcqg@mail.gmail.com> (raw)
In-Reply-To: <1535875700.17858.3.camel@med.uni-goettingen.de>
On Sun, Sep 2, 2018 at 1:08 AM, Uecker, Martin
<Martin.Uecker@med.uni-goettingen.de> 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
[1] https://git.kernel.org/linus/02361bc77888
--
Kees Cook
Pixel Security
next prev parent reply other threads:[~2018-09-02 17:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-02 8:08 VLAs and security 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 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=CAGXu5jJ7kGm-DGUB8uwwhd-Hv7i4WEcSKumTf2MBsj_-yzWcqg@mail.gmail.com \
--to=keescook@chromium.org \
--cc=Martin.Uecker@med.uni-goettingen.de \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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).