All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
@ 2025-01-13  9:11 Mattijs Korpershoek
  2025-01-13 13:22 ` Nicolas Belin
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Mattijs Korpershoek @ 2025-01-13  9:11 UTC (permalink / raw)
  To: Tom Rini, Nicolas Belin
  Cc: Guillaume La Roque, Aaron Kling, Julien Masson, u-boot,
	Mattijs Korpershoek

From: Aaron Kling <webgeek1234@gmail.com>

kcmdline and kcmdline_extra strings can be NULL. In that case, we still
read the content from 0x00000 and pass that to the kernel, which is
completely wrong.

Fix android_image_get_kernel() to check for NULL before checking if
they are empty strings.

Fixes: 53a0ddb6d3be ("boot: android: fix extra command line support")
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
Thanks to Aaron for reporting this on the aosp-devs discord and for
fixing this.
---
 boot/image-android.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index 60a422dfb74a6c683b3cf9d2b19b3ad1dbd0d151..fa4e14ca4698e1dea105388dd2ea590024cafa58 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -337,12 +337,12 @@ int android_image_get_kernel(const void *hdr,
 	if (bootargs)
 		len += strlen(bootargs);
 
-	if (*img_data.kcmdline) {
+	if (img_data.kcmdline && *img_data.kcmdline) {
 		printf("Kernel command line: %s\n", img_data.kcmdline);
 		len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */
 	}
 
-	if (*img_data.kcmdline_extra) {
+	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
 		printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
 		len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
 	}
@@ -357,13 +357,13 @@ int android_image_get_kernel(const void *hdr,
 	if (bootargs)
 		strcpy(newbootargs, bootargs);
 
-	if (*img_data.kcmdline) {
+	if (img_data.kcmdline && *img_data.kcmdline) {
 		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
 			strcat(newbootargs, " ");
 		strcat(newbootargs, img_data.kcmdline);
 	}
 
-	if (*img_data.kcmdline_extra) {
+	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
 		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
 			strcat(newbootargs, " ");
 		strcat(newbootargs, img_data.kcmdline_extra);

---
base-commit: bc157bb6667ed97e33be8ce8436c28baa275b295
change-id: 20250113-kcmdline-extra-fix-509331e4d7f3

Best regards,
-- 
Mattijs Korpershoek <mkorpershoek@baylibre.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
  2025-01-13  9:11 [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel() Mattijs Korpershoek
@ 2025-01-13 13:22 ` Nicolas Belin
  2025-01-23 13:29 ` Julien Masson
  2025-01-23 14:20 ` Mattijs Korpershoek
  2 siblings, 0 replies; 5+ messages in thread
From: Nicolas Belin @ 2025-01-13 13:22 UTC (permalink / raw)
  To: Mattijs Korpershoek
  Cc: Tom Rini, Guillaume La Roque, Aaron Kling, Julien Masson, u-boot

Le lun. 13 janv. 2025 à 10:11, Mattijs Korpershoek
<mkorpershoek@baylibre.com> a écrit :
>
> From: Aaron Kling <webgeek1234@gmail.com>
>
> kcmdline and kcmdline_extra strings can be NULL. In that case, we still
> read the content from 0x00000 and pass that to the kernel, which is
> completely wrong.
>
> Fix android_image_get_kernel() to check for NULL before checking if
> they are empty strings.
>
> Fixes: 53a0ddb6d3be ("boot: android: fix extra command line support")
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Thanks to Aaron for reporting this on the aosp-devs discord and for
> fixing this.
> ---
>  boot/image-android.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 60a422dfb74a6c683b3cf9d2b19b3ad1dbd0d151..fa4e14ca4698e1dea105388dd2ea590024cafa58 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -337,12 +337,12 @@ int android_image_get_kernel(const void *hdr,
>         if (bootargs)
>                 len += strlen(bootargs);
>
> -       if (*img_data.kcmdline) {
> +       if (img_data.kcmdline && *img_data.kcmdline) {
>                 printf("Kernel command line: %s\n", img_data.kcmdline);
>                 len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */
>         }
>
> -       if (*img_data.kcmdline_extra) {
> +       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>                 printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
>                 len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
>         }
> @@ -357,13 +357,13 @@ int android_image_get_kernel(const void *hdr,
>         if (bootargs)
>                 strcpy(newbootargs, bootargs);
>
> -       if (*img_data.kcmdline) {
> +       if (img_data.kcmdline && *img_data.kcmdline) {
>                 if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>                         strcat(newbootargs, " ");
>                 strcat(newbootargs, img_data.kcmdline);
>         }
>
> -       if (*img_data.kcmdline_extra) {
> +       if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>                 if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>                         strcat(newbootargs, " ");
>                 strcat(newbootargs, img_data.kcmdline_extra);
>
> ---
> base-commit: bc157bb6667ed97e33be8ce8436c28baa275b295
> change-id: 20250113-kcmdline-extra-fix-509331e4d7f3
>
> Best regards,
> --
> Mattijs Korpershoek <mkorpershoek@baylibre.com>
>

Reviewed-by: Nicolas Belin <nbelin@baylibre.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
  2025-01-13  9:11 [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel() Mattijs Korpershoek
  2025-01-13 13:22 ` Nicolas Belin
@ 2025-01-23 13:29 ` Julien Masson
  2025-01-23 13:52   ` Sam Day
  2025-01-23 14:20 ` Mattijs Korpershoek
  2 siblings, 1 reply; 5+ messages in thread
From: Julien Masson @ 2025-01-23 13:29 UTC (permalink / raw)
  To: Mattijs Korpershoek, Tom Rini, Nicolas Belin
  Cc: Guillaume La Roque, Aaron Kling, Julien Masson, u-boot,
	Mattijs Korpershoek


On Thu 23 Jan 2025 at 14:28, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:

> From: Aaron Kling <webgeek1234@gmail.com>
> 
> kcmdline and kcmdline_extra strings can be NULL. In that case, we still
> read the content from 0x00000 and pass that to the kernel, which is
> completely wrong.
> 
> Fix android_image_get_kernel() to check for NULL before checking if
> they are empty strings.
> 
> Fixes: 53a0ddb6d3be ("boot: android: fix extra command line support")
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> Thanks to Aaron for reporting this on the aosp-devs discord and for
> fixing this.
> ---
>  boot/image-android.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 60a422dfb74a6c683b3cf9d2b19b3ad1dbd0d151..fa4e14ca4698e1dea105388dd2ea590024cafa58 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -337,12 +337,12 @@ int android_image_get_kernel(const void *hdr,
>  	if (bootargs)
>  		len += strlen(bootargs);
>  
> -	if (*img_data.kcmdline) {
> +	if (img_data.kcmdline && *img_data.kcmdline) {
>  		printf("Kernel command line: %s\n", img_data.kcmdline);
>  		len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */
>  	}
>  
> -	if (*img_data.kcmdline_extra) {
> +	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>  		printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
>  		len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */
>  	}
> @@ -357,13 +357,13 @@ int android_image_get_kernel(const void *hdr,
>  	if (bootargs)
>  		strcpy(newbootargs, bootargs);
>  
> -	if (*img_data.kcmdline) {
> +	if (img_data.kcmdline && *img_data.kcmdline) {
>  		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>  			strcat(newbootargs, " ");
>  		strcat(newbootargs, img_data.kcmdline);
>  	}
>  
> -	if (*img_data.kcmdline_extra) {
> +	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
>  		if (*newbootargs) /* If there is something in newbootargs, a space is needed */
>  			strcat(newbootargs, " ");
>  		strcat(newbootargs, img_data.kcmdline_extra);
> 
> ---
> base-commit: bc157bb6667ed97e33be8ce8436c28baa275b295
> change-id: 20250113-kcmdline-extra-fix-509331e4d7f3
> 
> Best regards,
> -- 
> Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 

Reviewed-by: Julien Masson <jmasson@baylibre.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
  2025-01-23 13:29 ` Julien Masson
@ 2025-01-23 13:52   ` Sam Day
  0 siblings, 0 replies; 5+ messages in thread
From: Sam Day @ 2025-01-23 13:52 UTC (permalink / raw)
  To: Julien Masson
  Cc: Mattijs Korpershoek, Tom Rini, Nicolas Belin, Guillaume La Roque,
	Aaron Kling, u-boot

On Thursday, 23 January 2025 at 14:29, Julien Masson <jmasson@baylibre.com> wrote:

> 
> On Thu 23 Jan 2025 at 14:28, Mattijs Korpershoek mkorpershoek@baylibre.com wrote:
> 
> > From: Aaron Kling webgeek1234@gmail.com
> > 
> > kcmdline and kcmdline_extra strings can be NULL. In that case, we still
> > read the content from 0x00000 and pass that to the kernel, which is
> > completely wrong.
> > 
> > Fix android_image_get_kernel() to check for NULL before checking if
> > they are empty strings.
> > 
> > Fixes: 53a0ddb6d3be ("boot: android: fix extra command line support")
> > Signed-off-by: Aaron Kling webgeek1234@gmail.com
> > Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com
> > ---
> > Thanks to Aaron for reporting this on the aosp-devs discord and for
> > fixing this.
> > ---
> > boot/image-android.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/boot/image-android.c b/boot/image-android.c
> > index 60a422dfb74a6c683b3cf9d2b19b3ad1dbd0d151..fa4e14ca4698e1dea105388dd2ea590024cafa58 100644
> > --- a/boot/image-android.c
> > +++ b/boot/image-android.c
> > @@ -337,12 +337,12 @@ int android_image_get_kernel(const void *hdr,
> > if (bootargs)
> > len += strlen(bootargs);
> > 
> > - if (*img_data.kcmdline) {
> > + if (img_data.kcmdline && img_data.kcmdline) {
> > printf("Kernel command line: %s\n", img_data.kcmdline);
> > len += strlen(img_data.kcmdline) + (len ? 1 : 0); / +1 for extra space */
> > }
> > 
> > - if (*img_data.kcmdline_extra) {
> > + if (img_data.kcmdline_extra && img_data.kcmdline_extra) {
> > printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
> > len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); / +1 for extra space */
> > }
> > @@ -357,13 +357,13 @@ int android_image_get_kernel(const void *hdr,
> > if (bootargs)
> > strcpy(newbootargs, bootargs);
> > 
> > - if (*img_data.kcmdline) {
> > + if (img_data.kcmdline && *img_data.kcmdline) {
> > if (newbootargs) / If there is something in newbootargs, a space is needed */
> > strcat(newbootargs, " ");
> > strcat(newbootargs, img_data.kcmdline);
> > }
> > 
> > - if (*img_data.kcmdline_extra) {
> > + if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> > if (newbootargs) / If there is something in newbootargs, a space is needed */
> > strcat(newbootargs, " ");
> > strcat(newbootargs, img_data.kcmdline_extra);
> > 
> > ---
> > base-commit: bc157bb6667ed97e33be8ce8436c28baa275b295
> > change-id: 20250113-kcmdline-extra-fix-509331e4d7f3
> > 
> > Best regards,
> > --
> > Mattijs Korpershoek mkorpershoek@baylibre.com
> 
> 
> Reviewed-by: Julien Masson jmasson@baylibre.com

Tested-by: Sam Day <me@samcday.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
  2025-01-13  9:11 [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel() Mattijs Korpershoek
  2025-01-13 13:22 ` Nicolas Belin
  2025-01-23 13:29 ` Julien Masson
@ 2025-01-23 14:20 ` Mattijs Korpershoek
  2 siblings, 0 replies; 5+ messages in thread
From: Mattijs Korpershoek @ 2025-01-23 14:20 UTC (permalink / raw)
  To: Tom Rini, Nicolas Belin, Mattijs Korpershoek
  Cc: Guillaume La Roque, Aaron Kling, Julien Masson, u-boot

Hi,

On Mon, 13 Jan 2025 10:11:45 +0100, Mattijs Korpershoek wrote:
> kcmdline and kcmdline_extra strings can be NULL. In that case, we still
> read the content from 0x00000 and pass that to the kernel, which is
> completely wrong.
> 
> Fix android_image_get_kernel() to check for NULL before checking if
> they are empty strings.
> 
> [...]

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

[1/1] boot: android: Check kcmdline's for NULL in android_image_get_kernel()
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/4e599aa73a386dd1ba5091937ef2fc388d01ddd2

--
Mattijs

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-01-23 14:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-13  9:11 [PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel() Mattijs Korpershoek
2025-01-13 13:22 ` Nicolas Belin
2025-01-23 13:29 ` Julien Masson
2025-01-23 13:52   ` Sam Day
2025-01-23 14:20 ` Mattijs Korpershoek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.