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

[Heinrich Schuchardt]

On 7/31/23 09:00, Michael Nazzareno Trimarchi wrote:

Hi

On Sun, Jul 30, 2023 at 3:03 PM Heinrich Schuchardt
<heinrich.schucha...@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?

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.schucha...@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