Null-terminate 'arguments' in fallback #611
Conversation
In case CSV entry contains boot argument (e.g. an image to load for shim) it must be null-terminated. While populate_stanza() makes sure 'arguments' end with '\0', add_boot_option() doesn't account for it in 'size' calculations. E.g. for the following CSV entry: shimx64.efi,6.6.0-0.rc0.20230904git708283abf896.6.fc40.x86_64,\EFI\Linux\5f93b3c9cf1c488a99786fb8e99fb840-6.6.0-0.rc0.20230904git708283abf896.6.fc40.x86_64.efi,Comment the resulting variable after 'fallback' looks like: # hexdump /sys/firmware/efi/efivars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c | tail -3 0000180 0038 0036 005f 0036 0034 002e 0065 0066 0000190 0069 0000192 Add trailing '\0' to 'size' calculations in add_boot_option() when 'arguments' is not empty. The resulting variable looks like: # hexdump /sys/firmware/efi/efivars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c | tail -3 0000180 0038 0036 005f 0036 0034 002e 0065 0066 0000190 0069 0000 0000194 and the specified image is loaded by shim without issues. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
|
ping? |
|
FWIW, the message is misleading you into thinking it's a single \0 byte, but it's a \0 UCS-2 character (i.e. 2 bytes). But yes I believe this is useful. It's not strictly necessary: As I'm sure you know, you can always append a space to the entry in the .csv and it works. But it's an ugly workaround for sure. I can't speak to whether the implementation is otherwise correct as I haven't looked at it in detail, mostly just tracing the reasoning with my work. Generally I'd favor not copying trailing \0s, but append them explicitly again, but I don't have enough context in the code here. |
|
The workaround using extra space is indeed known and is already used in the wild, e.g. |
In case CSV entry contains boot argument (e.g. an image to load for shim) it must be null-terminated. While populate_stanza() makes sure 'arguments' end with '\0', add_boot_option() doesn't account for it in 'size' calculations. E.g. for the following CSV entry:
the resulting variable after 'fallback' looks like:
Add trailing '\0' to 'size' calculations in add_boot_option() when 'arguments' is not empty. The resulting variable looks like:
and the specified image is loaded by shim without issues.