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: Nicholas Vinson
Subject: Re: [PATCH v2] configure.ac: warn if stack-protector not allowed
Date: Thu, 7 Jul 2022 12:27:45 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1

On 7/7/22 09:17, Daniel Kiper wrote:
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...

Agreed. It's an errant white space only change. Unless someone says otherwise, I'll regenerate the request with this change removed.

Thanks,
Nicholas Vinson


  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]