grub-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]