[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] configure.ac: warn if stack-protector not allowed
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v2] configure.ac: warn if stack-protector not allowed |
Date: |
Thu, 7 Jul 2022 15:17:20 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Wed, Jul 06, 2022 at 03:25:58AM -0400, Nicholas Vinson wrote:
> Introduce ERROR_PLATFORM_NOT_SUPPORT_SSP environment variable to treat
> the '--enable-stack-protector is only supported on EFI platforms'
> message as a warning instead of an error. If
> ERROR_PLATFORM_NOT_SUPPORT_SSP is set to 'no' (case-insensitive), then
> the message will be printed as a warning. Otherwise, it prints as an
> error. The default behavior is to print the message as an error.
>
> For any wrapper build script that has some variation of:
>
> for p in SELECTED_GRUB_PLATFORMS; do \
> configure --enable-stack-protector \
> --with-platform${P} ... || die; \
> done
> make
>
> GRUB will fail to build if SELECTED_GRUB_PLATFORMS contains a platform
> that does not support SSP.
>
> Such wrapper scripts need to work-around this issue by modifying the
> above for-loop, so it conditionally passes '--enable-stack-protector' to
> configure for the proper GRUB platform(s).
>
> However, if the above example is modified to have to conditionally pass
> in --enable-stack-protector, its behavior is effectively the same as the
> proposed change. Additionally, The list of SSP supported platforms is
> now in 2 places. One in the configure script and one in the build
> wrapper script. If the second list is not properly maintained it could
> mistakenly disable SSP for a platform that later gained support for it.
>
> Signed-off-by: Nicholas Vinson <nvinson234@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
But one nit below...
> ---
> configure.ac | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 57fb70945..a08f3285e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -36,6 +36,10 @@ dnl description of the relationships between them.
>
> AC_INIT([GRUB],[2.11],[bug-grub@gnu.org])
>
> +AS_CASE(["$ERROR_PLATFORM_NOT_SUPPORT_SSP"],
> + [n | no | nO | N | No | NO], [ERROR_PLATFORM_NOT_SUPPORT_SSP=no],
> + [ERROR_PLATFORM_NOT_SUPPORT_SSP=yes])
> +
> # We don't want -g -O2 by default in CFLAGS
> : ${CFLAGS=""}
>
> @@ -1342,6 +1346,7 @@ AC_ARG_ENABLE([stack-protector],
> [enable the stack protector]),
> [],
> [enable_stack_protector=no])
> +
I think this change can be dropped...
> if test "x$enable_stack_protector" = xno; then
> if test "x$ssp_possible" = xyes; then
> # Need that, because some distributions ship compilers that include
> @@ -1349,7 +1354,12 @@ if test "x$enable_stack_protector" = xno; then
> TARGET_CFLAGS="$TARGET_CFLAGS -fno-stack-protector"
> fi
> elif test "x$platform" != xefi; then
> - AC_MSG_ERROR([--enable-stack-protector is only supported on EFI platforms])
> + if test "$ERROR_PLATFORM_NOT_SUPPORT_SSP" = "yes"; then
> + AC_MSG_ERROR([--enable-stack-protector is only supported on EFI
> platforms])
> + else
> + AC_MSG_WARN([--enable-stack-protector is only supported on EFI
> platforms])
> + fi
> + enable_stack_protector=no
> elif test "x$ssp_global_possible" != xyes; then
> AC_MSG_ERROR([--enable-stack-protector is not supported (compiler doesn't
> support -mstack-protector-guard=global)])
> else
Daniel