Re: [kvm-unit-tests PATCH] Makefiles: Remove the executable bit from the .elf and .flat files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/26/18 10:21, Thomas Huth wrote:
> The .elf and .flat files are not runnable on the host operating system,
> so they should not be marked as executable. As we discovered recently,
> the executable flag can also cause trouble when the files are packaged
> in an .rpm file, since rpm "colors" the package differently if there
> are 32-bit or 64-bit executables in the package (for multilib support).
> 
> Suggested-by: Laszlo Ersek <lersek@xxxxxxxxxx>
> Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
> ---
>  arm/Makefile.common     | 2 ++
>  powerpc/Makefile.common | 2 ++
>  s390x/Makefile          | 1 +
>  x86/Makefile.common     | 2 ++
>  4 files changed, 7 insertions(+)
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index 1cf9cdc..e62a978 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -73,9 +73,11 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
>  		-Wl,-T,$(SRCDIR)/arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
>  		$(filter %.o, $^) $(FLATLIBS) \
>  		$(SRCDIR)/lib/auxinfo.c -DPROGNAME=\"$(@:.elf=.flat)\" -DAUXFLAGS=$(AUXFLAGS)
> +	chmod a-x $@
>  
>  %.flat: %.elf
>  	$(OBJCOPY) -O binary $^ $@
> +	chmod a-x $@
>  
>  $(libeabi): $(eabiobjs)
>  	$(AR) rcs $@ $^
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 81c5074..af6b9d8 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -54,6 +54,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive)
>  		-T $(SRCDIR)/powerpc/flat.lds --build-id=none \
>  		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
>  	$(RM) $(@:.elf=.aux.o)
> +	chmod a-x $@
>  	@echo -n Checking $@ for unsupported reloc types...
>  	@if $(OBJDUMP) -R $@ | grep R_ | grep -v R_PPC64_RELATIVE; then	\
>  		false;							\
> @@ -70,6 +71,7 @@ $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf
>  $(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian
>  $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o
>  	$(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $<
> +	chmod a-x $@
>  
>  powerpc_clean: libfdt_clean asm_offsets_clean
>  	$(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/boot_rom.bin \
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 6546a02..6b32f2c 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -53,6 +53,7 @@ FLATLIBS = $(libcflat)
>  	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds -Ttext=0x10000 \
>  		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
>  	$(RM) $(@:.elf=.aux.o)
> +	chmod a-x $@
>  
>  arch_clean: asm_offsets_clean
>  	$(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/.*.d lib/s390x/.*.d
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 7dcf8c2..0d64284 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -42,9 +42,11 @@ FLATLIBS = lib/libcflat.a $(libgcc)
>  %.elf: %.o $(FLATLIBS) $(SRCDIR)/x86/flat.lds $(cstart.o)
>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,$(SRCDIR)/x86/flat.lds \
>  		$(filter %.o, $^) $(FLATLIBS)
> +	chmod a-x $@
>  
>  %.flat: %.elf
>  	$(OBJCOPY) -O elf32-i386 $^ $@
> +	chmod a-x $@
>  
>  tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
>                 $(TEST_DIR)/smptest.flat  $(TEST_DIR)/port80.flat \
> 

For integrity's sake, I must add the following:

While I think this change would be technically correct (= the file
should not be advertised, after build, as executable), it now seems that
the patch would not help with the packaging issue.

It appears that the RPM build infrastructure, at least in Fedora28+,
ignores the x file mode bits when it decides to color a file as "1" (=
32-bit ELF).

https://bugzilla.redhat.com/show_bug.cgi?id=1608852#c2

This behavior looks *entirely inconsistent* with how RPM building seems
to work in RHEL7, but that's the situation we got, apparently.

I believe this finding makes the commit message *in part* dubious.
Therefore, personally I'd be fine if we dropped the patch (although I do
believe what it does is still correct, just not with the original
justification).

Thanks,
Laszlo



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux