Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[James Youngman]

To be clear before we start, gnulib is doing the right thing here.  It
contains this code in lib/gettext.h:-

static const char *
dcpgettext_expr (const char *domain,
                 const char *msgctxt, const char *msgid,
                 int category)
{
  size_t msgctxt_len = strlen (msgctxt) + 1;
  size_t msgid_len = strlen (msgid) + 1;
  const char *translation;
#if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
  char msg_ctxt_id[msgctxt_len + msgid_len];
#else
  char buf[1024];
  char *msg_ctxt_id =
    (msgctxt_len + msgid_len <= sizeof (buf)
     ? buf
     : (char *) malloc (msgctxt_len + msgid_len));
  if (msg_ctxt_id != NULL)
#endif


tl;dr: it uses a variable-length array if we determined that the
compiler supports those.   All well and good.   But, if we compile the
code with more GCC warnings turned on via the manywarnings module, we
get this result:

gcc -std=gnu99 -DHAVE_CONFIG_H -I.
-I/home/james/source/GNU/findutils/git/gnu/findutils/find -I..
-I../gl/lib -I/home/james/source/GNU/findutils/git/gnu/findutils/lib
-I/home/james/source/GNU/findutils/git/gnu/findutils/gl/lib -I../intl
-DLOCALEDIR=\"/usr/local/share/locale\"   -Wall -W -Wformat-y2k
-Wformat-security -Winit-self -Wmissing-include-dirs -Wswitch-enum
-Wunused -Wunknown-pragmas -Wstrict-aliasing -Wstrict-overflow
-Wfloat-equal -Wdeclaration-after-statement -Wshadow
-Wunsafe-loop-optimizations -Wpointer-arith -Wbad-function-cast
-Wcast-qual -Wcast-align -Wwrite-strings -Wlogical-op
-Waggregate-return -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn
-Wmissing-format-attribute -Wpacked -Wredundant-decls -Wnested-externs
-Winline -Winvalid-pch -Wlong-long -Wvla -Wvolatile-register-var
-Wdisabled-optimization -Wstack-protector -Woverlength-strings
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat
-Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar
-Wunused-macros -Wno-missing-field-initializers -g -O2 -MT fstype.o
-MD -MP -MF .deps/fstype.Tpo -c -o fstype.o
/home/james/source/GNU/findutils/git/gnu/findutils/find/fstype.c
cc1: warning: ../intl: No such file or directory
In file included from
/home/james/source/GNU/findutils/git/gnu/findutils/find/fstype.c:56:
/home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:
In function 'dcpgettext_expr':
/home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:216:
warning: variable length array 'msg_ctxt_id' is used
/home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:
In function 'dcnpgettext_expr':
/home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:262:
warning: variable length array 'msg_ctxt_id' is used

In other words, "gcc -Wvla" is issuing a warning for a construct we
know is safe.   However, I can't be sure I won't accidentally write
code in the future which is not protected by something similar to
_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.   So I think that -Wvla is a
useful warning flag.

Is there a way of eliminating this false positive which doesn't force
me to give up -Wvla?   I mean, apart from giving up the use of VLAs in
gnulib even when it's safe to use them.

James.

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Paul Eggert]

On 07/09/11 15:32, James Youngman wrote:
> Is there a way of eliminating this false positive which doesn't force
> me to give up -Wvla?

You can use a pragma in the module that you've audited.
The pragma would tell GCC, "don't waste my time warning
about VLAs in this module".

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[James Youngman]

On Sun, Jul 10, 2011 at 5:47 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 07/09/11 15:32, James Youngman wrote:
>> Is there a way of eliminating this false positive which doesn't force
>> me to give up -Wvla?
>
> You can use a pragma in the module that you've audited.
> The pragma would tell GCC, "don't waste my time warning
> about VLAs in this module".

I can't see a way of making this strategy conveniently work, since
there the VLA is in gettext.h, which will be included by a number of
source files.   Adding
#pragma GCC ignored -Wvla
to gettexzt.h would result in VLA warnings being suppressed for any
module which uses gettext.h, which is one of the things I was trying
to avoid.

While I could balance this by remembering to put
#pragma GCC warning -Wvla
in my source files after all the includes, it seems to me I'm no more
likely to remember to do that than to avoid using VLAs in the first
place.   This would also take away from the user the configure-time
control they currently have over what warning flags are turned on or
off.

Something I haven't tried yet it a configure.ac change; if we discover
in configure that -Wvla is in CFLAGS and the compiler is GCC, we could
add this:
#pragma GCC diagnostic error "-Wvla"
... presumably if done correctly this would cause configure to set
_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS to 0.   This method seems
suboptimal to me though, since as I mentioned earlier in the thread,
the ideal thing would be to allow VLAs in parts of the code which are
protected by something like _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.

A function-level pragma would probably be ideal here, but
unfortunately they can only be used to tweak optimisation and function
attributes.

James.

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Simon Josefsson]

I ran into this issue as well some time ago, and my "solution" was to
remove those definitions from my local gettext.h copy.  It is a
non-solution, but I thought I should mention it in case your project
(like gsasl) does not use those gettext.h functions and you just want to
silence the warning.

http://git.savannah.gnu.org/gitweb/?p=gsasl.git;a=blob;f=gl/override/lib/gettext.h.diff;hb=HEAD

/Simon

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Paul Eggert]

On 07/10/11 02:16, James Youngman wrote:
> A function-level pragma would probably be ideal here, but
> unfortunately they can only be used to tweak optimisation and function
> attributes.

Can you do "#pragma GCC diagnostic push" at the start of the function,
and "#pragma GCC diagnostic pop" at the end?  That's *supposed* to
work (not that I've tried it....).

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[James Youngman]

On Mon, Jul 11, 2011 at 1:11 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 07/10/11 02:16, James Youngman wrote:
>> A function-level pragma would probably be ideal here, but
>> unfortunately they can only be used to tweak optimisation and function
>> attributes.
>
> Can you do "#pragma GCC diagnostic push" at the start of the function,
> and "#pragma GCC diagnostic pop" at the end?  That's *supposed* to
> work (not that I've tried it....).

I'll try it next week.   It's not documented in the GCC-4.4 Texinfo
manual, though; perhaps it's a recent introduction.

James.

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Paul Eggert]

>> Can you do "#pragma GCC diagnostic push"
> I'll try it next week.   It's not documented in the GCC-4.4 Texinfo
> manual, though; perhaps it's a recent introduction.

Yes, it's introduced in GCC 4.6.0.

http://gcc.gnu.org/gcc-4.6/changes.html

Presumably one can use it if available, and fall back
on the existing scheme otherwise.

Re: Correct but unhelpful VLA warning vs. gnulib's gettext.h; can we eliminate the false positive?

[Pádraig Brady]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

On 09/07/11 15:32, James Youngman wrote:
> To be clear before we start, gnulib is doing the right thing here.  It
> contains this code in lib/gettext.h:-
> 
> static const char *
> dcpgettext_expr (const char *domain,
>                  const char *msgctxt, const char *msgid,
>                  int category)
> {
>   size_t msgctxt_len = strlen (msgctxt) + 1;
>   size_t msgid_len = strlen (msgid) + 1;
>   const char *translation;
> #if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
>   char msg_ctxt_id[msgctxt_len + msgid_len];
> #else
>   char buf[1024];
>   char *msg_ctxt_id =
>     (msgctxt_len + msgid_len <= sizeof (buf)
>      ? buf
>      : (char *) malloc (msgctxt_len + msgid_len));
>   if (msg_ctxt_id != NULL)
> #endif
> 
> 
> tl;dr: it uses a variable-length array if we determined that the
> compiler supports those.   All well and good.   But, if we compile the
> code with more GCC warnings turned on via the manywarnings module, we
> get this result:

> In function 'dcpgettext_expr':
> /home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:216:
> warning: variable length array 'msg_ctxt_id' is used

> In other words, "gcc -Wvla" is issuing a warning for a construct we
> know is safe.   However, I can't be sure I won't accidentally write
> code in the future which is not protected by something similar to
> _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.   So I think that -Wvla is a
> useful warning flag.
> 
> Is there a way of eliminating this false positive which doesn't force
> me to give up -Wvla?   I mean, apart from giving up the use of VLAs in
> gnulib even when it's safe to use them.

We might want to disable use of VLAs even if the compiler supports it,
for security reasons (like the Linux kernel now does), or if you
didn't want to consider VLA portability in gnulib using projects,
as you've suggested.

Attached allows one to define GNULIB_NO_VLA to support that,
which I've tested in coreutils with:

  AC_DEFINE([GNULIB_NO_VLA], [1], [Define to 1 to disable use of VLAs])

Note -Wvla is implicitly added by gl_MANYWARN_ALL_GCC,
so we don't need any special handling of this option once GNULIB_NO_VLA is defined.

cheers,
Pádraig

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gnulib-no-vla.diff --]
[-- Type: text/x-patch; name="gnulib-no-vla.diff", Size: 1554 bytes --]

From fb2b401be4d57f035322ebba825292e66db0e999 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Sun, 13 Jan 2019 22:05:10 -0800
Subject: [PATCH] gettext: support disabling use of VLAs

* lib/gettext.h: Disable use of VLAs if GNULIB_NO_VLA is defined
---
 ChangeLog     | 5 +++++
 lib/gettext.h | 7 ++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d1f0d63..2e87a1b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-13  Pádraig Brady  <P@draigBrady.com>
+
+	gettext: support disabling use of VLAs
+	* lib/gettext.h: Disable use of VLAs if GNULIB_NO_VLA is defined
+
 2018-12-21  Bruno Haible  <bruno@clisp.org>
 
 	Assume Autoconf >= 2.63.
diff --git a/lib/gettext.h b/lib/gettext.h
index d5d56ec..a0d854e 100644
--- a/lib/gettext.h
+++ b/lib/gettext.h
@@ -184,9 +184,10 @@ npgettext_aux (const char *domain,
 
 #include <string.h>
 
-#if (((__GNUC__ >= 3 || __GNUG__ >= 2) && !defined __STRICT_ANSI__) \
-     /* || (__STDC_VERSION__ == 199901L && !defined __HP_cc)
-        || (__STDC_VERSION__ >= 201112L && !defined __STDC_NO_VLA__) */ )
+#if (!defined GNULIB_NO_VLA \
+     && (((__GNUC__ >= 3 || __GNUG__ >= 2) && !defined __STRICT_ANSI__) \
+     /*  || (__STDC_VERSION__ == 199901L && !defined __HP_cc)
+         || (__STDC_VERSION__ >= 201112L && !defined __STDC_NO_VLA__) */ ))
 # define _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS 1
 #else
 # define _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS 0
-- 
2.9.3

Re: VLA and alloca

[Bruno Haible]

Hi,

Pádraig Brady wrote:
> > In function 'dcpgettext_expr':
> > /home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:216:
> > warning: variable length array 'msg_ctxt_id' is used
> 
> > In other words, "gcc -Wvla" is issuing a warning for a construct we
> > know is safe.   However, I can't be sure I won't accidentally write
> > code in the future which is not protected by something similar to
> > _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.   So I think that -Wvla is a
> > useful warning flag.
> > 
> > Is there a way of eliminating this false positive which doesn't force
> > me to give up -Wvla?   I mean, apart from giving up the use of VLAs in
> > gnulib even when it's safe to use them.
> 
> We might want to disable use of VLAs even if the compiler supports it,
> for security reasons (like the Linux kernel now does)

The proposed patch looks reasonable to me. And it would be useful to
add a bit of documentation, namely: Who should define GNULIB_NO_VLA,
and for what reasons?

There are two related issues:

* GCC not only has a warning -Wvla (whose purpose is probably to avoid
  VLAs in order to produce code compatible with other compilers), but
  also -Wvla-larger-than=N [1], whose purpose is that the stack "jumps"
  by not more than one page at once.

* It also has a warning -Walloca-larger-than=N [1], with the same purpose.

Maybe we can come to a gnulib-wide policy regarding the use of alloca()
and VLAs?

Does glibc have such a policy?

My thoughts regarding these options are mixed:

* On one hand, we are regularly using machines where a process may have
  more than 1 million of memory pages. A policy that forbids stack allocations
  of more than 1 page has the effect that the stack will rarely grow beyond
  200 memory pages. The resulting pressure to use heap allocations instead
  of stack allocations may be justified for multi-thread applications, but
  is not justified for single-thread applications, which could otherwise
  use 200000 memory pages of stack without problems.

* On the other hand, there is the problem of detecting a stack that "jumps".
  There are mitigations in the Linux kernel [2][3], as well as in GCC:
  GCC has an option -fstack-clash-protection [4].

What are the widely accepted points of view on these topics?

Bruno

[1] https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Warning-Options.html
[2] https://lwn.net/Articles/725832/
[3] https://lwn.net/Articles/727703/
[4] https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Instrumentation-Options.html

Re: VLA and alloca

[Pádraig Brady]

On 14/01/19 17:57, Bruno Haible wrote:
> Hi,
> 
> Pádraig Brady wrote:
>>> In function 'dcpgettext_expr':
>>> /home/james/source/GNU/findutils/git/gnu/findutils/gl/lib/gettext.h:216:
>>> warning: variable length array 'msg_ctxt_id' is used
>>
>>> In other words, "gcc -Wvla" is issuing a warning for a construct we
>>> know is safe.   However, I can't be sure I won't accidentally write
>>> code in the future which is not protected by something similar to
>>> _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS.   So I think that -Wvla is a
>>> useful warning flag.
>>>
>>> Is there a way of eliminating this false positive which doesn't force
>>> me to give up -Wvla?   I mean, apart from giving up the use of VLAs in
>>> gnulib even when it's safe to use them.
>>
>> We might want to disable use of VLAs even if the compiler supports it,
>> for security reasons (like the Linux kernel now does)
> 
> The proposed patch looks reasonable to me. And it would be useful to
> add a bit of documentation, namely: Who should define GNULIB_NO_VLA,
> and for what reasons?
> 
> There are two related issues:
> 
> * GCC not only has a warning -Wvla (whose purpose is probably to avoid
>   VLAs in order to produce code compatible with other compilers), but
>   also -Wvla-larger-than=N [1], whose purpose is that the stack "jumps"
>   by not more than one page at once.
> 
> * It also has a warning -Walloca-larger-than=N [1], with the same purpose.
> 
> Maybe we can come to a gnulib-wide policy regarding the use of alloca()
> and VLAs?


Indeed alloca has similar considerations.
Though there is the alloca module to handle portability,
and use of alloca is explicit and so easier to audit/manage etc.
VLAs are tricky as they can be introduced if the array size
is not determined to be constant, which may happen inadvertently.
Also the portability concerns are more awkward.

I've pushed this with some comments at the current single GNULIB_NO_VLA usage.

> Does glibc have such a policy?
> 
> My thoughts regarding these options are mixed:
> 
> * On one hand, we are regularly using machines where a process may have
>   more than 1 million of memory pages. A policy that forbids stack allocations
>   of more than 1 page has the effect that the stack will rarely grow beyond
>   200 memory pages. The resulting pressure to use heap allocations instead
>   of stack allocations may be justified for multi-thread applications, but
>   is not justified for single-thread applications, which could otherwise
>   use 200000 memory pages of stack without problems.
> 
> * On the other hand, there is the problem of detecting a stack that "jumps".
>   There are mitigations in the Linux kernel [2][3], as well as in GCC:
>   GCC has an option -fstack-clash-protection [4].
> 
> What are the widely accepted points of view on these topics?

I've not analyzed the security concerns in detail, but in general
large allocations on the stack are bad for security and somewhat for performance,
so I think it's useful for programs to be able disable alloca and VLAs,
but independently from each other.

cheers,
Pádraig.

Re: VLA and alloca

[Bruno Haible]

Paul,

Pádraig Brady wrote:
> I've pushed this with some comments at the current single GNULIB_NO_VLA usage.

How about making use of this GNULIB_NO_VLA macro in all places that assume
VLA syntax? I'm proposing this patch:


2019-01-20  Bruno Haible  <bruno@clisp.org>

	vla: Consider GNULIB_NO_VLA.
	* lib/vla.h (VLA_ELEMS): Define to empty if GNULIB_NO_VLA is defined.

diff --git a/lib/vla.h b/lib/vla.h
index f6ebba0..fa5f39a 100644
--- a/lib/vla.h
+++ b/lib/vla.h
@@ -17,10 +17,21 @@
 
    Written by Paul Eggert.  */
 
-/* A function's argument must point to an array with at least N elements.
+/* GNULIB_NO_VLA can be defined to disable use of VLAs even if supported.
+   This relates to the -Wvla and -Wvla-larger-than warnings, enabled in
+   the default GCC many warnings set.  This allows programs to disable use
+   of VLAs, which may be unintended, or may be awkward to support portably,
+   or may have security implications due to non-deterministic stack usage.  */
+
+/* Types and variables which are variable-length arrays can be used without
+   particular macros.  */
+
+/* VLA_ELEMS is a helper macro used for declaring a function parameter that
+   is a variable-length array.
+   A function's argument must point to an array with at least N elements.
    Example: 'int main (int argc, char *argv[VLA_ELEMS (argc)]);'.  */
 
-#ifdef __STDC_NO_VLA__
+#if defined __STDC_NO_VLA__ || defined GNULIB_NO_VLA
 # define VLA_ELEMS(n)
 #else
 # define VLA_ELEMS(n) static n

Re: VLA and alloca

[Bruno Haible]

Pádraig Brady wrote:
> I've not analyzed the security concerns in detail, but in general
> large allocations on the stack are bad for security

Indeed. Just reading this CVE [1] from a week ago, makes me want to
disable all large allocations on the stack.

Bruno

[1] https://www.openwall.com/lists/oss-security/2019/01/09/3

Re: VLA and alloca

[Pádraig Brady]

On 20/01/19 02:19, Bruno Haible wrote:
> Paul,
> 
> Pádraig Brady wrote:
>> I've pushed this with some comments at the current single GNULIB_NO_VLA usage.
> 
> How about making use of this GNULIB_NO_VLA macro in all places that assume
> VLA syntax? I'm proposing this patch:
> 
> 
> 2019-01-20  Bruno Haible  <bruno@clisp.org>
> 
> 	vla: Consider GNULIB_NO_VLA.
> 	* lib/vla.h (VLA_ELEMS): Define to empty if GNULIB_NO_VLA is defined.
> 
> diff --git a/lib/vla.h b/lib/vla.h
> index f6ebba0..fa5f39a 100644
> --- a/lib/vla.h
> +++ b/lib/vla.h
> @@ -17,10 +17,21 @@
>  
>     Written by Paul Eggert.  */
>  
> -/* A function's argument must point to an array with at least N elements.
> +/* GNULIB_NO_VLA can be defined to disable use of VLAs even if supported.
> +   This relates to the -Wvla and -Wvla-larger-than warnings, enabled in
> +   the default GCC many warnings set.  This allows programs to disable use
> +   of VLAs, which may be unintended, or may be awkward to support portably,
> +   or may have security implications due to non-deterministic stack usage.  */
> +
> +/* Types and variables which are variable-length arrays can be used without
> +   particular macros.  */
> +
> +/* VLA_ELEMS is a helper macro used for declaring a function parameter that
> +   is a variable-length array.
> +   A function's argument must point to an array with at least N elements.
>     Example: 'int main (int argc, char *argv[VLA_ELEMS (argc)]);'.  */
>  
> -#ifdef __STDC_NO_VLA__
> +#if defined __STDC_NO_VLA__ || defined GNULIB_NO_VLA
>  # define VLA_ELEMS(n)
>  #else
>  # define VLA_ELEMS(n) static n

+1

thanks,
Pádraig

Re: VLA and alloca

[Tim Rühsen]

On 1/20/19 4:36 PM, Bruno Haible wrote:
> Pádraig Brady wrote:
>> I've not analyzed the security concerns in detail, but in general
>> large allocations on the stack are bad for security
> 
> Indeed. Just reading this CVE [1] from a week ago, makes me want to
> disable all large allocations on the stack.

Yes please. Any chance to remove it from gettext.h ?

#if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
  char msg_ctxt_id[msgctxt_len + msgid_len];
#else

> 
> Bruno
> 
> [1] https://www.openwall.com/lists/oss-security/2019/01/09/3

Regards, Tim

Re: VLA and alloca

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Bruno Haible wrote:
> How about making use of this GNULIB_NO_VLA macro in all places that assume
> VLA syntax?

That's OK for allocating VLAs, but it's too broad for vla.h since vla.h does not 
allocate VLAs and its uses of variable-length arrays do not have the security or 
performance issues that people ordinarily think of when they think of VLAs.

I see that vla.h's comments are too terse about this, so I installed the 
attached patch to try to clarify things.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vla-add-commentary-about-VLA_ELEMS.patch --]
[-- Type: text/x-patch; name="0001-vla-add-commentary-about-VLA_ELEMS.patch", Size: 2949 bytes --]

From 1631ac751e1dfdf09816e719e1571ab0d0e3ba88 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 2 Feb 2019 14:39:59 -0800
Subject: [PATCH] vla: add commentary about VLA_ELEMS
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/vla.h (VLA_ELEMS): Add commentary,
some inspired by Bruno Haible’s proposal in:
https://lists.gnu.org/r/bug-gnulib/2019-01/msg00109.html
---
 ChangeLog |  5 +++++
 lib/vla.h | 26 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 91ff7eaed..d16d437a2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2019-02-02  Paul Eggert  <eggert@cs.ucla.edu>
 
+	vla: add commentary about VLA_ELEMS
+	* lib/vla.h (VLA_ELEMS): Add commentary,
+	some inspired by Bruno Haible’s proposal in:
+	https://lists.gnu.org/r/bug-gnulib/2019-01/msg00109.html
+
 	dtoastr,ftoastr,ldtoastr: port to c-strtod changes
 	Decouple these modules from c-strtod.  Nowadays it’s reasonable to
 	assume the C99 signatures for strtod and strtold.  Programs that
diff --git a/lib/vla.h b/lib/vla.h
index f6ebba0ed..8f5dea76f 100644
--- a/lib/vla.h
+++ b/lib/vla.h
@@ -17,6 +17,20 @@
 
    Written by Paul Eggert.  */
 
+/* The VLA_ELEMS macro does not allocate variable-length arrays (VLAs),
+   so it does not have the security or performance issues commonly
+   associated with VLAs.  VLA_ELEMS is for exploiting a C11 feature
+   where a function can start like this:
+
+     double scan_array (int n, double v[static n])
+
+   to require a caller to pass a vector V with at least N elements;
+   this allows better static checking and performance in some cases.
+   In C11 this feature means that V is a VLA, so the feature is
+   supported only if __STDC_NO_VLA__ is defined, and for compatibility
+   to platforms that do not support VLAs, VLA_ELEMS (n) expands to
+   nothing when __STDC_NO_VLA__ is not defined.  */
+
 /* A function's argument must point to an array with at least N elements.
    Example: 'int main (int argc, char *argv[VLA_ELEMS (argc)]);'.  */
 
@@ -25,3 +39,15 @@
 #else
 # define VLA_ELEMS(n) static n
 #endif
+
+/* Although C99 requires support for variable-length arrays (VLAs),
+   some C compilers never supported VLAs and VLAs are optional in C11.
+   VLAs are controversial because their allocation may be unintended
+   or awkward to support, and large VLAs might cause security or
+   performance problems.  GCC can diagnose the use of VLAs via the
+   -Wvla and -Wvla-larger-than warnings options, and defining the
+   macro GNULIB_NO_VLA disables the allocation of VLAs in Gnulib code.
+
+   The VLA_ELEMS macro is unaffected by GNULIB_NO_VLA, since it does
+   not allocate VLAs.  Programs that use VLA_ELEMS should be compiled
+   with 'gcc -Wvla-larger-than' instead of with 'gcc -Wvla'.  */
-- 
2.17.1

Re: VLA and alloca

[Paul Eggert]

Tim Rühsen wrote:
>> Just reading this CVE [1] from a week ago, makes me want to
>> disable all large allocations on the stack.
> Yes please. Any chance to remove it from gettext.h ?
> 
> #if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
>    char msg_ctxt_id[msgctxt_len + msgid_len];
> #else
> 

It's already removed from gettext.h if you define GNULIB_NO_VLAS.

Typical applications never give dcpgettext_expr arguments so long that its VLA 
will crush the stack. For these applications enabling VLAs can be a minor 
performance win, so I'm not inclined to define GNULIB_NO_VLA for the apps I help 
maintain.

Ironically, though, one of the few applications that needs to support 
really-long message-IDs is the 'gettext' program itself. This might be an 
argument for disabling use of VLAs in libgettext, even though most applications 
work just fine with VLAs.  If we do that, GNULIB_NO_VLA will have no effect 
since lib/gettext.h is the only source file where Gnulib creates VLAs.