Bug 1652925

Summary:	annobin: Check that code at entry point starts with ENDBR32/ENDBR64 on i686/x86_64
Product:	Red Hat Enterprise Linux 8	Reporter:	Florian Weimer <fweimer>
Component:	annobin	Assignee:	Nick Clifton <nickc>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Martin Cermak <mcermak>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	8.1	CC:	fweimer, mcermak, nickc
Target Milestone:	rc
Target Release:	8.0
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	annobin-8.63-1.el8	Doc Type:	No Doc Update
Doc Text:	undefined	Story Points:	---
Clone Of:		Environment:
Last Closed:	2019-06-14 01:53:13 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Florian Weimer 2018-11-23 15:18:46 UTC

To verify the distribution-wide fix for bug 1631730, it would be great if we could enhance annobin to detect if the entry point starts with the right instruction.

A bad binary looks like this:

$ readelf -hW usr/lib/openmpi/bin/ompi_info | grep Entry
  Entry point address:               0x19d5
$ objdump -d --reloc usr/lib/openmpi/bin/ompi_info | grep -E '^\s+19d5:'
    19d5:	31 ed                	xor    %ebp,%ebp

And a good binary:

$ readelf -hW usr/lib/openmpi/bin/ompi_info | grep Entry
  Entry point address:               0x19e0
$ objdump -d --reloc  usr/lib/openmpi/bin/ompi_info | grep -E '^\s+19e0:'
    19e0:	f3 0f 1e fb          	endbr32 

So all you have to do is look up the entry point address and check that the four bytes there are 0xf3 0x0f 0x1e 0xfb (ENBR32, for i686), or 0xf3 0x0f 0x1e 0xfa (ENDBR64, for x86-64).

I think this would really help with verification of bug 1631730.  Thanks.

Comment 1 Nick Clifton 2018-11-26 11:05:23 UTC

    Hi Florian,

      I have updated the annobin sources with the check you have requested.

      I have not, however, checked these new sources into RHEL or Fedora,
      as I do not have any x86 or x86_64 binaries with the correct ENDBR
      instructions in order to test the new code.  (Is there an easy way
      to obtain such binaries).

      Also, since QE are now using annocheck as part of the rpmdiff suite,
      I did not want to risk failing lots of new builds until the updates
      to glibc, and other languages start-up code, have been put in place.

      You can download the new sources from the annobin git repository, or
      from the my Fedora web-page:

         https://nickc.fedorapeople.org/annobin-8.62.tar.xz

      Would you mind trying it out and letting me know if it does what you
      want ?

    Cheers
      Nick

Comment 2 Florian Weimer 2018-11-26 11:10:01 UTC

(In reply to Nick Clifton from comment #1)
>       I have updated the annobin sources with the check you have requested.
> 
>       I have not, however, checked these new sources into RHEL or Fedora,
>       as I do not have any x86 or x86_64 binaries with the correct ENDBR
>       instructions in order to test the new code.  (Is there an easy way
>       to obtain such binaries).

For i686, any RHEL 8 binary built after 2018-09-27 should have the correct instruction there.

For x86-64, all binaries built for Beta should have that instruction.

If you meant to write *incorrect* binaries, then openmpi-3.1.2-5.el8 will have such invalid i686 binaries.  For x86-64, you would have to go back much further and look at builds for Alpha.

Comment 3 Nick Clifton 2018-11-26 14:37:20 UTC

Hi Florian,

  Thanks.  I have now tested the new version with ENDBR type binaries
  and it works, so I have checked the change in.

  Unfortunately I have just discovered a problem.  The new annocheck
  will FAIL shared libraries because the instruction at their entry
  address is not ENDBR.  Is this wrong ?  (ie should shared objects
  also have ENDBR as their entry instruction ?)  If not, then how
  can we distinguish between a shared library and a dynamic executable ?

Cheers
  Nick

Comment 4 Florian Weimer 2018-11-26 14:47:12 UTC

(In reply to Nick Clifton from comment #3)

>   Unfortunately I have just discovered a problem.  The new annocheck
>   will FAIL shared libraries because the instruction at their entry
>   address is not ENDBR.  Is this wrong ?  (ie should shared objects
>   also have ENDBR as their entry instruction ?)  If not, then how
>   can we distinguish between a shared library and a dynamic executable ?

Yes, this is wrong.  I didn't expect shared objects to have an entry point address.  I think it just points to the start of the .text segment, and there is no expectation that this code is ever executed directly because the entry point is effectively ignored for shared objects.  (DT_INIT and DT_INITARRAY point to different places.)

I think a reasonable approximation would be to disable this check for binaries which do not have a program interpreter.

Comment 5 Nick Clifton 2018-11-26 15:13:58 UTC

Hi Florian,

  Good idea.  I have now added that check.  Please try out annobin-8.63.

Cheers
  Nick

Comment 7 Martin Cermak 2018-12-03 16:30:22 UTC

Reproduced with annobin-8.61-1.el8.

Verified with annobin-8.64-1.el8.  Annocheck now reports either
"FAIL: Entry point instruction is not ENDBR32." (and prints out
the bytes at the entry point address), or "PASS: Entry point
instruction is ENDBR.".

Tested with regular build openmpi-3.1.2-5.el8.i686 (which does
have  this problem) versus a custom scratchbuild
openmpi-3.1.2-5.mcermak.el8.i686 [1] (which doesn't).

Thanks for implementing!

-------
[1] https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=19352909