[PATCH 2/2] kernel-stub: fix nullptr deref on error getting boot part UUID

[Felix Moessbauer]

The caller of disk_get_part_uuid is responsible for checking the return
value, as this function returns NULL in case of an error. This was not
done, leading to a nullptr dereference on error.

We fix this by checking the return code. Further, we now issue a error
message in case the UUID cannot be determined.

Fixes: 7c90e82 ("efi: implement systemd boot loader interface")
Reported-by: Jan Kiszka <jan.k...@siemens.com>
Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
main.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/main.c b/main.c
index f0b9aa0..e19b1f9 100644
--- a/main.c
+++ b/main.c
@@ -189,14 +189,18 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)

UINT16 *boot_medium_uuidstr =
disk_get_part_uuid(loaded_image->DeviceHandle);
- bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
- status = set_bg_interface_vars(&bg_interface_params);
- if (EFI_ERROR(status)) {
- ERROR(L"Cannot set bootloader interface variables (%r)\n",
- status);
+ if (!boot_medium_uuidstr) {
+ ERROR(L"Cannot get boot partition UUID\n");
+ } else {
+ bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
+ status = set_bg_interface_vars(&bg_interface_params);
+ if (EFI_ERROR(status)) {
+ ERROR(L"Cannot set bootloader interface variables (%r)\n",
+ status);
+ }
+ INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
+ FreePool(boot_medium_uuidstr);
}
- INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
- FreePool(boot_medium_uuidstr);
FreePool(payload_dev_path);
FreePool(boot_medium_path);

--
2.49.0

[Jan Kiszka]

Same here: WARNING, we are continuing.

> + } else {
> + bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
> + status = set_bg_interface_vars(&bg_interface_params);
> + if (EFI_ERROR(status)) {
> + ERROR(L"Cannot set bootloader interface variables (%r)\n",
> + status);
> + }
> + INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
> + FreePool(boot_medium_uuidstr);
> }
> - INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
> - FreePool(boot_medium_uuidstr);
> FreePool(payload_dev_path);
> FreePool(boot_medium_path);
>

I'm still wondering, though, what could be the root causes for any of
those to fail, specifically failing to set the variable (this is what we
most likely saw in the field).

Jan

--
Siemens AG, Foundational Technologies
Linux Expert Center

[Felix Moessbauer]

The caller of disk_get_part_uuid is responsible for checking the return
value, as this function returns NULL in case of an error. This was not
done, leading to a nullptr dereference on error.

We fix this by checking the return code. Further, we now issue a error
message in case the UUID cannot be determined.

Fixes: 7c90e82 ("efi: implement systemd boot loader interface")
Reported-by: Jan Kiszka <jan.k...@siemens.com>
Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
main.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/main.c b/main.c

index 77475cb..4a81632 100644

--- a/main.c
+++ b/main.c
@@ -189,14 +189,18 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)

UINT16 *boot_medium_uuidstr =
disk_get_part_uuid(loaded_image->DeviceHandle);
- bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
- status = set_bg_interface_vars(&bg_interface_params);
- if (EFI_ERROR(status)) {

- WARNING(L"Cannot set bootloader interface variables (%r)\n",

- status);
+ if (!boot_medium_uuidstr) {

+ WARNING(L"Cannot get boot partition UUID\n");

+ } else {
+ bg_interface_params.loader_device_part_uuid = boot_medium_uuidstr;
+ status = set_bg_interface_vars(&bg_interface_params);
+ if (EFI_ERROR(status)) {

+ WARNING(L"Cannot set bootloader interface variables (%r)\n",

+ status);
+ }
+ INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
+ FreePool(boot_medium_uuidstr);
}
- INFO(L"LoaderDevicePartUUID=%s\n", boot_medium_uuidstr);
- FreePool(boot_medium_uuidstr);
FreePool(payload_dev_path);
FreePool(boot_medium_path);

--
2.49.0