On 7/27/26 13:29, Ilias Apalodimas wrote:
Hi Heinrich
On Mon, 27 Jul 2026 at 14:03, Heinrich Schuchardt
<[email protected]> wrote:
From Hem Parekh <[email protected]>
This needs a :
Checkpatch complains if there is not matching Signed-off: for From:.
And Hem never properly signed the patch.
I will add the ':' when merging.
Thank you for reviewing.
Best regards
Heinrich
get_dp_device() reads a Boot#### variable and passes its contents to
efi_deserialize_load_option() but ignores the return value. On failure
efi_deserialize_load_option() may return without having initialised the
caller's struct efi_load_option, and even on a malformed device path it
sets lo.file_path before validating it with efi_dp_check_length().
As a result get_dp_device() can proceed to walk lo.file_path with
efi_dp_split_file_path() (via efi_dp_dup()/efi_dp_size()) on a device
path that was never validated, or on an uninitialised pointer when the
variable is too short to be parsed. A device-path node with a length of
zero makes the walk loop forever, and a length below the 4-byte node
header leads to an out-of-bounds read. The Boot#### variable is
attacker-controlled in threat models where writing EFI variables does
not imply the ability to execute firmware code, so this is reachable
during capsule-on-disk processing at boot.
Check the return value and bail out, as every other caller of
efi_deserialize_load_option() already does.
Suggested-by: Hem Parekh <[email protected]>
And you probably dont need this if you want to keep Hem as the author
Cc: Hem Parekh <[email protected]>
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
v2:
Use common exit out:
Remove invalid Signed-off-by: Hem Parekh <[email protected]>
Changed error message: remove word 'for'
---
lib/efi_loader/efi_capsule.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 52887f7c274..a77810fa15c 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -860,7 +860,11 @@ static efi_status_t get_dp_device(u16 *boot_var,
if (!buf)
return EFI_NOT_FOUND;
- efi_deserialize_load_option(&lo, buf, &size);
+ ret = efi_deserialize_load_option(&lo, buf, &size);
+ if (ret != EFI_SUCCESS) {
+ log_err("Invalid load option %ls\n", boot_var);
+ goto out;
efi_deserialize_load_option() will only return EFI_INVALID_PARAMETER.
For the capsule update on disk this seems correct as the EFI spec says
"Invalid capsule size, or an incompatible set of flags were set in the
capsule header. In the case of a capsule file, the file size was not
valid or an error was detected in the internal structure of the file."
We just have to make sure that we either explicitly set it or
efi_deserialize_load_option() doesn't return something invalid in the
future
In any case
Reviewed-by: Ilias Apalodimas <[email protected]>
│
+ }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
efi_dp_split_file_path(lo.file_path, device_dp, &file_dp);
@@ -871,6 +875,7 @@ static efi_status_t get_dp_device(u16 *boot_var,
ret = EFI_NOT_FOUND;
}
+out:
free(buf);
return ret;
--
2.53.0
