[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH nbdkit] Ban use of stack Variable Length Arrays (VLAs).



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]