[PATCH 1/1] cmd: avoid overflow in mtd_is_aligned_with_min_io_size

[Michael Nazzareno Trimarchi]

Hi

On Mon, Jul 31, 2023 at 10:10 AM Heinrich Schuchardt <
heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 7/31/23 09:00, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Sun, Jul 30, 2023 at 3:03 PM Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> Multiplication of u32 factors has an u32 result even if an overflow
occurs.
> >> An overflow may occur in
> >>
> >>          u64 data_off = page * mtd->writesize;
> >>
> >> The size of the flash device may exceed 4 GiB.
> >>
> >
> > Instead of force variables to u64 just cast where expansion is needed
before
> > multiple. Why we should thinking to have u64 number of nand pages?
>
> Does anything stop MTD devices from reaching 2^32 pages?
>

Pages is integer right now in the code. What is the problem just to fix
what you are claim

Michael

> The driver layer uses loff_t for 'from'. I can't see any limitation there.
>
> Best regards
>
> Heinrich
>
> >
> > Michael
> >
> >> Play it safe and use u64 consistently.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> ---
> >>   cmd/mtd.c | 17 +++++++----------
> >>   1 file changed, 7 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/cmd/mtd.c b/cmd/mtd.c
> >> index eb6e2d6892..fb6e2bb047 100644
> >> --- a/cmd/mtd.c
> >> +++ b/cmd/mtd.c
> >> @@ -33,11 +33,9 @@ static struct mtd_info *get_mtd_by_name(const char
*name)
> >>          return mtd;
> >>   }
> >>
> >> -static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
> >> +static u64 mtd_len_to_pages(struct mtd_info *mtd, u64 len)
> >>   {
> >> -       do_div(len, mtd->writesize);
> >> -
> >> -       return len;
> >> +       return lldiv(len, mtd->writesize);
> >>   }
> >>
> >>   static bool mtd_is_aligned_with_min_io_size(struct mtd_info *mtd,
u64 size)
> >> @@ -72,8 +70,7 @@ static void mtd_dump_device_buf(struct mtd_info
*mtd, u64 start_off,
> >>   {
> >>          bool has_pages = mtd->type == MTD_NANDFLASH ||
> >>                  mtd->type == MTD_MLCNANDFLASH;
> >> -       int npages = mtd_len_to_pages(mtd, len);
> >> -       uint page;
> >> +       u64 page, npages = mtd_len_to_pages(mtd, len);
> >>
> >>          if (has_pages) {
> >>                  for (page = 0; page < npages; page++) {
> >> @@ -251,12 +248,12 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int
flag, int argc,
> >>                       char *const argv[])
> >>   {
> >>          bool dump, read, raw, woob, write_empty_pages, has_pages =
false;
> >> -       u64 start_off, off, len, remaining, default_len;
> >> +       u64 start_off, off, len, oob_len, remaining, default_len;
> >>          struct mtd_oob_ops io_op = {};
> >> -       uint user_addr = 0, npages;
> >> +       uint user_addr = 0;
> >> +       u64 npages;
> >>          const char *cmd = argv[0];
> >>          struct mtd_info *mtd;
> >> -       u32 oob_len;
> >>          u8 *buf;
> >>          int ret;
> >>
> >> @@ -322,7 +319,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int
flag, int argc,
> >>          }
> >>
> >>          if (has_pages)
> >> -               printf("%s %lld byte(s) (%d page(s)) at offset
0x%08llx%s%s%s\n",
> >> +               printf("%s %lld byte(s) (%lld page(s)) at offset
0x%08llx%s%s%s\n",
> >>                         read ? "Reading" : "Writing", len, npages,
start_off,
> >>                         raw ? " [raw]" : "", woob ? " [oob]" : "",
> >>                         !read && write_empty_pages ? " [dontskipff]" :
"");
> >> --
> >> 2.40.1
> >>