|
From: | Nicholas Vinson |
Subject: | Re: [PATCH] configure.ac: warn if stack-protector not allowed |
Date: | Sat, 25 Jun 2022 03:07:09 -0400 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 |
On 6/24/22 21:11, Glenn Washburn wrote:
On Fri, 24 Jun 2022 14:12:44 -0400 Nicholas Vinson <nvinson234@gmail.com> wrote:On 6/24/22 12:28, Daniel Kiper wrote:Adding Chris... On Tue, Jun 14, 2022 at 06:19:00PM -0400, Nicholas Vinson wrote:Previous version of configure.ac would error out when --enable-stack-protector was given and a selected GRUB platform did not support the flag. The new behavior is to warn that the flag is not supported and force disable it for that platform.Could you explain why do we need this change? I think it is safer to fail in this case than warn.What happened is I tried to modify Gentoo's GRUB ebuild so it would unconditionally pass --enable-stack-protector to configure. However, the ebuild also has a GRUB_PLATFORMS variable which allows it to build for multiple GRUB platforms simultaneously for. For me the value of that variable is set to "pc efi-64". Therefore, when I tried to install GRUB the build would fail because the GRUB "pc" platform does not support --enable-stack-protector. This essentially means that for any wrapper script that has some variation of: for p in SELECTED_GRUB_PLATFORMS; \ do configure --enable-stack-protector \ --with-platform${P} ... || die; \ done make will fail to build if SELECTED_GRUB_PLATFORMS contains a platform that does not support SSP. Therefore, the only way to work-around this issue is to modify the above for-loop, so it conditionally passes '--enable-stack-protector' to configure. If I modify the above example to have the conditional list, its behavior is effectively the same as the change I'm proposing (sans warning message). However, it does mean that the list of SSP supported platforms is now in 2 places. One in the configure script and one in my hypothetical for-loop. Furthermore, if the second list is not properly maintained it could mistakenly disable SSP for a platform that later gained support for it. In this particular case, the safety of warn vs error is about the same and switching the statement to warn makes it easier to automate building multiple GRUB platforms while passing '--enable-stack-protector' to configure.I've run across this issue too and support doing something about it for the reasons listed here. I'm fine with the current implementation. However, I think a better solution would be to have '--enable-stack-protector=warn' issue a warning when it can not be enabled and '--enable-stack-protector' or '--enable-stack-protector=yes' be the current behavior. Glenn
Currently --enable-stack-protector accepts 'yes' and 'strong' as arguments, and is used to select between '-fstack-protector' and '-fstack-protector-strong'. To keep that behavior and allow for the builder to choose between a warning and an error might require a new switch. Otherwise, I propose dropping -fstack-protector support and let --enable-stack-protector=[yes,warn] always enable -fstack-protector-strong. Thanks, Nicholas Vinson
Thanks, Nicholas VinsonSigned-off-by: Nicholas Vinson <nvinson234@gmail.com> --- configure.ac | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 57fb70945..9bdc102b2 100644 --- a/configure.ac +++ b/configure.ac @@ -1349,7 +1349,8 @@ 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]) + AC_MSG_WARN([--enable-stack-protector is only supported on EFI platforms]) + 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 -- 2.35.1Daniel_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[Prev in Thread] | Current Thread | [Next in Thread] |