[PATCH v2] boot: android: fix booting without a ramdisk

[PATCH v2] boot: android: fix booting without a ramdisk

[Michael Walle]

android_image_get_ramdisk() will return an error if there is no ramdisk.
Using the android image without a ramdisk worked until commit
1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
the return code wasn't checked until then. Return -ENOENT in case
there is no ramdisk and translate that into -ENOPKG in the calling
code, which will then indicate "no ramdisk" to its caller
(boot_get_ramdisk()).

This way, we can get rid of the "*rd_data = *rd_len = 0;" in the error
path, too.

With this, I'm able to boot a linux kernel using fastboot again:

  fastboot --base 0x41000000 --header-version 2 --dtb /path/to/dtb \
  --cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 boot/image-android.c | 7 +++----
 boot/image-board.c   | 4 +++-
 include/image.h      | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index 09c7a44e058..774565fd1fe 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -393,10 +393,9 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
 	if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
 		return -EINVAL;
 
-	if (!img_data.ramdisk_size) {
-		*rd_data = *rd_len = 0;
-		return -1;
-	}
+	if (!img_data.ramdisk_size)
+		return -ENOENT;
+
 	if (img_data.header_version > 2) {
 		ramdisk_ptr = img_data.ramdisk_addr;
 		memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
diff --git a/boot/image-board.c b/boot/image-board.c
index f2124013046..eca1b1d2bff 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -427,7 +427,9 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a
 				unmap_sysmem(ptr);
 			}
 
-			if (ret)
+			if (ret == -ENOENT)
+				return -ENOPKG;
+			else if (ret)
 				return ret;
 			done = true;
 		}
diff --git a/include/image.h b/include/image.h
index dd4042d1bd9..2ab17075287 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1858,7 +1858,7 @@ int android_image_get_kernel(const void *hdr,
  * @vendor_boot_img : Pointer to vendor boot image header
  * @rd_data:	Pointer to a ulong variable, will hold ramdisk address
  * @rd_len:	Pointer to a ulong variable, will hold ramdisk length
- * Return: 0 if succeeded, -1 if ramdisk size is 0
+ * Return: 0 if OK, -ENOPKG if no ramdisk, -EINVAL if invalid image
  */
 int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
 			      ulong *rd_data, ulong *rd_len);
-- 
2.39.2

Re: [PATCH v2] boot: android: fix booting without a ramdisk

[Mattijs Korpershoek]

Hi Michael,

Thank you for the patch.

On lun., juil. 29, 2024 at 23:36, Michael Walle <mwalle@kernel.org> wrote:

> android_image_get_ramdisk() will return an error if there is no ramdisk.
> Using the android image without a ramdisk worked until commit
> 1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
> the return code wasn't checked until then. Return -ENOENT in case
> there is no ramdisk and translate that into -ENOPKG in the calling
> code, which will then indicate "no ramdisk" to its caller
> (boot_get_ramdisk()).
>
> This way, we can get rid of the "*rd_data = *rd_len = 0;" in the error
> path, too.
>
> With this, I'm able to boot a linux kernel using fastboot again:
>
>   fastboot --base 0x41000000 --header-version 2 --dtb /path/to/dtb \
>   --cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  boot/image-android.c | 7 +++----
>  boot/image-board.c   | 4 +++-
>  include/image.h      | 2 +-
>  3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 09c7a44e058..774565fd1fe 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -393,10 +393,9 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
>  	if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
>  		return -EINVAL;
>  
> -	if (!img_data.ramdisk_size) {
> -		*rd_data = *rd_len = 0;
> -		return -1;
> -	}
> +	if (!img_data.ramdisk_size)
> +		return -ENOENT;
> +
>  	if (img_data.header_version > 2) {
>  		ramdisk_ptr = img_data.ramdisk_addr;
>  		memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
> diff --git a/boot/image-board.c b/boot/image-board.c
> index f2124013046..eca1b1d2bff 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -427,7 +427,9 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a
>  				unmap_sysmem(ptr);
>  			}
>  
> -			if (ret)
> +			if (ret == -ENOENT)
> +				return -ENOPKG;
> +			else if (ret)
>  				return ret;
>  			done = true;
>  		}
> diff --git a/include/image.h b/include/image.h
> index dd4042d1bd9..2ab17075287 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1858,7 +1858,7 @@ int android_image_get_kernel(const void *hdr,
>   * @vendor_boot_img : Pointer to vendor boot image header
>   * @rd_data:	Pointer to a ulong variable, will hold ramdisk address
>   * @rd_len:	Pointer to a ulong variable, will hold ramdisk length
> - * Return: 0 if succeeded, -1 if ramdisk size is 0
> + * Return: 0 if OK, -ENOPKG if no ramdisk, -EINVAL if invalid image
>   */
>  int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
>  			      ulong *rd_data, ulong *rd_len);
> -- 
> 2.39.2

Re: [PATCH v2] boot: android: fix booting without a ramdisk

[Simon Glass]

On Tue, 30 Jul 2024 at 02:51, Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> Hi Michael,
>
> Thank you for the patch.
>
> On lun., juil. 29, 2024 at 23:36, Michael Walle <mwalle@kernel.org> wrote:
>
> > android_image_get_ramdisk() will return an error if there is no ramdisk.
> > Using the android image without a ramdisk worked until commit
> > 1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
> > the return code wasn't checked until then. Return -ENOENT in case
> > there is no ramdisk and translate that into -ENOPKG in the calling
> > code, which will then indicate "no ramdisk" to its caller
> > (boot_get_ramdisk()).
> >
> > This way, we can get rid of the "*rd_data = *rd_len = 0;" in the error
> > path, too.
> >
> > With this, I'm able to boot a linux kernel using fastboot again:
> >
> >   fastboot --base 0x41000000 --header-version 2 --dtb /path/to/dtb \
> >   --cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image
> >
> > Signed-off-by: Michael Walle <mwalle@kernel.org>
>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

But see below

>
> > ---
> >  boot/image-android.c | 7 +++----
> >  boot/image-board.c   | 4 +++-
> >  include/image.h      | 2 +-
> >  3 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/boot/image-android.c b/boot/image-android.c
> > index 09c7a44e058..774565fd1fe 100644
> > --- a/boot/image-android.c
> > +++ b/boot/image-android.c
> > @@ -393,10 +393,9 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
> >       if (!android_image_get_data(hdr, vendor_boot_img, &img_data))
> >               return -EINVAL;
> >
> > -     if (!img_data.ramdisk_size) {
> > -             *rd_data = *rd_len = 0;
> > -             return -1;
> > -     }
> > +     if (!img_data.ramdisk_size)
> > +             return -ENOENT;
> > +
> >       if (img_data.header_version > 2) {
> >               ramdisk_ptr = img_data.ramdisk_addr;
> >               memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
> > diff --git a/boot/image-board.c b/boot/image-board.c
> > index f2124013046..eca1b1d2bff 100644
> > --- a/boot/image-board.c
> > +++ b/boot/image-board.c
> > @@ -427,7 +427,9 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a
> >                               unmap_sysmem(ptr);
> >                       }
> >
> > -                     if (ret)
> > +                     if (ret == -ENOENT)
> > +                             return -ENOPKG;

We normally use -ENOENT for this sort of thing.

> > +                     else if (ret)
> >                               return ret;
> >                       done = true;
> >               }
> > diff --git a/include/image.h b/include/image.h
> > index dd4042d1bd9..2ab17075287 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1858,7 +1858,7 @@ int android_image_get_kernel(const void *hdr,
> >   * @vendor_boot_img : Pointer to vendor boot image header
> >   * @rd_data: Pointer to a ulong variable, will hold ramdisk address
> >   * @rd_len:  Pointer to a ulong variable, will hold ramdisk length
> > - * Return: 0 if succeeded, -1 if ramdisk size is 0
> > + * Return: 0 if OK, -ENOPKG if no ramdisk, -EINVAL if invalid image
> >   */
> >  int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
> >                             ulong *rd_data, ulong *rd_len);
> > --
> > 2.39.2

Regards,
Simon

Re: [PATCH v2] boot: android: fix booting without a ramdisk

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

Hi,

> > > -                     if (ret)
> > > +                     if (ret == -ENOENT)
> > > +                             return -ENOPKG;
>
> We normally use -ENOENT for this sort of thing.

That's the way select_ramdisk() is documented. It was actually
introduced by yourself in commit e4c928792e93 ("image: Split up
boot_get_ramdisk()") :)

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH v2] boot: android: fix booting without a ramdisk

[Simon Glass]

Hi Michael,

On Wed, 31 Jul 2024 at 09:33, Michael Walle <mwalle@kernel.org> wrote:
>
> Hi,
>
> > > > -                     if (ret)
> > > > +                     if (ret == -ENOENT)
> > > > +                             return -ENOPKG;
> >
> > We normally use -ENOENT for this sort of thing.
>
> That's the way select_ramdisk() is documented. It was actually
> introduced by yourself in commit e4c928792e93 ("image: Split up
> boot_get_ramdisk()") :)

Hmm, OK. Perhaps I was worried about conflicting with a file-not-found
error in bootstd. Anyway, let's stick with ENOPKG.

Regards,
Simon

Re: [PATCH v2] boot: android: fix booting without a ramdisk

[Mattijs Korpershoek]

Hi,

On Mon, 29 Jul 2024 23:36:57 +0200, Michael Walle wrote:
> android_image_get_ramdisk() will return an error if there is no ramdisk.
> Using the android image without a ramdisk worked until commit
> 1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
> the return code wasn't checked until then. Return -ENOENT in case
> there is no ramdisk and translate that into -ENOPKG in the calling
> code, which will then indicate "no ramdisk" to its caller
> (boot_get_ramdisk()).
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu)

[1/1] boot: android: fix booting without a ramdisk
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/2a75793bc55ec1ace0a66e96102beec3cb7ab64d

--
Mattijs