Date: Sat, 4 Nov 2017 09:02:00 +1100 From: "Tobin C. Harding" <me@...in.cc> To: Tycho Andersen <tycho@...ker.com> Cc: kernel-hardening@...ts.openwall.com, Kees Cook <keescook@...omium.org> Subject: Re: [RFC] vla: add VLA macro and testing On Fri, Nov 03, 2017 at 11:44:44AM +0300, Tycho Andersen wrote: > On Thu, Nov 02, 2017 at 10:50:40AM +1100, Tobin C. Harding wrote: > > Variable Length Arrays (VLA) pose a risk to the stack if the variable > > passed into the array declaration is too large. If the variable used can > > be controlled by a malicious party then this poses a security risk to > > the kernel. > > > > Add a macro for declaring VLA's. Macro includes a requested size and a > > maximum size, if requested size is larger than maximum size then > > requested size is capped at maximum. Requested size is passed by > > reference and updated by macro so caller has access to size of array > > after declaration. > > > > Signed-off-by: Tobin C. Harding <me@...in.cc> > > > > --- > > > > I was unable to get the test module to integrate with the kernel build system > > correctly. The attempt was to mirror the way `lib/test_printf.c` functions. This > > effort was unsuccessful, it is included in the patch in the hope of getting > > better suggestions. To test, the test module was built out of tree and all tests > > pass. > > > > The macro needs some work. It functions as intended but > > > > Checkpatch emits ERROR: Macros with multiple statements should be enclosed in a > > do - while loop. > > > > Also for each use of VLA() checkpatch emits WARNING: Missing a blank line after > > declarations. > > > > Also I was unsure where to put the macro definition, appreciate any suggestions. > > > > thanks, > > Tobin. > > > > include/linux/vla.h | 24 ++++++++++ > > lib/Kconfig.debug | 3 ++ > > lib/Makefile | 1 + > > lib/test_vla.c | 98 ++++++++++++++++++++++++++++++++++++++ > > tools/testing/selftests/lib/config | 1 + > > 5 files changed, 127 insertions(+) > > create mode 100644 include/linux/vla.h > > create mode 100644 lib/test_vla.c > > > > diff --git a/include/linux/vla.h b/include/linux/vla.h > > new file mode 100644 > > index 000000000000..e8f1572bbf42 > > --- /dev/null > > +++ b/include/linux/vla.h > > @@ -0,0 +1,24 @@ > > +/* > > + * This software is licensed under the terms of the GNU General Public > > + * License version 2, as published by the Free Software Foundation, and > > + * may be copied, distributed, and modified under those terms. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + */ > > +#ifndef __LINUX_VLA_H > > +#define __LINUX_VLA_H > > + > > +/* > > + * When declaring a local variable using VLA(), VLA() must be the last > > + * declaration otherwise a compiler warning [-Wdeclaration-after-statement] will > > + * be generated > > + */ > > +#define VLA(type, name, size, max) \ > > + type name[(*size < max ? *size : max)]; \ > > + *size = (*size < max ? *size : max) > > This requires you to have your VLAs at the end of the declarations, > which is annoying and potentially limiting (limited to one VLA, etc.). > I also don't think you need the pointer. Something like: > > #define VLA(type, name, size, max) \ > type name[(size < max ? max : (size = max))] > > should work I think. type name[(size < max ? size : (size = max))] Oh, cool. Thanks Tycho. I'll test out your suggestion. At first glance I think that will remove both the checkpatch issues and the declaration annoyance. Nice one! > It would also be nice to see some users of the VLA macro as part of > this series. Righto, I wondered if that was the done thing. Will add one for the next spin. thanks, Tobin.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.