[PATCH v2 1/5] boot/image-android: Workaround kernel/ramdisk invalid addr

george chan gchan9527 at gmail.com
Thu May 8 06:22:54 CEST 2025


Hi Mattijs,

Thx for feedback.

在 2025年5月7日週三 15:47,Mattijs Korpershoek <mkorpershoek at kernel.org> 寫道:

> Hi George,
>
> Thank you for the patch.
> I really prefer this solution to the one proposed in v1.
>
> I have tested this on Khadas VIM3 board (using
> khadas-vim3_android_defconfig)
>
> I have a couple of small remarks below.
>
> On lun., mai 05, 2025 at 17:17, George Chan via B4 Relay <
> devnull+gchan9527.gmail.com at kernel.org> wrote:
>
> > From: George Chan <gchan9527 at gmail.com>
> >
> > Some androidboot image have invalid kernel/ramdisk load addr,
> > force to ignore those value and use loadaddr instead.
>
> Can we please elaborate a bit more in the commit message?
> We have not explained/justified why repacking a boot image with proper
> kernel ramdisk/load addr is not possible.
>
> Is the following an acceptable justification for you?
> """
> Since it's not always possible to change the load addr by repacking
> the boot.img (due to AVB signature mismatch/<or_another_justification>),
> we need a way to use kernel_addr_r and ramdisk_addr_r.
> """
>
> Feel free to reformulate the above.
>

Thx. let me sum up as below 2 reasons.

1. there is a concern on exposing the whole memory to image loading is
dangerous

2. Since it's not always possible to change the load addr by repacking
the boot.img (mainly due to AVB signature mismatch),
we need a way to use kernel_addr_r and ramdisk_addr_r.


> >
> > Suggested-by: Casey Connolly <casey.connolly at linaro.org>
> > Signed-off-by: George Chan <gchan9527 at gmail.com>
> > ---
> >  boot/Kconfig         | 6 ++++++
> >  boot/image-android.c | 9 ++++++---
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index fb37d912bc9..4bdac384181 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -11,6 +11,12 @@ config ANDROID_BOOT_IMAGE
> >         This enables support for booting images which use the Android
> >         image format header.
> >
> > +config ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR
> > +     bool "Android Boot Image ignore addr"
> > +     default n
> > +     help
> > +       This ignore kernel/ramdisk load addr specified in androidboot
> header.
>
> Please add a bit more context here, checkpatch.pl reports issues:
> $ ./scripts/checkpatch.pl --u-boot --git master..HEAD
>
> WARNING: please write a paragraph that describes the config symbol fully
> #27: FILE: boot/Kconfig:14:
> +config ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR
>

same as above.

>
> > +
> >  config TIMESTAMP
> >       bool "Show image date and time when displaying image information"
> >       default y if CMD_DATE
> > diff --git a/boot/image-android.c b/boot/image-android.c
> > index 1746b018900..7b8eb6a4f64 100644
> > --- a/boot/image-android.c
> > +++ b/boot/image-android.c
> > @@ -268,7 +268,8 @@ static ulong android_image_get_kernel_addr(struct
> andr_image_data *img_data,
> >        *
> >        * Otherwise, we will return the actual value set by the user.
> >        */
> > -     if (img_data->kernel_addr  == ANDROID_IMAGE_DEFAULT_KERNEL_ADDR) {
> > +     if (img_data->kernel_addr  == ANDROID_IMAGE_DEFAULT_KERNEL_ADDR ||
> > +             IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR)) {
>
> Please fix indentation, should be:
>
> +       if (img_data->kernel_addr  == ANDROID_IMAGE_DEFAULT_KERNEL_ADDR ||
> +           IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR)) {
>
> (checkpatch.pl reports this)
>

ack.


> >               if (comp == IH_COMP_NONE)
> >                       return img_data->kernel_ptr;
> >               return env_get_ulong("kernel_addr_r", 16, 0);
> > @@ -464,7 +465,8 @@ int android_image_get_ramdisk(const void *hdr, const
> void *vendor_boot_img,
> >        */
> >       if (img_data.header_version > 2) {
> >               /* Ramdisk can't be used in-place, copy it to
> ramdisk_addr_r */
> > -             if (img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {
> > +             if (img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR ||
> > +
>  (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR))) {
>
> Same here. Also, can we please drop the leading parenthesis (IS_ENABLED(
> -> IS_ENABLED( to be consistent with the previous code change?
>

ack.


> >                       ramdisk_ptr = env_get_ulong("ramdisk_addr_r", 16,
> 0);
> >                       if (!ramdisk_ptr) {
> >                               printf("Invalid ramdisk_addr_r to copy
> ramdisk into\n");
> > @@ -488,7 +490,8 @@ int android_image_get_ramdisk(const void *hdr, const
> void *vendor_boot_img,
> >       } else {
> >               /* Ramdisk can be used in-place, use current ptr */
> >               if (img_data.ramdisk_addr == 0 ||
> > -                 img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {
> > +                 img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR ||
> > +
>  (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR))) {
>
> Same here. Also, can we please drop the leading parenthesis (IS_ENABLED(
> -> IS_ENABLED( to be consistent with the previous code change?
>

ack.

l planed to release v3 later around end of next week and contain only patch
#1 #2 and #5 to ease maintainer processing. please feel free to comment on
any idea, i will include into next version.

regards,
George

>


More information about the U-Boot mailing list