Re: [PATCH] configure.ac: warn if stack-protector not allowed

[Nicholas Vinson]

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.

Thanks,
Nicholas Vinson

> > Signed-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.1
>
> Daniel