[PATCH v3] Hexagon: implement machine flag check

[PATCH v3] Hexagon: implement machine flag check

[Matheus Tavares Bernardino]

This fixes the "invalid machine flag" error from eu-elflint when passing
hexagon binaries.

        * backends/hexagon_init.c (hexagon_init): Hook
        machine_flag_check
        * backends/hexagon_symbol.c (hexagon_machine_flag_check):
	new function
        * libelf/elf-knowledge.h: add EF_HEXAGON_TINY constant

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---

v2: https://sourceware.org/pipermail/elfutils-devel/2024q2/006987.html

Changes in v3:
- Added ChangeLog to commit message.
- Implemented better machine_flag_check operation, as suggested by
  bcain.
- Extracted only patch 2/2 as 1/2 was already merged.

 backends/hexagon_init.c   | 1 +
 backends/hexagon_symbol.c | 7 +++++++
 libelf/elf-knowledge.h    | 1 +
 3 files changed, 9 insertions(+)

diff --git a/backends/hexagon_init.c b/backends/hexagon_init.c
index 9c8c6d8d..1cd27513 100644
--- a/backends/hexagon_init.c
+++ b/backends/hexagon_init.c
@@ -45,6 +45,7 @@ hexagon_init (Elf *elf __attribute__ ((unused)),
 {
   hexagon_init_reloc (eh);
   HOOK (eh, reloc_simple_type);
+  HOOK (eh, machine_flag_check);
 
   return eh;
 }
diff --git a/backends/hexagon_symbol.c b/backends/hexagon_symbol.c
index b341243e..5f6e0fe5 100644
--- a/backends/hexagon_symbol.c
+++ b/backends/hexagon_symbol.c
@@ -56,3 +56,10 @@ hexagon_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type,
       return ELF_T_NUM;
     }
 }
+
+bool
+hexagon_machine_flag_check (GElf_Word flags)
+{
+  GElf_Word reserved_flags = ~(EF_HEXAGON_TINY | EF_HEXAGON_MACH);
+  return (flags & reserved_flags) == 0;
+}
diff --git a/libelf/elf-knowledge.h b/libelf/elf-knowledge.h
index 71535934..23e34ca1 100644
--- a/libelf/elf-knowledge.h
+++ b/libelf/elf-knowledge.h
@@ -119,6 +119,7 @@
 #define EF_HEXAGON_MACH_V71T 0x00008071 /* Hexagon V71T */
 #define EF_HEXAGON_MACH_V73  0x00000073 /* Hexagon V73 */
 #define EF_HEXAGON_MACH      0x000003ff /* Hexagon V.. */
+#define EF_HEXAGON_TINY      0x00008000 /* Hexagon V..T */
 
 /* Special section indices.  */
 #define SHN_HEXAGON_SCOMMON    0xff00 /* Other access sizes */
-- 
2.37.2

Re: [PATCH v3] Hexagon: implement machine flag check

[Mark Wielaard]

Hi Matheus,

On Thu, Apr 04, 2024 at 02:19:40PM -0300, Matheus Tavares Bernardino wrote:
> This fixes the "invalid machine flag" error from eu-elflint when passing
> hexagon binaries.
> 
>         * backends/hexagon_init.c (hexagon_init): Hook
>         machine_flag_check
>         * backends/hexagon_symbol.c (hexagon_machine_flag_check):
> 	new function
>         * libelf/elf-knowledge.h: add EF_HEXAGON_TINY constant
> 
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
> 
> v2: https://sourceware.org/pipermail/elfutils-devel/2024q2/006987.html

Note that we also have a public-inbox instance, which is useful in
some cases since you can address patches by message-id:
https://inbox.sourceware.org/elfutils-devel/d198f1806a486bd5129c996f10680b947ecdc581.1712087613.git.quic_mathbern@quicinc.com/

> Changes in v3:
> - Added ChangeLog to commit message.
> - Implemented better machine_flag_check operation, as suggested by
>   bcain.
> - Extracted only patch 2/2 as 1/2 was already merged.

This looks good.

Pushed,

Mark

Re: [PATCH v3] Hexagon: implement machine flag check

[Matheus Tavares Bernardino]

On Thu, Apr 04, 2024 at 21:43:11 +0200, Mark Wielaard wrote:
>
> 
> Note that we also have a public-inbox instance, which is useful in
> some cases since you can address patches by message-id:
> https://inbox.sourceware.org/elfutils-devel/d198f1806a486bd5129c996f10680b947ecdc581.1712087613.git.quic_mathbern@quicinc.com/

Awesome! I much rather prefer the public-inbox interface :) Thanks!

BTW, just out of curiosity, since the last incident with xz's backdoor
(which apparently involved malicious code disguised as a test binary),
has the elfutils community already considered using something like
Dockerfiles to generate the tests/*.ko.bz2 binaries instead of checking
than in the git repo? Just something that crossed my mind while I was
developing these patches.

RE: [PATCH v3] Hexagon: implement machine flag check

[Andrew Pinski]

> -----Original Message-----
> From: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>
> Sent: Thursday, April 4, 2024 12:57 PM
> To: mark@klomp.org
> Cc: Brian Cain <bcain@quicinc.com>; elfutils-devel@sourceware.org; Andrew
> Pinski (QUIC) <quic_apinski@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>
> Subject: Re: [PATCH v3] Hexagon: implement machine flag check
> 
> On Thu, Apr 04, 2024 at 21:43:11 +0200, Mark Wielaard wrote:
> >
> >
> > Note that we also have a public-inbox instance, which is useful in
> > some cases since you can address patches by message-id:
> > https://inbox.sourceware.org/elfutils-
> devel/d198f1806a486bd5129c996f10
> > 680b947ecdc581.1712087613.git.quic_mathbern@quicinc.com/
> 
> Awesome! I much rather prefer the public-inbox interface :) Thanks!
> 
> BTW, just out of curiosity, since the last incident with xz's backdoor (which
> apparently involved malicious code disguised as a test binary), has the elfutils
> community already considered using something like Dockerfiles to generate
> the tests/*.ko.bz2 binaries instead of checking than in the git repo? Just
> something that crossed my mind while I was developing these patches.

That came across my mind too. 

Especially after Mark wrote backdoor on what might need to be improved for sourceware and what tooling is needed for the projects hosted on sourceware (which is most of the GNU toolchain):
https://inbox.sourceware.org/gcc/20240329203909.GS9427@gnu.wildebeest.org/T/#mc980204081b149132aeace4bf3d83cf35dad72d8

Thanks,
Andrew Pinski

Re: [PATCH v3] Hexagon: implement machine flag check

[Mark Wielaard]

Hi Matheus,

On Thu, 2024-04-04 at 16:56 -0300, Matheus Tavares Bernardino wrote:
> BTW, just out of curiosity, since the last incident with xz's backdoor
> (which apparently involved malicious code disguised as a test binary),
> has the elfutils community already considered using something like
> Dockerfiles to generate the tests/*.ko.bz2 binaries instead of checking
> than in the git repo? Just something that crossed my mind while I was
> developing these patches.

Good question. Especially since I have been poking backend developers
to add more of them... I think the honest question is, no we haven't
considered, or even discussed, getting rid of them. So lets :)

In the xz-backdoor case it was actually hidden in a test binary which
wasn't actually used in the testsuite. So that is certainly something
to watch out for. Does someone add a binary file for no good reason?
Also this seems to be a somewhat sophisticated hack and the would
probably found some other way to hide something.

But you are right of course that hiding something is much easier in a
binary file. So it is reasonable to ask what the alternatives are.

We do actually have native testcases. So we could just rely on those.
But it is really helpful to see if the code works cross-32/64 bit and
cross-endian. Because one of the advantages of elfutils is that you can
inspect and manipulate binaries from other arches.

Another might be what bzip2 does, have all these binaries in a separate
test git repo https://sourceware.org/cgit/bzip2-tests/tree/README
But that kind of just moves the issue. We would encourage people to
always check out that repo and run all tests in it.

Another would be what you suggest. Create containers for all arches
supported and (re)generate all test binaries in that container. But
that would be a lot of containers and for some arches you like to have
different versions of the tools to generate them. And can that be done
for all arches? e.g. Does hexagon have qemu support? One advantage of
this might be that you could just rely on the native tests. Assuming we
add them all to builder.sourceware.org and run them on each commit.

Finally we might want to only create binary files from some easily
understood textual representation. Maybe using GNU Poke ELF pickles?
https://jemarch.net/poke-elf-1.0-manual/poke-elf.html
That sounds really attractive, but it would be a lot of work recreating
the existing binary test files. And nobody has tried yet, so we don't
know if this (or some other tool) is expressive enough.

Cheers,

Mark

Re: [PATCH v3] Hexagon: implement machine flag check

[Matheus Tavares Bernardino]

On Fri, 05 Apr 2024 16:45:40 +0200 Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Matheus,
> 
> On Thu, 2024-04-04 at 16:56 -0300, Matheus Tavares Bernardino wrote:
> > BTW, just out of curiosity, since the last incident with xz's backdoor
> > (which apparently involved malicious code disguised as a test binary),
> > has the elfutils community already considered using something like
> > Dockerfiles to generate the tests/*.ko.bz2 binaries instead of checking
> > than in the git repo? Just something that crossed my mind while I was
> > developing these patches.
> 
> [...] 
> In the xz-backdoor case it was actually hidden in a test binary which
> wasn't actually used in the testsuite. So that is certainly something
> to watch out for. Does someone add a binary file for no good reason?
> Also this seems to be a somewhat sophisticated hack and the would
> probably found some other way to hide something.

Good point :)

> Another would be what you suggest. Create containers for all arches
> supported and (re)generate all test binaries in that container. But
> that would be a lot of containers and for some arches you like to have
> different versions of the tools to generate them. And can that be done
> for all arches? e.g. Does hexagon have qemu support?

It does :) But I was actually thinking about using the containers to
cross-build the binaries, like we do for the QEMU tests. E.g.
https://github.com/qemu/qemu/blob/master/tests/docker/dockerfiles/debian-hexagon-cross.docker

Nonetheless, yeah, that will be a lot of containers, and a significant ammount
of work.