When android_image_get_vendor_bootimg_size is called, its buffer is only
allocated with enough space for the bootconfig header, but the
android_vendor_boot_image_v3_v4_parse_hdr helper attempts to append a
bootconfig trailer to it, causing an out-of-bounds access and heap
corruption in some cases (e.g. triggered in sandbox test builds when
extra bootmeths are added, resulting in a segfault of the sandbox
process).
Skip the dangerous memcpy operations altogether when the
android_vendor_boot_image_v3_v4_parse_hdr helper is only called for size
calculation purposes, and only append the bootconfig trailer when called
from the actual bootconfig parsing code path.
Fixes: 57e405e1f474 ("android: boot: support bootconfig")
Signed-off-by: Alexey Charkov <[email protected]>
---
This was uncovered while adjusting the test suite to cover the BLS type 1
boot method following Simon's feedback at [1]. While my BLS code did
indeed leak memory as Simon pointed out (tripping some of the tests),
it also happened to uncover a latent bug in the existing Android boot
method which only seems to be triggered when the heap is pre-filled with
more boot methods than were used in the master branch so far, so even
fixed BLS code still resulted in a failing test there.
[1]
https://lore.kernel.org/u-boot/caflsztikihqsumogdmb8q9wypiehb0nq-a7vxh5xrxnxrve...@mail.gmail.com/
---
boot/image-android.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c
index fb26290d40c7..7740cae8cb62 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -131,7 +131,8 @@ static void android_boot_image_v3_v4_parse_hdr(const struct
andr_boot_img_hdr_v3
}
static void android_vendor_boot_image_v3_v4_parse_hdr(const struct
andr_vnd_boot_img_hdr
- *hdr, struct
andr_image_data *data)
+ *hdr, struct
andr_image_data *data,
+ bool write_trailer)
{
ulong end;
@@ -167,12 +168,23 @@ static void
android_vendor_boot_image_v3_v4_parse_hdr(const struct andr_vnd_boot
end += ALIGN(hdr->vendor_ramdisk_table_size, hdr->page_size);
data->bootconfig_addr = end;
if (hdr->bootconfig_size) {
- void *bootconfig_ptr = map_sysmem(data->bootconfig_addr,
- data->bootconfig_size +
- BOOTCONFIG_TRAILER_SIZE);
- data->bootconfig_size += add_trailer((ulong)bootconfig_ptr,
- data->bootconfig_size);
- unmap_sysmem(bootconfig_ptr);
+ if (write_trailer) {
+ void *bootconfig_ptr = map_sysmem(data->bootconfig_addr,
+ data->bootconfig_size
+
+
BOOTCONFIG_TRAILER_SIZE);
+ data->bootconfig_size +=
add_trailer((ulong)bootconfig_ptr,
+
data->bootconfig_size);
+ unmap_sysmem(bootconfig_ptr);
+ } else {
+ /*
+ * Only the header has been loaded here (this is a
+ * size-only query), so the bootconfig region is not
+ * present in the buffer. Account for the trailer that
+ * will be appended at load time without writing it, to
+ * avoid corrupting memory past the header buffer.
+ */
+ data->bootconfig_size += BOOTCONFIG_TRAILER_SIZE;
+ }
data->ramdisk_size += data->bootconfig_size;
}
end += ALIGN(data->bootconfig_size, hdr->page_size);
@@ -265,7 +277,7 @@ bool android_image_get_vendor_bootimg_size(const void *hdr,
u32 *vendor_boot_img
return false;
}
- android_vendor_boot_image_v3_v4_parse_hdr(hdr, &data);
+ android_vendor_boot_image_v3_v4_parse_hdr(hdr, &data, false);
*vendor_boot_img_size = data.vendor_boot_img_total_size;
@@ -304,7 +316,7 @@ bool android_image_get_data(const void *boot_hdr, const
void *vendor_boot_hdr,
return false;
}
android_boot_image_v3_v4_parse_hdr((const struct
andr_boot_img_hdr_v3 *)bhdr, data);
- android_vendor_boot_image_v3_v4_parse_hdr(vhdr, data);
+ android_vendor_boot_image_v3_v4_parse_hdr(vhdr, data, true);
unmap_sysmem(vhdr);
} else {
android_boot_image_v0_v1_v2_parse_hdr(bhdr, data);
---
base-commit: a18265f1ccb7a272721ed4286ed3b5a6182ff424
change-id: 20260708-image-android-oob-fix-8b8ceb17604e
Best regards,
--
Alexey Charkov <[email protected]>
