Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup

Miquel Raynal
2021-01-25T08:54:46Z

Hello,

yangerkun <yangerkun@huawei.com> wrote on Mon, 25 Jan 2021 16:36:50
+0800:

> 在 2021/1/25 16:28, Miquel Raynal 写道:
> > Hi yangerkun,
> >
> > yangerkun <yangerkun@huawei.com> wrote on Mon, 25 Jan 2021 13:48:32
> > +0800:
> >
> >> Hi,
> >>
> >> Recheck the patch, and it seems we need check erasesize to prevent DIV/0...
> >
> > Can you elaborate a little bit?
> >
> > The do_div() != 0 has been replaced by the 'rem' condition and it seems
> > good to me. So please tell me what division by zero you are talking
> > about because the beginning of the if condition is "len == 0 ||
> > erasesize == 0 || erasesize > len", so it does already take care of
> > this situation.
>
> The patch I send show as follow:
>
> --- a/drivers/mtd/devices/phram.c
> +++ b/drivers/mtd/devices/phram.c
> @@ -222,6 +222,7 @@ static int phram_setup(const char *val)
> uint64_t start;
> uint64_t len;
> uint64_t erasesize = PAGE_SIZE;
> + uint32_t rem;
> int i, ret;
>
> if (strnlen(val, sizeof(buf)) >= sizeof(buf))
> @@ -263,8 +264,9 @@ static int phram_setup(const char *val)
> }
> }
>
> + div_u64_rem(len, (uint32_t)erasesize, &rem);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Oh right this has been moved before the checks, please send a v2 then.

> Once user set erasesize == 0, the div_u64_rem will trigger the
> DIV/0...
>
> if (len == 0 || erasesize == 0 || erasesize > len
> - || erasesize > UINT_MAX || do_div(len, (uint32_t)erasesize) != 0) {
> + || erasesize > UINT_MAX || rem) {
> parse_err("illegal erasesize or len\n");
> goto error;
> }

Thanks,
Miquèl