[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171103084444.r7ls2klvwk7ndq7a@docker>
Date: Fri, 3 Nov 2017 11:44:44 +0300
From: Tycho Andersen <tycho@...ker.com>
To: "Tobin C. Harding" <me@...in.cc>
Cc: kernel-hardening@...ts.openwall.com, Kees Cook <keescook@...omium.org>
Subject: Re: [RFC] vla: add VLA macro and testing
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.
It would also be nice to see some users of the VLA macro as part of
this series.
Cheers,
Tycho
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.
