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

Simon Glass
2023-01-27T17:18:13Z

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>
> >> ---
> >> 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>