compute the bootloader stage size to workaround PIE

[sharkcz]

This should allow building the user-space zipl correctly with PIE. Fedora embeds the hardening flags in the compiler and linker flags, so after fixing https://bugzilla.redhat.com/show_bug.cgi?id=1552661 zipl started to fail with "Error: Internal error: Size mismatch of FBA stage 0 loader".

The bootloader stages run during IPL are built with their own flags and converted to raw binaries. These binaries are then converted to ELF objects with objdump and linked together to create data.o and data.h files (see boot/Makefile). During the bin->ELF conversion 3 symbols are added - start, end and size. When zipl is built with PIE the "size" symbols is relocated and the comparisons in boot_check_data() fail. "size" is marked as ABS in readelf output, so maybe there is also a binutils/linker bug. At the end instead of using the provided "size" we compute that from the "end" and "start" pointers and all seems to work correctly.

The command line we use to build zipl (and the whole s390-tools) is

make CFLAGS="$(rpmbuild --eval %{build_cflags})" CXXFLAGS="$(rpmbuild --eval %{build_cflags})" LDFLAGS="$(rpmbuild --eval %{build_ldflags})" BINDIR=/usr/sbin DISTRELEASE=2.fc29 V=1

[sharkcz]

pie.sh.gz - a script to reproduce the mentioned behaviour

[sharkcz]

https://github.com/ibm-s390-tools/s390-tools/blob/master/zipl/src/boot.c#L41 is the location of the failing checks

[fweimer]

_binary_data_bin_size is an SHN_ABS symbol. Absolute relocations are problematic for most ELF targets, and there are undoubtedly many remaining bugs. In this case, the link editor should perhaps refuse to create an executable because there is no suitable S/390 relocation to express what is needed here. Or generate a R_390_GLOB_DAT relocation for an SHN_ABS symbol and hope that the glibc bug in this area has been fixed.

This happens to work:

size_t size = &_binary_data_bin_end - &_binary_data_bin_start;                                                          

But I think it will break if the file size is odd because on S/390, all global objects must have even addresses.

The right solution seems to be to use nm to read the size of the A symbol and generate a C constant from that directly, and avoid the indirection through the linker.

[stefan-haberland]

Hi,

sorry for the late reply. I just tried to reproduce the problem....

Just for my understanding: The fix for https://bugzilla.redhat.com/show_bug.cgi?id=1552661 is to replace all LDFLAGS by the system wide LDFLAGS? Afterwards zipl fails because it is build with gcc -fno-pie but linked without -no-pie.

You do not replace the CFLAGS, don't you?

Overall, for me it feels strange to not have the corresponding linker flag set to the -fno-pie CFLAG.

[sharkcz]

We set the CFLAGS/CXXFLAGS/LDFLAGS to a distro defined ones (https://src.fedoraproject.org/rpms/s390utils/blob/master/f/s390utils.spec#_91) as they are a superset of the upstream ones.

zipl itself has 2 parts - first is the "boot records" in zipl/boot and it filters out any "pie" flags, they correctly use "CFLAGS_BOOT" in https://github.com/ibm-s390-tools/s390-tools/blob/master/zipl/boot/Makefile. These boot records are then transformed into "data" that are bundled to the user-space zipl binary. The user-space zipl is built with distro CFLAGS/LDFLAGS that include PIE. The whole build log is at https://kojipkgs.fedoraproject.org//packages/s390utils/2.4.0/1.fc29/data/logs/s390x/build.log The gcc/ld spec files are at https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/master/f/redhat-hardened-cc1 and https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/master/f/redhat-hardened-ld (they contain the distro "PIE" handling). Every boot record is identified in the user-space zipl with start, end and size constants. The problem is that "size" also treated as relocatable symbol, while it shouldn't, see Florian's comment for details.

As a whole I think we use the distro *FLAGS as consistent as possible in s390-tools.

[stefan-haberland]

OK, thanks for clarification.

[Andreas-Krebbel]

Looks like a problem in Binutils :( An ld optimization rewrites a GOT access with lgrl r1, x@GOTENT to larl r1, x if it detects the symbol being referenced locally. Bug 1 is that it does this also for SHN_ABS symbols. Bug 2 is that it doesn't even check whether the symbol resides at an even address. I'll fix this in Binutils.

However, as an Binutils-independent fix I agree that the proposal from Florian is a nice way to solve it. I don't think we really have to care about odd lengths here since the boot loader stages are generated from .S files and gas will pad the sections to a 4 byte boundary.

Longterm I think it would be better to include the bootloader code into the zipl binary using the .incbin gas directive in an inline assembly in the code. Labels around it could be used to perform the size sanity checking.

[Andreas-Krebbel]

I finally got around to fixing this in mainline Binutils: https://sourceware.org/ml/binutils/2018-09/msg00201.html

[xnox]

Thanks. In Ubuntu, I have PIE disabled for zipl as well. I don't think many bootloader code is PIE capable. I think we disable it for a few of bootloader-like things i.e. qemu firmwares, and the like.

[sharkcz]

PIE (and hardening in general) is enabled in the user-space zipl command line tool, not in the actual bootloader, which uses its own set of compiler flags anyway.