[Buildroot] [PATCH v2 1/2] arch: add BR2_READELF_ARCH_NAME hidden config option

[Buildroot] [PATCH v2 1/2] arch: add BR2_READELF_ARCH_NAME hidden config option

[Thomas Petazzoni]

This config option corresponds to the string returned by readelf for
the "Machine" field of the ELF header. It will be used to check if the
architecture of binaries built by Buildroot match the target
architecture.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v1:
 - None
---
 arch/Config.in            | 4 ++++
 arch/Config.in.arc        | 3 +++
 arch/Config.in.arm        | 4 ++++
 arch/Config.in.bfin       | 3 +++
 arch/Config.in.csky       | 2 ++
 arch/Config.in.m68k       | 3 +++
 arch/Config.in.microblaze | 3 +++
 arch/Config.in.mips       | 3 +++
 arch/Config.in.nios2      | 3 +++
 arch/Config.in.or1k       | 3 +++
 arch/Config.in.powerpc    | 4 ++++
 arch/Config.in.sh         | 3 +++
 arch/Config.in.sparc      | 4 ++++
 arch/Config.in.x86        | 4 ++++
 arch/Config.in.xtensa     | 3 +++
 15 files changed, 49 insertions(+)

diff --git a/arch/Config.in b/arch/Config.in
index 65a33fb..bc6434e 100644
--- a/arch/Config.in
+++ b/arch/Config.in
@@ -292,6 +292,10 @@ config BR2_GCC_TARGET_MODE
 config BR2_BINFMT_SUPPORTS_SHARED
 	bool
 
+# Must match the name of the architecture from readelf point of view
+config BR2_READELF_ARCH_NAME
+	string
+
 # Set up target binary format
 choice
 	prompt "Target Binary Format"
diff --git a/arch/Config.in.arc b/arch/Config.in.arc
index 7d341f3..dcdba68 100644
--- a/arch/Config.in.arc
+++ b/arch/Config.in.arc
@@ -38,6 +38,9 @@ config BR2_GCC_TARGET_CPU
 	default "arc700" if BR2_arc770d
 	default "archs"	 if BR2_archs38
 
+config BR2_READELF_ARCH_NAME
+	default "ARCv2"
+
 choice
 	prompt "MMU Page Size"
 	default BR2_ARC_PAGE_SIZE_8K
diff --git a/arch/Config.in.arm b/arch/Config.in.arm
index 2617976..f910364 100644
--- a/arch/Config.in.arm
+++ b/arch/Config.in.arm
@@ -568,3 +568,7 @@ config BR2_GCC_TARGET_FLOAT_ABI
 config BR2_GCC_TARGET_MODE
 	default "arm"		if BR2_ARM_INSTRUCTIONS_ARM
 	default "thumb"		if BR2_ARM_INSTRUCTIONS_THUMB || BR2_ARM_INSTRUCTIONS_THUMB2
+
+config BR2_READELF_ARCH_NAME
+	default "ARM"		if BR2_arm || BR2_armeb
+	default "AArch64"	if BR2_aarch64 || BR2_aarch64_be
diff --git a/arch/Config.in.bfin b/arch/Config.in.bfin
index 9f7056a..90e4ab9 100644
--- a/arch/Config.in.bfin
+++ b/arch/Config.in.bfin
@@ -105,3 +105,6 @@ config BR2_GCC_TARGET_CPU_REVISION
 	  value of the -mcpu option. For example, if the selected CPU is
 	  bf609, and then selected CPU revision is "0.0", then gcc will
 	  receive the -mcpu=bf609-0.0 option.
+
+config BR2_READELF_ARCH_NAME
+	default "Analog Devices Blackfin"
diff --git a/arch/Config.in.csky b/arch/Config.in.csky
index 7029c60..e88e4e2 100644
--- a/arch/Config.in.csky
+++ b/arch/Config.in.csky
@@ -44,3 +44,5 @@ config BR2_GCC_TARGET_CPU
 	default "ck810f"	if (BR2_ck810 &&  BR2_CSKY_FPU && !BR2_CSKY_DSP)
 	default "ck810ef"	if (BR2_ck810 &&  BR2_CSKY_FPU &&  BR2_CSKY_DSP)
 
+config BR2_READELF_ARCH_NAME
+	default "CSKY"
diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k
index ced871f..c56031c 100644
--- a/arch/Config.in.m68k
+++ b/arch/Config.in.m68k
@@ -35,3 +35,6 @@ endchoice
 config BR2_GCC_TARGET_CPU
 	default "68040"		if BR2_m68k_68040
 	default "5208"		if BR2_m68k_cf5208
+
+config BR2_READELF_ARCH_NAME
+	default "MC68000"
diff --git a/arch/Config.in.microblaze b/arch/Config.in.microblaze
index 2d4c1fe..042712a 100644
--- a/arch/Config.in.microblaze
+++ b/arch/Config.in.microblaze
@@ -6,6 +6,9 @@ config BR2_ENDIAN
 	default "LITTLE" if BR2_microblazeel
 	default "BIG"	 if BR2_microblazebe
 
+config BR2_READELF_ARCH_NAME
+	default "Xilinx MicroBlaze"
+
 config BR2_microblaze
 	bool
 	default y if BR2_microblazeel || BR2_microblazebe
diff --git a/arch/Config.in.mips b/arch/Config.in.mips
index ce41e9e..22819d0 100644
--- a/arch/Config.in.mips
+++ b/arch/Config.in.mips
@@ -161,3 +161,6 @@ config BR2_GCC_TARGET_ABI
 	default "32"		if BR2_MIPS_OABI32
 	default "n32"		if BR2_MIPS_NABI32
 	default "64"		if BR2_MIPS_NABI64
+
+config BR2_READELF_ARCH_NAME
+	default "MIPS R3000"
diff --git a/arch/Config.in.nios2 b/arch/Config.in.nios2
index ed63898..7466331 100644
--- a/arch/Config.in.nios2
+++ b/arch/Config.in.nios2
@@ -3,3 +3,6 @@ config BR2_ARCH
 
 config BR2_ENDIAN
 	default "LITTLE"
+
+config BR2_READELF_ARCH_NAME
+	default "Altera Nios II"
diff --git a/arch/Config.in.or1k b/arch/Config.in.or1k
index dba64a6..b31ab3e 100644
--- a/arch/Config.in.or1k
+++ b/arch/Config.in.or1k
@@ -3,3 +3,6 @@ config BR2_ARCH
 
 config BR2_ENDIAN
 	default "BIG"
+
+config BR2_READELF_ARCH_NAME
+	default "OpenRISC 1000"
diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc
index 09ac794..0968412 100644
--- a/arch/Config.in.powerpc
+++ b/arch/Config.in.powerpc
@@ -212,3 +212,7 @@ config BR2_GCC_TARGET_ABI
 	default "no-spe"		if BR2_PPC_ABI_no-spe
 	default "ibmlongdouble"		if BR2_PPC_ABI_ibmlongdouble
 	default "ieeelongdouble"	if BR2_PPC_ABI_ieeelongdouble
+
+config BR2_READELF_ARCH_NAME
+	default "PowerPC"	if BR2_powerpc
+	default "PowerPC64"	if BR2_powerpc64 || BR2_powerpc64le
diff --git a/arch/Config.in.sh b/arch/Config.in.sh
index 4705212..deb7244 100644
--- a/arch/Config.in.sh
+++ b/arch/Config.in.sh
@@ -27,3 +27,6 @@ config BR2_ARCH
 config BR2_ENDIAN
 	default "LITTLE"	if BR2_sh4 || BR2_sh4a
 	default "BIG"		if BR2_sh2a || BR2_sh4eb || BR2_sh4aeb
+
+config BR2_READELF_ARCH_NAME
+	default "Renesas / SuperH SH"
diff --git a/arch/Config.in.sparc b/arch/Config.in.sparc
index 307540f..9b6a6aa 100644
--- a/arch/Config.in.sparc
+++ b/arch/Config.in.sparc
@@ -28,3 +28,7 @@ config BR2_GCC_TARGET_CPU
 	default "leon3"		if BR2_sparc_leon3
 	default "v8"		if BR2_sparc_v8
 	default "ultrasparc"	if BR2_sparc_v9
+
+config BR2_READELF_ARCH_NAME
+	default "Sparc"		if BR2_sparc
+	default "Sparc v9"	if BR2_sparc64
diff --git a/arch/Config.in.x86 b/arch/Config.in.x86
index efa9567..0d9e93b 100644
--- a/arch/Config.in.x86
+++ b/arch/Config.in.x86
@@ -275,3 +275,7 @@ config BR2_GCC_TARGET_ARCH
 	default "c3"		if BR2_x86_c3
 	default "c3-2"		if BR2_x86_c32
 	default "geode"		if BR2_x86_geode
+
+config BR2_READELF_ARCH_NAME
+	default "Intel 80386"			if BR2_i386
+	default "Advanced Micro Devices X86-64" if BR2_x86_64
diff --git a/arch/Config.in.xtensa b/arch/Config.in.xtensa
index fcb3dc9..88dbe18 100644
--- a/arch/Config.in.xtensa
+++ b/arch/Config.in.xtensa
@@ -54,3 +54,6 @@ config BR2_ENDIAN
 
 config BR2_ARCH
 	default "xtensa"	if BR2_xtensa
+
+config BR2_READELF_ARCH_NAME
+	default "Tensilica Xtensa Processor"
-- 
2.7.4

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Thomas Petazzoni]

As shown recently by the firejail example, it is easy to miss that a
package builds and installs binaries without actually cross-compiling
them: they are built for the host architecture instead of the target
architecture.

This commit adds a small helper script, check-bin-arch, called from
the main Makefile as a TARGET_FINALIZE_HOOKS, to verify that all ELF
binaries have been built for the correct CPU architecture.

Example output with the firejail package enabled, when building for an
ARM target:

support/scripts/check-bin-arch .../buildroot/output/target .../buildroot/output/host/usr/bin/arm-linux-gnueabihf- "ARM"
ERROR: .../buildroot/output/target/usr/bin/firemon architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/bin/firejail architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/bin/firecfg architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/lib/firejail/libconnect.so architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/lib/firejail/faudit architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/lib/firejail/ftee architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/lib/firejail/libtrace.so architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
ERROR: .../buildroot/output/target/usr/lib/firejail/libtracelog.so architecture is 'Advanced Micro Devices X86-64', should be 'ARM'
Makefile:665: recipe for target 'target-finalize' failed
make[1]: *** [target-finalize] Error 1

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v1:
 - Following Yann E. Morin's comment, restrict the check to bin, lib,
   sbin, usr/bin, usr/lib and usr/sbin, in order to avoid matching
   firmware files that could use the ELF format but be targeted for
   other architectures.
---
 Makefile                       | 10 ++++++++++
 support/scripts/check-bin-arch | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100755 support/scripts/check-bin-arch

diff --git a/Makefile b/Makefile
index fb2c235..3cb1f7a 100644
--- a/Makefile
+++ b/Makefile
@@ -649,6 +649,16 @@ endef
 TARGET_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 
+READELF_ARCH_NAME = $(call qstrip,$(BR2_READELF_ARCH_NAME))
+
+ifneq ($(READELF_ARCH_NAME),)
+define CHECK_BIN_ARCH
+	support/scripts/check-bin-arch $(TARGET_DIR) \
+		$(TARGET_CROSS) "$(READELF_ARCH_NAME)"
+endef
+TARGET_FINALIZE_HOOKS += CHECK_BIN_ARCH
+endif
+
 $(TARGETS_ROOTFS): target-finalize
 
 target-finalize: $(PACKAGES)
diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
new file mode 100755
index 0000000..cb29ded
--- /dev/null
+++ b/support/scripts/check-bin-arch
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+# This script looks at all files in the target filesystem, and for
+# those that are ELF files, verifies that they have been built for the
+# correct architecture.
+
+TARGET_DIR=$1
+TARGET_CROSS=$2
+READELF_ARCH_NAME=$3
+
+exitcode=0
+
+# In order to avoid matching firmware files that could have the ELF
+# format, but for other architectures, we only look in bin, lib, sbin,
+# usr/bin, usr/lib and usr/sbin
+for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
+    # Skip non-ELF files
+    if ! file -b ${f} | grep -q "ELF " ; then
+	continue
+    fi
+
+    # Get architecture using readelf
+    farchname=$(${TARGET_CROSS}readelf -h ${f} | \
+		       grep '^  Machine:' | \
+		       sed 's/^  Machine: *\(.*\)/\1/')
+
+    if test "${farchname}" != "${READELF_ARCH_NAME}" ; then
+	echo "ERROR: ${f} architecture is '${farchname}', should be '${READELF_ARCH_NAME}'"
+	exitcode=1
+    fi
+done
+
+exit ${exitcode}
-- 
2.7.4

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Yann E. MORIN]

Thomas, All,

On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
> As shown recently by the firejail example, it is easy to miss that a
> package builds and installs binaries without actually cross-compiling
> them: they are built for the host architecture instead of the target
> architecture.
> 
> This commit adds a small helper script, check-bin-arch, called from
> the main Makefile as a TARGET_FINALIZE_HOOKS, to verify that all ELF
> binaries have been built for the correct CPU architecture.

I'd add the following:

    Two locations are excluded: /lib/firmware and /usr/lib/firmware, as
    they could contain firmware files in ELF format, for external
    processors.

[--SNIP--]
> diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> new file mode 100755
> index 0000000..cb29ded
> --- /dev/null
> +++ b/support/scripts/check-bin-arch
> @@ -0,0 +1,33 @@
> +#!/bin/bash
> +
> +# This script looks at all files in the target filesystem, and for
> +# those that are ELF files, verifies that they have been built for the
> +# correct architecture.
> +
> +TARGET_DIR=$1
> +TARGET_CROSS=$2
> +READELF_ARCH_NAME=$3
> +
> +exitcode=0
> +
> +# In order to avoid matching firmware files that could have the ELF
> +# format, but for other architectures, we only look in bin, lib, sbin,
> +# usr/bin, usr/lib and usr/sbin

Here I'd cd into TARGET_DIR first (see [0] below for the reason):

    cd "${TARGET_DIR}"
    for f in $(find ./{usr/,}{bin,lib,sbin} -type f) ; do

> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
> +    # Skip non-ELF files
> +    if ! file -b ${f} | grep -q "ELF " ; then
> +	continue

You have a mix of space-and-tab indetation. Please use only one or
the other (I favour spaces, but I'm happy with tabs as long as it is
consistent).

> +    fi
> +
> +    # Get architecture using readelf
> +    farchname=$(${TARGET_CROSS}readelf -h ${f} | \
> +		       grep '^  Machine:' | \
> +		       sed 's/^  Machine: *\(.*\)/\1/')
> +
> +    if test "${farchname}" != "${READELF_ARCH_NAME}" ; then

[0] because we could then point to the offending package:

        pkg="$( sed -r -e "\:^([^,]+),${f}$:!d; s//\1/;" "${BUILD_DIR}/packages-file-list.txt" )"

Of course, do not forget to pass BUILD_DIR when calling the script. ;-)

> +	echo "ERROR: ${f} architecture is '${farchname}', should be '${READELF_ARCH_NAME}'"

    printf 'ERROR: %s (from %s) architecture is %s, should be %s\n' \
        "${f}" "${pkg}" "${farchname}" "${READELF_ARCH_NAME}"

Regards,
Yann E. MORIN.

> +	exitcode=1
> +    fi
> +done
> +
> +exit ${exitcode}
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Yann E. MORIN]

Thomas, All,

On 2017-03-12 19:05 +0100, Yann E. MORIN spake thusly:
> On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
> > +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do

So, as I said earlier:
  - cd in TARGET_DIR first;
  - exclude .lib/firmware and /usr/lib/firmware

    cd "${TARGET_DIR}"
    find ./{usr/,}{bin,lib,sbin} \
        -type f \( \( -path './lib/firmware/*' -o -path './usr/lib/firmware*' \) \ 
                    -prune -o -print \)

Regards,
Yann E. MORIN.

> > +    # Skip non-ELF files
> > +    if ! file -b ${f} | grep -q "ELF " ; then
> > +	continue
> 
> You have a mix of space-and-tab indetation. Please use only one or
> the other (I favour spaces, but I'm happy with tabs as long as it is
> consistent).
> 
> > +    fi
> > +
> > +    # Get architecture using readelf
> > +    farchname=$(${TARGET_CROSS}readelf -h ${f} | \
> > +		       grep '^  Machine:' | \
> > +		       sed 's/^  Machine: *\(.*\)/\1/')
> > +
> > +    if test "${farchname}" != "${READELF_ARCH_NAME}" ; then
> 
> [0] because we could then point to the offending package:
> 
>         pkg="$( sed -r -e "\:^([^,]+),${f}$:!d; s//\1/;" "${BUILD_DIR}/packages-file-list.txt" )"
> 
> Of course, do not forget to pass BUILD_DIR when calling the script. ;-)
> 
> > +	echo "ERROR: ${f} architecture is '${farchname}', should be '${READELF_ARCH_NAME}'"
> 
>     printf 'ERROR: %s (from %s) architecture is %s, should be %s\n' \
>         "${f}" "${pkg}" "${farchname}" "${READELF_ARCH_NAME}"
> 
> Regards,
> Yann E. MORIN.
> 
> > +	exitcode=1
> > +    fi
> > +done
> > +
> > +exit ${exitcode}
> > -- 
> > 2.7.4
> > 
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Arnout Vandecappelle]

On 12-03-17 19:21, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2017-03-12 19:05 +0100, Yann E. MORIN spake thusly:
>> On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
>>> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
> So, as I said earlier:
>   - cd in TARGET_DIR first;
>   - exclude .lib/firmware and /usr/lib/firmware
> 
>     cd "${TARGET_DIR}"
>     find ./{usr/,}{bin,lib,sbin} \
>         -type f \( \( -path './lib/firmware/*' -o -path './usr/lib/firmware*' \) \ 
>                     -prune -o -print \)


 As I wrote in the older thread, there are also binaries in /usr/share. That
basically leaves nothing except /etc, and a specific exclusion is anyway still
needed for /lib/firmware, so I would just do "find .".

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Yann E. MORIN]

Arnout, All,

On 2017-03-12 20:43 +0100, Arnout Vandecappelle spake thusly:
> On 12-03-17 19:21, Yann E. MORIN wrote:
> > Thomas, All,
> > 
> > On 2017-03-12 19:05 +0100, Yann E. MORIN spake thusly:
> >> On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
> >>> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
> > So, as I said earlier:
> >   - cd in TARGET_DIR first;
> >   - exclude .lib/firmware and /usr/lib/firmware
> > 
> >     cd "${TARGET_DIR}"
> >     find ./{usr/,}{bin,lib,sbin} \
> >         -type f \( \( -path './lib/firmware/*' -o -path './usr/lib/firmware*' \) \ 
> >                     -prune -o -print \)
> 
> 
>  As I wrote in the older thread, there are also binaries in /usr/share.

What kind of binaries are in /usr/share? I've looked at my system
(Ubuntu 16.04, x86_64), and none of the 139615 files in there is an ELF
file.

> That
> basically leaves nothing except /etc, and a specific exclusion is anyway still
> needed for /lib/firmware, so I would just do "find .".

To be clear, you would do 'find .' instead of 'find ./{usr/,}{bin,lib,sbin}',
but still do the exclusion as I suggested above, right?

I would be OK with that. Afterall, if one is smart enough to put an ELF
file for a co-proc somewhere else than in /lib/firmware, too bad for
them.

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Arnout Vandecappelle]

On 12-03-17 21:11, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-03-12 20:43 +0100, Arnout Vandecappelle spake thusly:
>> On 12-03-17 19:21, Yann E. MORIN wrote:
>>> Thomas, All,
>>>
>>> On 2017-03-12 19:05 +0100, Yann E. MORIN spake thusly:
>>>> On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
>>>>> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
>>> So, as I said earlier:
>>>   - cd in TARGET_DIR first;
>>>   - exclude .lib/firmware and /usr/lib/firmware
>>>
>>>     cd "${TARGET_DIR}"
>>>     find ./{usr/,}{bin,lib,sbin} \
>>>         -type f \( \( -path './lib/firmware/*' -o -path './usr/lib/firmware*' \) \ 
>>>                     -prune -o -print \)
>>
>>
>>  As I wrote in the older thread, there are also binaries in /usr/share.
> 
> What kind of binaries are in /usr/share? I've looked at my system
> (Ubuntu 16.04, x86_64), and none of the 139615 files in there is an ELF
> file.

 I found /usr/share/bash-completion/helpers/gst-completion-helper-1.0 in one of
my build results. Wolfgang (now in Cc) also found something, I'm not sure if it
was the same thing or something different.


 Regards,
 Arnout

> 
>> That
>> basically leaves nothing except /etc, and a specific exclusion is anyway still
>> needed for /lib/firmware, so I would just do "find .".
> 
> To be clear, you would do 'find .' instead of 'find ./{usr/,}{bin,lib,sbin}',
> but still do the exclusion as I suggested above, right?
> 
> I would be OK with that. Afterall, if one is smart enough to put an ELF
> file for a co-proc somewhere else than in /lib/firmware, too bad for
> them.
> 
> Regards,
> Yann E. MORIN.

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Yann E. MORIN]

Arnout, All,

On 2017-03-13 18:06 +0100, Arnout Vandecappelle spake thusly:
> On 12-03-17 21:11, Yann E. MORIN wrote:
> > Arnout, All,
> > 
> > On 2017-03-12 20:43 +0100, Arnout Vandecappelle spake thusly:
> >> On 12-03-17 19:21, Yann E. MORIN wrote:
> >>> Thomas, All,
> >>>
> >>> On 2017-03-12 19:05 +0100, Yann E. MORIN spake thusly:
> >>>> On 2017-03-12 18:49 +0100, Thomas Petazzoni spake thusly:
> >>>>> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
> >>> So, as I said earlier:
> >>>   - cd in TARGET_DIR first;
> >>>   - exclude .lib/firmware and /usr/lib/firmware
> >>>
> >>>     cd "${TARGET_DIR}"
> >>>     find ./{usr/,}{bin,lib,sbin} \
> >>>         -type f \( \( -path './lib/firmware/*' -o -path './usr/lib/firmware*' \) \ 
> >>>                     -prune -o -print \)
> >>
> >>
> >>  As I wrote in the older thread, there are also binaries in /usr/share.
> > 
> > What kind of binaries are in /usr/share? I've looked at my system
> > (Ubuntu 16.04, x86_64), and none of the 139615 files in there is an ELF
> > file.
> 
>  I found /usr/share/bash-completion/helpers/gst-completion-helper-1.0 in one of
> my build results. Wolfgang (now in Cc) also found something, I'm not sure if it
> was the same thing or something different.

OK, gstreamer-1 will install its bash completion helpers in two
different locations:

  - if it finds the bash-completion >= 2.0 with pkg-config, then it uses
     to decide where to put its completion helpers,

  - otherwise, it puts them in $datadir/bash-completion/helpers

OK, we may have binary files in /usr/share/.

Note however that there is no gst-completion-helper-1.0 in my distro
(Ubuntu 16.04).

Regards,
Yann E. MORIN.

> >> That
> >> basically leaves nothing except /etc, and a specific exclusion is anyway still
> >> needed for /lib/firmware, so I would just do "find .".
> > 
> > To be clear, you would do 'find .' instead of 'find ./{usr/,}{bin,lib,sbin}',
> > but still do the exclusion as I suggested above, right?
> > 
> > I would be OK with that. Afterall, if one is smart enough to put an ELF
> > file for a co-proc somewhere else than in /lib/firmware, too bad for
> > them.
> > 
> > Regards,
> > Yann E. MORIN.
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Arnout Vandecappelle]

On 12-03-17 18:49, Thomas Petazzoni wrote:

[snip]
> +READELF_ARCH_NAME = $(call qstrip,$(BR2_READELF_ARCH_NAME))
> +
> +ifneq ($(READELF_ARCH_NAME),)

 We set it for all arches, so this is never empty. Except when we forget to set
it, but in that case we probably want this thing to be executed so we notice.

> +define CHECK_BIN_ARCH
> +	support/scripts/check-bin-arch $(TARGET_DIR) \
> +		$(TARGET_CROSS) "$(READELF_ARCH_NAME)"
> +endef
> +TARGET_FINALIZE_HOOKS += CHECK_BIN_ARCH
> +endif

 It would be nice to do it immediately after package install, so it becomes a
package error rather than some post-build error that is more difficult to
localise. Of course, with Yann's suggestion it would still report which package
was the culprit, but I don't think it would show up in the autobuild results
under the right package.

 To avoid too much overhead rechecking files all the time, it could be done as
part of step_pkg_size_end which iterates over the files installed in target for
that particular package. It would make the code a little more complicated,
though, and perhaps also slower.

> +
>  $(TARGETS_ROOTFS): target-finalize
>  
>  target-finalize: $(PACKAGES)
> diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
> new file mode 100755
> index 0000000..cb29ded
> --- /dev/null
> +++ b/support/scripts/check-bin-arch
> @@ -0,0 +1,33 @@
> +#!/bin/bash
> +
> +# This script looks at all files in the target filesystem, and for
> +# those that are ELF files, verifies that they have been built for the
> +# correct architecture.
> +
> +TARGET_DIR=$1

 Top-level Makefile already exports TARGET_DIR, so no need to pass it as an
argument. And it's not as if this script is meant to be ran outside of the build.

> +TARGET_CROSS=$2
> +READELF_ARCH_NAME=$3

 This could also be grepped out of BR2_CONFIG, like mkusers does it.
> +
> +exitcode=0
> +
> +# In order to avoid matching firmware files that could have the ELF
> +# format, but for other architectures, we only look in bin, lib, sbin,
> +# usr/bin, usr/lib and usr/sbin
> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
> +    # Skip non-ELF files
> +    if ! file -b ${f} | grep -q "ELF " ; then

 As observed by Wolfgang in the context of rpath sanitisation, file is pretty
inefficient. We came to the conclusion that the most efficient is just checking
the first 4 bytes (0x7f 0x45 0x4c 0x46). Since it is also needed for the rpath
sanitisation, perhaps we should have a common script, or even compiled code, to
do this check.

 Note however that just checking the first 4 bytes gives a bunch of false
positives, so we'd have to ignore errors from readelf as well.


> +	continue
> +    fi
> +
> +    # Get architecture using readelf
> +    farchname=$(${TARGET_CROSS}readelf -h ${f} | \
> +		       grep '^  Machine:' | \
> +		       sed 's/^  Machine: *\(.*\)/\1/')

 Why not the simpler sed -n '/^  Machine: *\(.*\)/s//\1/p' instead of the extra
grep?


 Regards,
 Arnout

> +
> +    if test "${farchname}" != "${READELF_ARCH_NAME}" ; then
> +	echo "ERROR: ${f} architecture is '${farchname}', should be '${READELF_ARCH_NAME}'"
> +	exitcode=1
> +    fi
> +done
> +
> +exit ${exitcode}
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Yann E. MORIN]

Arnout, All,

On 2017-03-12 21:27 +0100, Arnout Vandecappelle spake thusly:
> On 12-03-17 18:49, Thomas Petazzoni wrote:
> 
> [snip]
> > +READELF_ARCH_NAME = $(call qstrip,$(BR2_READELF_ARCH_NAME))
> > +
> > +ifneq ($(READELF_ARCH_NAME),)
> 
>  We set it for all arches, so this is never empty. Except when we forget to set
> it, but in that case we probably want this thing to be executed so we notice.
> 
> > +define CHECK_BIN_ARCH
> > +	support/scripts/check-bin-arch $(TARGET_DIR) \
> > +		$(TARGET_CROSS) "$(READELF_ARCH_NAME)"
> > +endef
> > +TARGET_FINALIZE_HOOKS += CHECK_BIN_ARCH
> > +endif
> 
>  It would be nice to do it immediately after package install, so it becomes a
> package error rather than some post-build error that is more difficult to
> localise. Of course, with Yann's suggestion it would still report which package
> was the culprit, but I don't think it would show up in the autobuild results
> under the right package.
> 
>  To avoid too much overhead rechecking files all the time, it could be done as
> part of step_pkg_size_end which iterates over the files installed in target for
> that particular package. It would make the code a little more complicated,
> though, and perhaps also slower.

Or it could be a step after the step_pkg_size_end, which would basically
do:

    sed -r -e "/^${pkg},(.+)$/!d; s//\1/;" "${BUILD_DIR}/packages-file-list.txt

to get the list of file installed by the current package.

Of course, that would only really and cleanly work from a clean build.

> > +    farchname=$(${TARGET_CROSS}readelf -h ${f} | \

Please be sure to run it under the C locale, to be sure we can find what
we are looking for.

> > +		       grep '^  Machine:' | \
> > +		       sed 's/^  Machine: *\(.*\)/\1/')
> 
>  Why not the simpler sed -n '/^  Machine: *\(.*\)/s//\1/p' instead of the extra
> grep?

Or a bit easier to read (at least for me):

    readelf [..] |sed -r -e '/^  Machine: +(.+)/!d; s//\1/;'

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Wolfgang Grandegger]

Hello,

Am 12.03.2017 um 21:27 schrieb Arnout Vandecappelle:
>
>
> On 12-03-17 18:49, Thomas Petazzoni wrote:
...snip...
>
>> +# In order to avoid matching firmware files that could have the ELF
>> +# format, but for other architectures, we only look in bin, lib, sbin,
>> +# usr/bin, usr/lib and usr/sbin
>> +for f in $(find ${TARGET_DIR}/{usr/,}{bin,lib,sbin} -type f) ; do
>> +    # Skip non-ELF files
>> +    if ! file -b ${f} | grep -q "ELF " ; then
>
>  As observed by Wolfgang in the context of rpath sanitisation, file is pretty
> inefficient. We came to the conclusion that the most efficient is just checking
> the first 4 bytes (0x7f 0x45 0x4c 0x46). Since it is also needed for the rpath
> sanitisation, perhaps we should have a common script, or even compiled code, to
> do this check.
>
>  Note however that just checking the first 4 bytes gives a bunch of false
> positives, so we'd have to ignore errors from readelf as well.

Using readelf for ELF file checking is as fast checking the first 4 
bytes. My favorite solution is:

       header=$(readelf -h "${f}" 2>/dev/null)
       if [ "$?" -eq 0 ]; then
	... process header ...
       fi

Wolfgang.

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Arnout Vandecappelle]

 I forgot to mention this:

> +    # Get architecture using readelf
> +    farchname=$(${TARGET_CROSS}readelf -h ${f} | \

 Since this is the only use of TARGET_CROSS, it would make more sense to pass
TARGET_READELF.

 We could even use the host readelf, except I'm not sure that older readelf
binaries know about newish architectures like ARC. Trying with an old 2.20
binary... Right, it *can* read an ARC file, but it reports
  Machine:                           <unknown>: 0xc3

 So, pass TARGET_READELF.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 2/2] Makefile: add check of binaries architecture

[Thomas Petazzoni]

Hello,

On Sun, 12 Mar 2017 21:27:41 +0100, Arnout Vandecappelle wrote:

> > +ifneq ($(READELF_ARCH_NAME),)  
> 
>  We set it for all arches, so this is never empty. Except when we forget to set
> it, but in that case we probably want this thing to be executed so we notice.

It was sometimes empty at the beginning of my tests, and then I decided
to provide the value for all architectures we support. So yes, this
test has been removed in my v3.

>  It would be nice to do it immediately after package install, so it becomes a
> package error rather than some post-build error that is more difficult to
> localise. Of course, with Yann's suggestion it would still report which package
> was the culprit, but I don't think it would show up in the autobuild results
> under the right package.
> 
>  To avoid too much overhead rechecking files all the time, it could be done as
> part of step_pkg_size_end which iterates over the files installed in target for
> that particular package. It would make the code a little more complicated,
> though, and perhaps also slower.

I've implemented this in my v3. The code is not much more complicated
in fact.

>  Top-level Makefile already exports TARGET_DIR, so no need to pass it as an
> argument. And it's not as if this script is meant to be ran outside of the build.

OK.

>  This could also be grepped out of BR2_CONFIG, like mkusers does it.

Implemented.

>  As observed by Wolfgang in the context of rpath sanitisation, file is pretty
> inefficient. We came to the conclusion that the most efficient is just checking
> the first 4 bytes (0x7f 0x45 0x4c 0x46). Since it is also needed for the rpath
> sanitisation, perhaps we should have a common script, or even compiled code, to
> do this check.

I'm not using just readelf, and if the sed expression extracting the
Machine value returns an empty string, assume it's not an ELF file.

It would be nice to get the return value of readelf, but being in a
subshell, before a pipe, it's not trivial (at least not trivial
compared to my limited bash-fu).

>  Why not the simpler sed -n '/^  Machine: *\(.*\)/s//\1/p' instead of the extra
> grep?

Implemented, using Yann's suggestion.

Thanks for the useful suggestions!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com