Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

shim 15.7 .sbatlevel section format is not compatible with binutils older than v2.36 #533

Closed
iokomin opened this issue Dec 2, 2022 · 3 comments
Closed

shim 15.7 .sbatlevel section format is not compatible with binutils older than v2.36 #533

iokomin opened this issue Dec 2, 2022 · 3 comments

Comments

@iokomin
Copy link
Contributor

iokomin commented Dec 2, 2022

Commit 0eb07e1 introduced compatibility issue with binutils versions prior to 2.36. Shim built with older versions might not handle sbat policy properly.

Evaluation:
.sbatlevel section data composed from strings using .asciz asm directive:

shim/sbat_var.S

Lines 16 to 20 in 1149161

.Lsbat_var_previous:
.asciz SBAT_VAR_PREVIOUS
.balign 1, 0
.Lsbat_var_latest:
.asciz SBAT_VAR_LATEST

Strings provided as macro and as a result treated as a list of string literals. binutils versions <= 2.35 adds zero byte after each string literal from the macro.
For binutils-2.30-117.0.3.el8.x86_64 on Oracle Linux 8, observed section output for cmd $ objdump -s -j .sbatlevel shimx64.efi:

 85000 00000000 08000000 26000000 73626174  ........&...sbat
 85010 2c00312c 00323032 32303532 34303000  ,.1,.2022052400.
 85020 0a006772 75622c32 0a007362 61742c00  ..grub,2..sbat,.
 85030 312c0032 30323231 31313530 30000a00  1,.2022111500...
 85040 7368696d 2c320a67 7275622c 330a00    shim,2.grub,3..

Expected output:

 85000 00000000 08000000 22000000 73626174  ........"...sbat
 85010 2c312c32 30323230 35323430 300a6772  ,1,2022052400.gr
 85020 75622c32 0a007362 61742c31 2c323032  ub,2..sbat,1,202
 85030 32313131 3530300a 7368696d 2c320a67  2111500.shim,2.g
 85040 7275622c 330a00                      rub,3..

Intended behavior for these .asciz directives - for multiple string arguments not separated by commas to be concatenated together and only one final zero byte to be stored. This feature/fix apparently introduced in binutils v2.36, see commit https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3d955acb36f483c05724181da5ffba46b1303c43

@vathpela it may affect rhel7 submission rhboot/shim-review#293 please check.
@vathpela
Copy link
Contributor

vathpela commented Dec 2, 2022

Yikes! Thanks for the heads up, I'll check it out in the morning.

vathpela added a commit to vathpela/mallory that referenced this issue Dec 5, 2022
@vathpela
Make sbat_var.S parse right with buggy gcc/binutils
9d63460 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
@vathpela vathpela mentioned this issue Dec 5, 2022
Merged
vathpela added a commit to vathpela/mallory that referenced this issue Dec 6, 2022
@vathpela
Make sbat_var.S parse right with buggy gcc/binutils
4eaf288 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
vathpela added a commit that referenced this issue Dec 7, 2022
@vathpela
Make sbat_var.S parse right with buggy gcc/binutils
657b248 
In #533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
@vathpela
Copy link
Contributor

vathpela commented Dec 7, 2022

This should be remedied by #535 .

@vathpela vathpela closed this as completed Dec 7, 2022
vathpela added a commit to vathpela/mallory that referenced this issue Dec 7, 2022
@vathpela
Make sbat_var.S parse right with buggy gcc/binutils
f975a4d 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
@iokomin
Copy link
Contributor Author

iokomin commented Dec 7, 2022

@vathpela the fix is a straightforward and indeed addresses older binutils issue, thanks for it!
Built shim with the patch using ol7 binutils version - confirm that .sbatlevel section looks fine now, sanity testing for revocation scenarios also passed.

This was referenced Dec 7, 2022
Closed
Closed
This was referenced Dec 20, 2022
Closed
Closed
christoph-at-unicon pushed a commit to UniconSoftware/shim that referenced this issue Jan 2, 2023
@vathpela @christoph-at-unicon
Make sbat_var.S parse right with buggy gcc/binutils
6e48aec 
In rhboot/shim#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
dbnicholson pushed a commit to endlessm/shim that referenced this issue Apr 11, 2023
@vathpela @dbnicholson
Make sbat_var.S parse right with buggy gcc/binutils
cf1d9c8 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
(cherry picked from commit 657b248)
@aronowski aronowski mentioned this issue Sep 11, 2023
Closed
8 tasks
@aronowski aronowski mentioned this issue Sep 18, 2023
Open
jclab-joseph added a commit to jc-lab/shim-review-bot that referenced this issue Sep 19, 2023
@jclab-joseph
feat: check buggy gcc/binutils: rhboot/shim#533
7979a92
@aronowski aronowski mentioned this issue Sep 25, 2023
Closed
8 tasks
joseph-zeronsoftn pushed a commit to zeronsoftn/zerox-shim that referenced this issue Sep 25, 2023
@vathpela @joseph-zeronsoftn
Make sbat_var.S parse right with buggy gcc/binutils
ef01cef 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
This was referenced Oct 18, 2023
Closed
Closed
@aronowski aronowski mentioned this issue Dec 4, 2023
Open
8 tasks
brianredbeard pushed a commit to brianredbeard/redhat-efi-boot-shim that referenced this issue Feb 22, 2024
@vathpela @brianredbeard
Make sbat_var.S parse right with buggy gcc/binutils
7cea938 
In rhboot#533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vathpela @iokomin