[PATCH] boot: android: Check kcmdline's for NULL in android_image_get_kernel()

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,

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

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

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

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/4e599aa73a386dd...
-- Mattijs
participants (4)
-
Julien Masson
-
Mattijs Korpershoek
-
Nicolas Belin
-
Sam Day