Re: [PATCH v2 12/17] android: boot: support extra command line

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

On Fri, Jan 27, 2023 at 10:15, Simon Glass <sjg@chromium.org> wrote:

> On Fri, 27 Jan 2023 at 08:51, Safae Ouajih <souajih@baylibre.com> wrote:
>>
>>
>> On 27/01/2023 01:54, Simon Glass wrote:
>> > Hi Safae,
>> >
>> > On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <souajih@baylibre.com> wrote:
>> >> In version 3 and 4 of boot image header, the vendor specific
>> > vendor-spefcific
>> >
>> >> command line are located in vendor boot image. Thus, using
>> > use the
>> >
>> >> extra command line to add those cmd to bootargs.
>> >>
>> >> Signed-off-by: Safae Ouajih <souajih@baylibre.com>

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

>> >> ---
>> >>   boot/image-android.c | 11 +++++++++++
>> >>   1 file changed, 11 insertions(+)
>> >>
>> >> diff --git a/boot/image-android.c b/boot/image-android.c
>> >> index 5b270e4417..cb4fc22b00 100644
>> >> --- a/boot/image-android.c
>> >> +++ b/boot/image-android.c
>> >> @@ -55,6 +55,7 @@ static void android_vendor_boot_image_v3_v4_parse_hdr(const struct andr_vendor_i
>> >>           * The header takes a full page, the remaining components are aligned
>> >>           * on page boundary.
>> >>           */
>> >> +       data->kcmdline_extra = hdr->cmdline;
>> >>          data->tags_addr = hdr->tags_addr;
>> >>          data->image_name = hdr->name;
>> >>          data->kernel_addr = hdr->kernel_addr;
>> >> @@ -233,6 +234,11 @@ int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr,
>> >>                  len += strlen(img_data.kcmdline);
>> >>          }
>> >>
>> >> +       if (img_data.kcmdline_extra) {
>> >> +               printf("Kernel extra command line: %s\n", img_data.kcmdline_extra);
>> >> +               len += strlen(img_data.kcmdline_extra);
>> >> +       }
>> >> +
>> >>          char *bootargs = env_get("bootargs");
>> >>          if (bootargs)
>> >>                  len += strlen(bootargs);
>> >> @@ -252,6 +258,11 @@ int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr,
>> >>          if (*img_data.kcmdline)
>> >>                  strcat(newbootargs, img_data.kcmdline);
>> >>
>> >> +       if (img_data.kcmdline_extra) {
>> >> +               strcat(newbootargs, " ");
>> >> +               strcat(newbootargs, img_data.kcmdline_extra);
>> > Do we need to worry about overflow?
>>
>> Hi Simon,
>>
>> This line adds kcmdline_extra length to the len variable:
>>
>>         len += strlen(img_data.kcmdline_extra);
>>
>> newbootargs is allocated just after in :
>>
>>          char *newbootargs = malloc(len + 2);
>>
>> I can not see a possible overflow, please let me know if you
>>
>> think this is not enough.
>
> OK I see, thank you.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>