rhboot / shim

UEFI shim loader
Other
819 stars 289 forks source link

Further improve load_certs() for non-compliant drivers/firmwares #560

Closed pbatard closed 1 year ago

pbatard commented 1 year ago

Following the discovery of more problematic firmwares and drivers affected by the issue f23883ccf78f1f605a272f9e5700f47e5494a71d is designed to address (e.g. https://github.com/rhboot/shim/issues/558), this patch further improves the code so that, instead of simply bailing out, we progressively increase the buffer sizes, until either success or a maximum size limit is reached.

In most cases, this workaround should be enough to ensure completion of the directory read and thus provide full shim functionality (while still warning the user about the non-compliance of their environment).

pbatard commented 1 year ago

Just going to add that this is a follow up to #547.

We tested this patch in the context of #558 (i.e. with a file system driver that returned 0 instead of the required size on EFI_BUFFER_TOO_SMALL) and found that it was enough to complete the directory read.

Note that, as buffer size will be increased, multiple messages might be displayed, but since the point is also to alert the user about the non-compliance of their driver or firmware, we don't see these potential repeated alerts as an issue.

We set the buffer size limit for bail out to 1024, which we think should be more than enough to handle file names that are expected to be less than 512 UTF-16 characters...

vathpela commented 1 year ago

Pushed with a minor fixup as cf59f3452d478455c5f3d83790b37a372d2837ea .

pbatard commented 1 year ago

Thanks!