public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug
@ 2023-10-30 20:24 Ashish Singhal via groups.io
  2023-10-30 20:24 ` [edk2-devel] [PATCH 2/2] EmbeddedPkg: Allow longer android kernel command line Ashish Singhal via groups.io
  2023-11-06 17:55 ` [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
  0 siblings, 2 replies; 5+ messages in thread
From: Ashish Singhal via groups.io @ 2023-10-30 20:24 UTC (permalink / raw)
  To: devel, quic_llindhol, ardb+tianocore, abner.chang, git, jbrasen
  Cc: Ashish Singhal

Curently, AndroidBootImgLib expects input kernel command line
to never exceed 256 unicode characters where the image header
allows for 512 ascii characters. If image header allows 512
ascii characters, similar number of unicode characters should be
allowed at the minimum.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
 .../AndroidBootImgLib/AndroidBootImgLib.c     | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 1359a66db2..02769cd0df 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -321,8 +321,9 @@ AndroidBootImgGetFdt (
 
 EFI_STATUS
 AndroidBootImgUpdateArgs (
-  IN  VOID  *BootImg,
-  OUT VOID  *KernelArgs
+  IN  VOID    *BootImg,
+  OUT VOID    *KernelArgs,
+  IN  UINT32  KernelArgsSize
   )
 {
   CHAR8       ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
@@ -337,13 +338,13 @@ AndroidBootImgUpdateArgs (
   AsciiStrToUnicodeStrS (
     ImageKernelArgs,
     KernelArgs,
-    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
+    KernelArgsSize
     );
   // Append platform kernel arguments
   if (mAndroidBootImg->AppendArgs) {
     Status = mAndroidBootImg->AppendArgs (
                                 KernelArgs,
-                                ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
+                                KernelArgsSize
                                 );
   }
 
@@ -611,11 +612,16 @@ AndroidBootImgBoot (
   MEMORY_DEVICE_PATH         KernelDevicePath;
   EFI_HANDLE                 ImageHandle;
   VOID                       *NewKernelArg;
+  UINT32                     NewKernelArgSize;
   EFI_LOADED_IMAGE_PROTOCOL  *ImageInfo;
   VOID                       *RamdiskData;
   UINTN                      RamdiskSize;
   IN  VOID                   *FdtBase;
 
+  if ((Buffer == NULL) || (BufferSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   NewKernelArg = NULL;
   ImageHandle  = NULL;
 
@@ -637,14 +643,15 @@ AndroidBootImgBoot (
     goto Exit;
   }
 
-  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArg     = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
   if (NewKernelArg == NULL) {
     DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
     Status = EFI_OUT_OF_RESOURCES;
     goto Exit;
   }
 
-  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
+  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg, NewKernelArgSize);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110337): https://edk2.groups.io/g/devel/message/110337
Mute This Topic: https://groups.io/mt/102284239/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* [edk2-devel] [PATCH 2/2] EmbeddedPkg: Allow longer android kernel command line
  2023-10-30 20:24 [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
@ 2023-10-30 20:24 ` Ashish Singhal via groups.io
  2023-11-06 17:55 ` [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
  1 sibling, 0 replies; 5+ messages in thread
From: Ashish Singhal via groups.io @ 2023-10-30 20:24 UTC (permalink / raw)
  To: devel, quic_llindhol, ardb+tianocore, abner.chang, git, jbrasen
  Cc: Ashish Singhal

AndroidBootImgLib allows for platforms to append to kernel command
line but does not allow for the overall kernel command line to go
beyond the limit set by the image header. Address this limitation
by adding a pcd where platform can tell how many extra characters
they expect on their platform in addition to what the image header
specifies.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
 EmbeddedPkg/EmbeddedPkg.dec                                 | 5 +++++
 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c   | 2 +-
 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 341ef5e6a6..94dc3c9b76 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -183,3 +183,8 @@
   # Selection between DT and ACPI as a default
   #
   gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x0000059
+
+  #
+  # Expected Overflow Android Kernel Command Line Characters
+  #
+  gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UINT32|0x000005C
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 02769cd0df..abd3f87d14 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -643,7 +643,7 @@ AndroidBootImgBoot (
     goto Exit;
   }
 
-  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32 (PcdAndroidKernelCommandLineOverflow);
   NewKernelArg     = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
   if (NewKernelArg == NULL) {
     DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
index 8eefeef4f9..9754664df5 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
@@ -45,5 +45,6 @@
   gEfiAcpiTableGuid
   gFdtTableGuid
 
-[FeaturePcd]
+[Pcd]
   gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
+  gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110338): https://edk2.groups.io/g/devel/message/110338
Mute This Topic: https://groups.io/mt/102284240/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug
  2023-10-30 20:24 [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
  2023-10-30 20:24 ` [edk2-devel] [PATCH 2/2] EmbeddedPkg: Allow longer android kernel command line Ashish Singhal via groups.io
@ 2023-11-06 17:55 ` Ashish Singhal via groups.io
  2023-11-10  2:06   ` Chang, Abner via groups.io
  1 sibling, 1 reply; 5+ messages in thread
From: Ashish Singhal via groups.io @ 2023-11-06 17:55 UTC (permalink / raw)
  To: devel@edk2.groups.io, quic_llindhol@quicinc.com,
	ardb+tianocore@kernel.org, abner.chang@amd.com,
	git@danielschaefer.me, Jeff Brasen

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

Hello,

Hoping to get some feedback on these 2 patches this week.

Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Monday, October 30, 2023 2:24 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; quic_llindhol@quicinc.com <quic_llindhol@quicinc.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; abner.chang@amd.com <abner.chang@amd.com>; git@danielschaefer.me <git@danielschaefer.me>; Jeff Brasen <jbrasen@nvidia.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com>
Subject: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

Curently, AndroidBootImgLib expects input kernel command line
to never exceed 256 unicode characters where the image header
allows for 512 ascii characters. If image header allows 512
ascii characters, similar number of unicode characters should be
allowed at the minimum.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
 .../AndroidBootImgLib/AndroidBootImgLib.c     | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 1359a66db2..02769cd0df 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -321,8 +321,9 @@ AndroidBootImgGetFdt (

 EFI_STATUS
 AndroidBootImgUpdateArgs (
-  IN  VOID  *BootImg,
-  OUT VOID  *KernelArgs
+  IN  VOID    *BootImg,
+  OUT VOID    *KernelArgs,
+  IN  UINT32  KernelArgsSize
   )
 {
   CHAR8       ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
@@ -337,13 +338,13 @@ AndroidBootImgUpdateArgs (
   AsciiStrToUnicodeStrS (
     ImageKernelArgs,
     KernelArgs,
-    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
+    KernelArgsSize
     );
   // Append platform kernel arguments
   if (mAndroidBootImg->AppendArgs) {
     Status = mAndroidBootImg->AppendArgs (
                                 KernelArgs,
-                                ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
+                                KernelArgsSize
                                 );
   }

@@ -611,11 +612,16 @@ AndroidBootImgBoot (
   MEMORY_DEVICE_PATH         KernelDevicePath;
   EFI_HANDLE                 ImageHandle;
   VOID                       *NewKernelArg;
+  UINT32                     NewKernelArgSize;
   EFI_LOADED_IMAGE_PROTOCOL  *ImageInfo;
   VOID                       *RamdiskData;
   UINTN                      RamdiskSize;
   IN  VOID                   *FdtBase;

+  if ((Buffer == NULL) || (BufferSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   NewKernelArg = NULL;
   ImageHandle  = NULL;

@@ -637,14 +643,15 @@ AndroidBootImgBoot (
     goto Exit;
   }

-  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArg     = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
   if (NewKernelArg == NULL) {
     DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
     Status = EFI_OUT_OF_RESOURCES;
     goto Exit;
   }

-  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
+  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg, NewKernelArgSize);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
--
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110752): https://edk2.groups.io/g/devel/message/110752
Mute This Topic: https://groups.io/mt/102284239/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 8196 bytes --]

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

* Re: [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug
  2023-11-06 17:55 ` [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
@ 2023-11-10  2:06   ` Chang, Abner via groups.io
  2023-11-15  3:13     ` Ashish Singhal via groups.io
  0 siblings, 1 reply; 5+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-10  2:06 UTC (permalink / raw)
  To: Ashish Singhal, devel@edk2.groups.io, quic_llindhol@quicinc.com,
	ardb+tianocore@kernel.org, git@danielschaefer.me, Jeff Brasen

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

[AMD Official Use Only - General]



From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Tuesday, November 7, 2023 1:55 AM
To: devel@edk2.groups.io; quic_llindhol@quicinc.com; ardb+tianocore@kernel.org; Chang, Abner <Abner.Chang@amd.com>; git@danielschaefer.me; Jeff Brasen <jbrasen@nvidia.com>
Subject: Re: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Hello,

Hoping to get some feedback on these 2 patches this week.

Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Sent: Monday, October 30, 2023 2:24 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; quic_llindhol@quicinc.com<mailto:quic_llindhol@quicinc.com> <quic_llindhol@quicinc.com<mailto:quic_llindhol@quicinc.com>>; ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org> <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; abner.chang@amd.com<mailto:abner.chang@amd.com> <abner.chang@amd.com<mailto:abner.chang@amd.com>>; git@danielschaefer.me<mailto:git@danielschaefer.me> <git@danielschaefer.me<mailto:git@danielschaefer.me>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Subject: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

Curently, AndroidBootImgLib expects input kernel command line
to never exceed 256 unicode characters where the image header
allows for 512 ascii characters. If image header allows 512
ascii characters, similar number of unicode characters should be
allowed at the minimum.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
---
 .../AndroidBootImgLib/AndroidBootImgLib.c     | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 1359a66db2..02769cd0df 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -321,8 +321,9 @@ AndroidBootImgGetFdt (

 EFI_STATUS
 AndroidBootImgUpdateArgs (
-  IN  VOID  *BootImg,
-  OUT VOID  *KernelArgs
+  IN  VOID    *BootImg,
+  OUT VOID    *KernelArgs,
+  IN  UINT32  KernelArgsSize
   )
 {
   CHAR8       ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
@@ -337,13 +338,13 @@ AndroidBootImgUpdateArgs (
   AsciiStrToUnicodeStrS (
     ImageKernelArgs,
     KernelArgs,
-    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
+    KernelArgsSize
     );
   // Append platform kernel arguments
   if (mAndroidBootImg->AppendArgs) {
     Status = mAndroidBootImg->AppendArgs (
                                 KernelArgs,
-                                ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
+                                KernelArgsSize
                                 );
   }

@@ -611,11 +612,16 @@ AndroidBootImgBoot (
   MEMORY_DEVICE_PATH         KernelDevicePath;
   EFI_HANDLE                 ImageHandle;
   VOID                       *NewKernelArg;
+  UINT32                     NewKernelArgSize;
   EFI_LOADED_IMAGE_PROTOCOL  *ImageInfo;
   VOID                       *RamdiskData;
   UINTN                      RamdiskSize;
   IN  VOID                   *FdtBase;

+  if ((Buffer == NULL) || (BufferSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   NewKernelArg = NULL;
   ImageHandle  = NULL;

@@ -637,14 +643,15 @@ AndroidBootImgBoot (
     goto Exit;
   }

-  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArg     = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
I think you can just move the memory allocation code to inside AndroidBootImgUpdateArgs then you don't need the additional arg for AndroidBootImgUpdateArgs.
Also Change AndroidBootImgUpdateArgs (Buffer, NewKernelArg); to AndroidBootImgUpdateArgs (Buffer, &NewKernelArg);
With this the code looks simple.
thanks
Abner

   if (NewKernelArg == NULL) {
     DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
     Status = EFI_OUT_OF_RESOURCES;
     goto Exit;
   }

-  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
+  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg, NewKernelArgSize);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
--
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111006): https://edk2.groups.io/g/devel/message/111006
Mute This Topic: https://groups.io/mt/102284239/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12809 bytes --]

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

* Re: [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug
  2023-11-10  2:06   ` Chang, Abner via groups.io
@ 2023-11-15  3:13     ` Ashish Singhal via groups.io
  0 siblings, 0 replies; 5+ messages in thread
From: Ashish Singhal via groups.io @ 2023-11-15  3:13 UTC (permalink / raw)
  To: Chang, Abner, devel@edk2.groups.io, quic_llindhol@quicinc.com,
	ardb+tianocore@kernel.org, git@danielschaefer.me, Jeff Brasen

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

Hello Abner,

Thanks for the feedback. I have pushed v2 of the patch set.

Thanks
Ashish
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Thursday, November 9, 2023 7:06 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; quic_llindhol@quicinc.com <quic_llindhol@quicinc.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; git@danielschaefer.me <git@danielschaefer.me>; Jeff Brasen <jbrasen@nvidia.com>
Subject: RE: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

External email: Use caution opening links or attachments


[AMD Official Use Only - General]






From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Tuesday, November 7, 2023 1:55 AM
To: devel@edk2.groups.io; quic_llindhol@quicinc.com; ardb+tianocore@kernel.org; Chang, Abner <Abner.Chang@amd.com>; git@danielschaefer.me; Jeff Brasen <jbrasen@nvidia.com>
Subject: Re: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug



Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.



Hello,



Hoping to get some feedback on these 2 patches this week.



Thanks

Ashish

________________________________

From: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Sent: Monday, October 30, 2023 2:24 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; quic_llindhol@quicinc.com<mailto:quic_llindhol@quicinc.com> <quic_llindhol@quicinc.com<mailto:quic_llindhol@quicinc.com>>; ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org> <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; abner.chang@amd.com<mailto:abner.chang@amd.com> <abner.chang@amd.com<mailto:abner.chang@amd.com>>; git@danielschaefer.me<mailto:git@danielschaefer.me> <git@danielschaefer.me<mailto:git@danielschaefer.me>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Subject: [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug



Curently, AndroidBootImgLib expects input kernel command line
to never exceed 256 unicode characters where the image header
allows for 512 ascii characters. If image header allows 512
ascii characters, similar number of unicode characters should be
allowed at the minimum.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
---
 .../AndroidBootImgLib/AndroidBootImgLib.c     | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 1359a66db2..02769cd0df 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -321,8 +321,9 @@ AndroidBootImgGetFdt (

 EFI_STATUS
 AndroidBootImgUpdateArgs (
-  IN  VOID  *BootImg,
-  OUT VOID  *KernelArgs
+  IN  VOID    *BootImg,
+  OUT VOID    *KernelArgs,
+  IN  UINT32  KernelArgsSize
   )
 {
   CHAR8       ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
@@ -337,13 +338,13 @@ AndroidBootImgUpdateArgs (
   AsciiStrToUnicodeStrS (
     ImageKernelArgs,
     KernelArgs,
-    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1

+    KernelArgsSize
     );
   // Append platform kernel arguments
   if (mAndroidBootImg->AppendArgs) {
     Status = mAndroidBootImg->AppendArgs (
                                 KernelArgs,
-                                ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
+                                KernelArgsSize
                                 );
   }

@@ -611,11 +612,16 @@ AndroidBootImgBoot (
   MEMORY_DEVICE_PATH         KernelDevicePath;
   EFI_HANDLE                 ImageHandle;
   VOID                       *NewKernelArg;
+  UINT32                     NewKernelArgSize;
   EFI_LOADED_IMAGE_PROTOCOL  *ImageInfo;
   VOID                       *RamdiskData;
   UINTN                      RamdiskSize;
   IN  VOID                   *FdtBase;

+  if ((Buffer == NULL) || (BufferSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   NewKernelArg = NULL;
   ImageHandle  = NULL;

@@ -637,14 +643,15 @@ AndroidBootImgBoot (
     goto Exit;
   }

-  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArg     = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);

I think you can just move the memory allocation code to inside AndroidBootImgUpdateArgs then you don’t need the additional arg for AndroidBootImgUpdateArgs.

Also Change AndroidBootImgUpdateArgs (Buffer, NewKernelArg); to AndroidBootImgUpdateArgs (Buffer, &NewKernelArg);

With this the code looks simple.

thanks

Abner

   if (NewKernelArg == NULL) {
     DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
     Status = EFI_OUT_OF_RESOURCES;
     goto Exit;
   }

-  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
+  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg, NewKernelArgSize);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
--
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111230): https://edk2.groups.io/g/devel/message/111230
Mute This Topic: https://groups.io/mt/102284239/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 13665 bytes --]

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

end of thread, other threads:[~2023-11-15  3:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-30 20:24 [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
2023-10-30 20:24 ` [edk2-devel] [PATCH 2/2] EmbeddedPkg: Allow longer android kernel command line Ashish Singhal via groups.io
2023-11-06 17:55 ` [edk2-devel] [PATCH 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug Ashish Singhal via groups.io
2023-11-10  2:06   ` Chang, Abner via groups.io
2023-11-15  3:13     ` Ashish Singhal via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox