rhboot / shim

UEFI shim loader
Other
819 stars 289 forks source link

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

Closed iokomin closed 1 year ago

iokomin commented 1 year ago

Commit https://github.com/rhboot/shim/commit/0eb07e11b20680200d3ce9c5bc59299121a75388 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:

https://github.com/rhboot/shim/blob/11491619f4336fef41c3519877ba242161763580/sbat_var.S#L16-L20

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 https://github.com/rhboot/shim-review/issues/293 please check.

vathpela commented 1 year ago

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

vathpela commented 1 year ago

This should be remedied by #535 .

iokomin commented 1 year ago

@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.