On Fri, 24 Jun 2022 14:12:44 -0400
Nicholas Vinson <nvinson...@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
>
> Thanks,
> Nicholas Vinson
>
> >
> >> Signed-off-by: Nicholas Vinson <nvinson...@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.1
> >
> > Daniel
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
