On Thu, Sep 05, 2019 at 07:57:44AM -0500, Eric Blake wrote:
On 9/5/19 6:28 AM, Richard W.M. Jones wrote:
I'm not someone who thinks VLAs are automatically bad and unlike Linux kernel code they can sometimes be used safely in userspace. However for an internet exposed server there is an argument that they might cause some kind of exploitable situation especially if the code is compiled without other stack hardening features. Also in highly multithreaded code with limited stack sizes (as nbdkit is on some platforms) allowing unbounded stack allocation can be a bad idea because it can cause a segfault.
So this commit bans them. Only when using --enable-gcc-warnings, but upstream developers ought to be using that.
There were in fact only two places where VLAs were being used. In one of those places (plugins/sh) removing the VLA actually made the code better.
For interesting discussion about VLAs in the kernel see: https://lwn.net/Articles/763253/ --- configure.ac | 2 +- plugins/sh/sh.c | 7 +++---- server/sockets.c | 8 +++++++- 3 files changed, 11 insertions(+), 6 deletions(-)
+++ b/configure.ac @@ -90,7 +90,7 @@ AC_ARG_ENABLE([gcc-warnings], [gcc_warnings=no] ) if test "x$gcc_warnings" = "xyes"; then - WARNINGS_CFLAGS="-Wall -Wshadow -Werror" + WARNINGS_CFLAGS="-Wall -Wshadow -Wvla -Werror"
I'm guessing that both gcc and clang are okay with our current list; we may reach the point where we need to probe at configure time on which options we can safely use, instead of merely open-coding a list, but we'll deal with that when it breaks the build.
With the proviso that I didn't actually test it under clang, it does seem to have the flag: https://clang.llvm.org/docs/DiagnosticsReference.html#wvla [...] Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top