[PATCH 1/1] cmd: fix loads, saves on sandbox

[Simon Glass]

Hi Heinrich,

On Sun, 25 Jun 2023 at 10:54, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> The loads and saves commands crash on the sandbox due to illegal memory
> access.
>
> For command line arguments the sandbox uses a virtual address space which
> does not equal the addresses of the memory allocated with memmap(). Add the
> missing address translations for the loads and saves commands.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  cmd/load.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

>
> diff --git a/cmd/load.c b/cmd/load.c
> index 5c4f34781d..2715cf5957 100644
> --- a/cmd/load.c
> +++ b/cmd/load.c
> @@ -181,13 +181,17 @@ static ulong load_serial(long offset)
>                     } else
>  #endif
>                     {
> +                       void *dst;
> +
>                         ret = lmb_reserve(&lmb, store_addr, binlen);
>                         if (ret) {
>                                 printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
>                                         store_addr, store_addr + binlen);
>                                 return ret;
>                         }
> -                       memcpy((char *)(store_addr), binbuf, binlen);
> +                       dst = map_sysmem(store_addr, binlen);
> +                       memcpy(dst, binbuf, binlen);
> +                       unmap_sysmem(dst);
>                         lmb_free(&lmb, store_addr, binlen);
>                     }
>                     if ((store_addr) < start_addr)
> @@ -350,15 +354,19 @@ static int save_serial(ulong address, ulong count)
>         if(write_record(SREC3_START))                   /* write the header */
>                 return (-1);
>         do {
> -               if(count) {                                             /* collect hex data in the buffer  */
> -                       c = *(volatile uchar*)(address + reclen);       /* get one byte    */
> -                       checksum += c;                                                  /* accumulate checksum */
> +               volatile uchar *src;
> +
> +               src = map_sysmem(address, count);
> +               if (count) {                            /* collect hex data in the buffer */
> +                       c = src[reclen];                /* get one byte */
> +                       checksum += c;                  /* accumulate checksum */
>                         data[2*reclen]   = hex[(c>>4)&0x0f];
>                         data[2*reclen+1] = hex[c & 0x0f];
>                         data[2*reclen+2] = '\0';
>                         ++reclen;
>                         --count;
>                 }
> +               unmap_sysmem((void *)src);

nit: You should not need the cast.

>                 if(reclen == SREC_BYTES_PER_RECORD || count == 0) {
>                         /* enough data collected for one record: dump it */
>                         if(reclen) {    /* build & write a data record: */
> --
> 2.40.1
>

Regards,
Simon