Skip to contentSkip to conversation

New: grub failure with BR2_OPTIMIZE_3

Buildroot bugzilla at busybox.net 4 participants 10 messages Mar 7, 2021 – Jun 4, 2021

lore
Share & tools
lore
#0 lore raw

https://bugs.busybox.net/show_bug.cgi?id=13586

Bug ID: 13586
Summary: grub failure with BR2_OPTIMIZE_3
Product: buildroot
Version: 2021.02
Hardware: All
OS: Linux
Status: NEW
Severity: normal
Priority: P5
Component: Other
Assignee: unassigned at buildroot.uclibc.org
Reporter: ps.report at gmx.net
CC: buildroot at uclibc.org
Target Milestone: ---

Following defconfig works (real hardware and qemu):

BR2_x86_64=y
BR2_KERNEL_HEADERS_5_10=y BR2_TOOLCHAIN_BUILDROOT_WCHAR=y BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y BR2_TARGET_GENERIC_GETTY_PORT="tty1" BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh" BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh" BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-bios.cfg" BR2_LINUX_KERNEL=y BR2_LINUX_KERNEL_CUSTOM_VERSION=y BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="5.10.20" BR2_LINUX_KERNEL_DEFCONFIG="x86_64" BR2_LINUX_KERNEL_INSTALL_TARGET=y BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y BR2_TARGET_ROOTFS_EXT2=y BR2_TARGET_ROOTFS_EXT2_4=y BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
# BR2_TARGET_ROOTFS_TAR is not set BR2_TARGET_GRUB2=y
BR2_PACKAGE_HOST_GENIMAGE=y

Started with:

$ qemu-system-x86_64 -m 4096 --drive format=raw,file=images/disk.img

Shows BIOS, Grub and Startup until buildroot login...

Changing from BR2_OPTIMIZE_S to BR2_OPTIMIZE_3:

BR2_x86_64=y
BR2_OPTIMIZE_3=y
BR2_KERNEL_HEADERS_5_10=y BR2_TOOLCHAIN_BUILDROOT_WCHAR=y BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y BR2_TARGET_GENERIC_GETTY_PORT="tty1" BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh" BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh" BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-bios.cfg" BR2_LINUX_KERNEL=y BR2_LINUX_KERNEL_CUSTOM_VERSION=y BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="5.10.20" BR2_LINUX_KERNEL_DEFCONFIG="x86_64" BR2_LINUX_KERNEL_INSTALL_TARGET=y BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y BR2_TARGET_ROOTFS_EXT2=y BR2_TARGET_ROOTFS_EXT2_4=y BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
# BR2_TARGET_ROOTFS_TAR is not set BR2_TARGET_GRUB2=y
BR2_PACKAGE_HOST_GENIMAGE=y

Resulting image fails to start on real hardware, started with

$ emu-system-x86_64 -m 4096 --drive format=raw,file=images/disk.img

shows BIOS message 'Booting from Hard Disk...', 'GRUB loading...' and loops endless re-booting...

Signature · 2 lines
You are receiving this mail because:
You are on the CC list for the bug.
#1 lore raw

Hi,

following up on this because I also encountered this error.

There is a bug report on the grub bug tracker by Tony Battersby who bisected the error down to a certain patch/bugfix:
https://savannah.gnu.org/bugs/?60458

The mentioned patch has also been included in buildroot as boot/grub2/0132-kern-parser-Fix-a-stack-buffer-overflow.patch with commit e840f2d469.

I can also confirm, that removing that patch fixes the issue of grub loop of endless rebooting.

Best regards
Andreas Hilse

#2 lore raw

Andreas, All,

On 2021-05-27 17:33 +0200, Andreas Hilse via buildroot spake thusly:

Quoted 6 lines

following up on this because I also encountered this error.

There is a bug report on the grub bug tracker by Tony Battersby who bisected the error down to a certain patch/bugfix:
https://savannah.gnu.org/bugs/?60458

Quoted 6 lines

The mentioned patch has also been included in buildroot as boot/grub2/0132-kern-parser-Fix-a-stack-buffer-overflow.patch with commit e840f2d469.

I can also confirm, that removing that patch fixes the issue of grub loop of endless rebooting.

Thanks for the report.

However, this patch is a security fix, so reverting it is not so nice either.

Since it seems that using another optimisation level works arorund the issue, I would be more inclined to change that, e.g. to -Os, as the reporter seems to imply it works for them.

Would you please send a patch with such a workaround, please?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

From: Yann E. MORIN

Quoted 20 lines

Sent: 27 May 2021 16:45

Andreas, All,

On 2021-05-27 17:33 +0200, Andreas Hilse via buildroot spake thusly:

Quoted 6 lines

following up on this because I also encountered this error.

There is a bug report on the grub bug tracker by Tony Battersby who bisected the error down to a certain patch/bugfix:
https://savannah.gnu.org/bugs/?60458

Quoted 6 lines

The mentioned patch has also been included in buildroot as boot/grub2/0132-kern-parser-Fix-a-stack-buffer-overflow.patch with commit e840f2d469.

I can also confirm, that removing that patch fixes the issue of grub loop of endless rebooting.

Thanks for the report.

Quoted 8 lines

However, this patch is a security fix, so reverting it is not so nice either.

Since it seems that using another optimisation level works arorund the issue, I would be more inclined to change that, e.g. to -Os, as the reporter seems to imply it works for them.

Would you please send a patch with such a workaround, please?

Hmmmm if the code fails to compile with -O2 it is probably broken. (Unlike -O3 which tends to be a compiler bug!)

So they may have replaced one security bug with another one. The patch is too big to eyeball for errors.

I did manage to build grub with AVX support.
Spent ages working out why it wouldn't boot a kernel on an Atom. (I knew userspace was broken.)
It is probably worth passing -mno-avx when building grub.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)

#4 lore raw

David, All,

On 2021-05-27 16:28 +0000, David Laight spake thusly:

Quoted 21 lines

From: Yann E. MORIN

Quoted 20 lines

On 2021-05-27 17:33 +0200, Andreas Hilse via buildroot spake thusly:

Quoted 6 lines

following up on this because I also encountered this error.

There is a bug report on the grub bug tracker by Tony Battersby who bisected the error down to a certain patch/bugfix:
https://savannah.gnu.org/bugs/?60458

Quoted 6 lines

The mentioned patch has also been included in buildroot as boot/grub2/0132-kern-parser-Fix-a-stack-buffer-overflow.patch with commit e840f2d469.

I can also confirm, that removing that patch fixes the issue of grub loop of endless rebooting.

However, this patch is a security fix, so reverting it is not so nice either.

Since it seems that using another optimisation level works arorund the issue, I would be more inclined to change that, e.g. to -Os, as the reporter seems to imply it works for them.

Quoted 6 lines
Quoted 1 line

Would you please send a patch with such a workaround, please?

Hmmmm if the code fails to compile with -O2 it is probably broken. (Unlike -O3 which tends to be a compiler bug!)

So they may have replaced one security bug with another one.

Maybe... However, I prefer we keep the patch that is actually known for fixing a security issue, and workaround the breakage it introduces, rather than revert it just because it "may have replaced one security bug with another one."

If/when there is a report that an actual security issue is intriduced in that patch, then we can reconsider, and evaluate which issue we want to sacrifice.

Of course, if there is a better fix, then we can use that (I've looked at their git tree, there is nothing I could identify).

In the meantime, I stand by my position that we should just work around the isue by building with -Os, because at the very least it makes for a bootable system again.

Quoted 1 line

The patch is too big to eyeball for errors.

Indeed.

Quoted 4 lines

I did manage to build grub with AVX support.
Spent ages working out why it wouldn't boot a kernel on an Atom. (I knew userspace was broken.)
It is probably worth passing -mno-avx when building grub.

But this seems like a totally different issue, is it not?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

Hello Yann, David,

Quoted 1 line

Would you please send a patch with such a workaround, please?

I tried to workaround in grub2.mk setting -Os as additional flags in the TARGET_* variables for GRUB2_CONF_ENV as that seemed the right place according to this comment:
# Grub2 is kind of special: it considers CC, LD and so on to be the

# tools to build the host programs and uses TARGET_CC, TARGET_CFLAGS,
# TARGET_CPPFLAGS, TARGET_LDFLAGS to build the bootloader itself.
...
GRUB2_CONF_ENV = \
    CPP="$(TARGET_CC) -E" \
    TARGET_CC="$(TARGET_CC)" \
    TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
    TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
    TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
    TARGET_NM="$(TARGET_NM)" \
    TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
    TARGET_STRIP="$(TARGET_CROSS)strip"

But -O2 still is in the build commands in a place after -Os and thus takes preference.
Do you have any suggestions for a different workaround or on how to fix the above one?

Andreas

#6 lore raw

Andreas, All,

On 2021-05-28 15:48 +0200, Andreas Hilse via buildroot spake thusly:

Quoted 19 lines
Quoted 1 line

Would you please send a patch with such a workaround, please?

I tried to workaround in grub2.mk setting -Os as additional flags in the TARGET_* variables for GRUB2_CONF_ENV as that seemed the right place according to this comment:
# Grub2 is kind of special: it considers CC, LD and so on to be the

# tools to build the host programs and uses TARGET_CC, TARGET_CFLAGS,
# TARGET_CPPFLAGS, TARGET_LDFLAGS to build the bootloader itself.
...
GRUB2_CONF_ENV = \
    CPP="$(TARGET_CC) -E" \
    TARGET_CC="$(TARGET_CC)" \
    TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
    TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
    TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
    TARGET_NM="$(TARGET_NM)" \
    TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
    TARGET_STRIP="$(TARGET_CROSS)strip"
Quoted 4 lines

But -O2 still is in the build commands in a place after -Os and thus takes preference.
Do you have any suggestions for a different workaround or on how to fix the above one?

So, grub2 is indeed... special...

So I took the above claim aboce variable naming and their usage, and applied this patch:

    diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk
    index af16ee91e1..0e5ab89301 100644
    --- a/boot/grub2/grub2.mk
    +++ b/boot/grub2/grub2.mk
    @@ -118,9 +118,9 @@ HOST_GRUB2_CONF_ENV = \
     GRUB2_CONF_ENV = \
            CPP="$(TARGET_CC) -E" \
            TARGET_CC="$(TARGET_CC)" \
    -       TARGET_CFLAGS="$(TARGET_CFLAGS)" \
    -       TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -fno-stack-protector" \
    -       TARGET_LDFLAGS="$(TARGET_LDFLAGS)" \
    +       TARGET_CFLAGS="$(filter-out -O%,$(TARGET_CFLAGS)) -Os" \
    +       TARGET_CPPFLAGS="$(filter-out -O%,$(TARGET_CPPFLAGS)) -Os -fno-stack-protector" \
    +       TARGET_LDFLAGS="$(filter-out -O%,$(TARGET_LDFLAGS)) -Os" \
            TARGET_NM="$(TARGET_NM)" \
            TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
            TARGET_STRIP="$(TARGET_CROSS)strip"

And I configured with -Ofast (which is used nowhere in grub2's source tree, so I know it comes from Buildroot when I see it).

So, what's the CONF_ENV looking like? Seems my -Ofast is removed, and the -Os is added:

    $ make printvars VARS=GRUB2_CONF_ENV
    ... \
    TARGET_CFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=1 -Os" \
    TARGET_CPPFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -fno-stack-protector" \
    TARGET_LDFLAGS=" -Os" \
    ...

So far, so good.

But what happens when I actually build?

    $ make grub2-build >build.log
    [...]
    $ grep kern/parser.c build.log     # the file patched by the offending patch)
    .../x86_64-linux-gcc \
        -DHAVE_CONFIG_H -I. -I.. \
        -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -fno-stack-protector \
        [...] \
        -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=1 -Os
        [...] \
        -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Ofast  -D_FORTIFY_SOURCE=1 \
        -c -o kern/kernel_exec-parser.o `test -f 'kern/parser.c' || echo './'`kern/parser.c

He, what? Where's that -Ofast coming from?

    $ grep ./configure build.log
    cd /home/ymorin/dev/buildroot/O/build/grub2-2.04/ && rm -rf config.cache && \
    PATH="..." \
    [...] \
    CFLAGS_FOR_BUILD="-O2 -I/home/ymorin/dev/buildroot/O/host/include" \
    CXXFLAGS_FOR_BUILD="-O2 -I/home/ymorin/dev/buildroot/O/host/include" \
    [...] \
    CPPFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64" \
    CFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Ofast -D_FORTIFY_SOURCE=1" \
    CXXFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Ofast  -D_FORTIFY_SOURCE=1" \
    LDFLAGS="" \
    FCFLAGS=" -Ofast " \
    FFLAGS=" -Ofast " \
    [...] \
    TARGET_CFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=1 -Os" \
    TARGET_CPPFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -fno-stack-protector" \
    TARGET_LDFLAGS=" -Os" \
    [...] \
    ./configure \
    [...]

OK, so -Ofast still comes from a few locations;

- CFLAGS_FOR_BUILD or CXXFLAGS_FOR_BUILD: unlikely, or we'd also note include path pointing to the host dir, which is not the case

- FCFLAGS, FFLAGS: unlikely, this is not fortran

- CXXFLAGS: unlikely, this is not C++ code

We're left with just CPPFLAGS and CFLAGS. Those are not supposed to be used, but they are...

Meh... :-(

So, maybe things have changed from way back when we wrote that comment (back in 2014, seven years ago!), or maybe we did not undestand all of it, or maybe we did not explain it properly, or a bit of all of that...

In any case, we might want to also override CFLAGS and CPPFLAGS in the same vein as the above patch does for TARGET_CFLAGS et al.

Care to have a look at that, see if it fixes the issue, and you can get a bootable grub2, please?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#7 lore raw

Hi,

Quoted 9 lines

So, maybe things have changed from way back when we wrote that comment (back in 2014, seven years ago!), or maybe we did not undestand all of it, or maybe we did not explain it properly, or a bit of all of that...

In any case, we might want to also override CFLAGS and CPPFLAGS in the same vein as the above patch does for TARGET_CFLAGS et al.

Care to have a look at that, see if it fixes the issue, and you can get a bootable grub2, please?

Additionally overriding CFLAGS and CPPFLAGS results in a bootable grub2.

boot/grub2/grub2.mk +5 -3
index af16ee91e1..52e9199ae9 100644
@@ -118,9 +118,11 @@ HOST_GRUB2_CONF_ENV = \
 GRUB2_CONF_ENV = \
        CPP="$(TARGET_CC) -E" \
        TARGET_CC="$(TARGET_CC)" \
-       TARGET_CFLAGS="$(TARGET_CFLAGS)" \
-       TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -fno-stack-protector" \
-       TARGET_LDFLAGS="$(TARGET_LDFLAGS)" \
+       CFLAGS="$(TARGET_CFLAGS) -Os" \
+       TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
+       CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
+       TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
+       TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
        TARGET_NM="$(TARGET_NM)" \
        TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
        TARGET_STRIP="$(TARGET_CROSS)strip"

Best regards
Andreas

#8 lore raw

Andreas, All,

On 2021-05-31 12:22 +0200, Andreas Hilse via buildroot spake thusly:

Quoted 21 lines
Quoted 9 lines

So, maybe things have changed from way back when we wrote that comment (back in 2014, seven years ago!), or maybe we did not undestand all of it, or maybe we did not explain it properly, or a bit of all of that...

In any case, we might want to also override CFLAGS and CPPFLAGS in the same vein as the above patch does for TARGET_CFLAGS et al.

Care to have a look at that, see if it fixes the issue, and you can get a bootable grub2, please?

Additionally overriding CFLAGS and CPPFLAGS results in a bootable grub2.

boot/grub2/grub2.mk +0 -1
index af16ee91e1..52e9199ae9 100644
@@ -118,9 +118,11 @@ HOST_GRUB2_CONF_ENV = \
 GRUB2_CONF_ENV = \
        CPP="$(TARGET_CC) -E" \
        TARGET_CC="$(TARGET_CC)" \
-       TARGET_CFLAGS="$(TARGET_CFLAGS)" \

boot/grub2/grub2.mk HOST_GRUB2_CONF_ENV = \ · line 128

Quoted 10 lines
boot/grub2/grub2.mk +5 -2
-       TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -fno-stack-protector" \
-       TARGET_LDFLAGS="$(TARGET_LDFLAGS)" \
+       CFLAGS="$(TARGET_CFLAGS) -Os" \
+       TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
+       CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
+       TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
+       TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
        TARGET_NM="$(TARGET_NM)" \
        TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
        TARGET_STRIP="$(TARGET_CROSS)strip"

Thanks for the feedback. Could you submit a proper patch, with an actual commit log (title, body with a summary of this thread, and your SoB-line), please?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#9 lore raw

[Buildroot] [Bug 13586] grub failure with BR2_OPTIMIZE_3

In reply to bugzilla at busybox.net

https://bugs.busybox.net/show_bug.cgi?id=13586

--- Comment #1 from Peter Seiderer <[email protected]> ---
There is some follow up discussion on the mailing list, see [1], resulting in suggested patch to force '-Os' for grub compile, see [2]...

[1] http://lists.busybox.net/pipermail/buildroot/2021-May/311524.html
[2] http://lists.busybox.net/pipermail/buildroot/2021-May/311630.html

Signature · 2 lines
You are receiving this mail because:
You are on the CC list for the bug.