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

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

[yangerkun]

We now support user to set erase page size, and use do_div between len
and erase size to determine the reasonableness for the erase size.
However, do_div is a macro and will overwrite the value of len. Which
results a mtd device with unexcepted size. Fix it by use div_u64_rem.

Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
Signed-off-by: yangerkun <yangerkun@huawei.com>
---
 drivers/mtd/devices/phram.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index cfd170946ba4..6e72b0abcb71 100644
--- 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);
 	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;
 	}
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Miquel Raynal]

Hi yangerkun,

yangerkun <yangerkun@huawei.com> wrote on Wed, 30 Dec 2020 10:17:18
+0800:

> We now support user to set erase page size, and use do_div between len
> and erase size to determine the reasonableness for the erase size.
> However, do_div is a macro and will overwrite the value of len. Which
> results a mtd device with unexcepted size. Fix it by use div_u64_rem.

                       with unexpected size. Use div_u64_rem instead.

(I will fix it when applying, no need to resend)

> 
> Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
> Signed-off-by: yangerkun <yangerkun@huawei.com>
> ---
>  drivers/mtd/devices/phram.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> index cfd170946ba4..6e72b0abcb71 100644
> --- 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);
>  	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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Miquel Raynal]

On Wed, 2020-12-30 at 02:17:18 UTC, yangerkun wrote:
> We now support user to set erase page size, and use do_div between len
> and erase size to determine the reasonableness for the erase size.
> However, do_div is a macro and will overwrite the value of len. Which
> results a mtd device with unexcepted size. Fix it by use div_u64_rem.
> 
> Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
> Signed-off-by: yangerkun <yangerkun@huawei.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[yangerkun]

Hi,

Recheck the patch, and it seems we need check erasesize to prevent DIV/0...

Sorry for the mistake. Can you add a fix patch for this?

Thanks,
Kun.

在 2021/1/4 18:18, Miquel Raynal 写道:
> On Wed, 2020-12-30 at 02:17:18 UTC, yangerkun wrote:
>> We now support user to set erase page size, and use do_div between len
>> and erase size to determine the reasonableness for the erase size.
>> However, do_div is a macro and will overwrite the value of len. Which
>> results a mtd device with unexcepted size. Fix it by use div_u64_rem.
>>
>> Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
>> Signed-off-by: yangerkun <yangerkun@huawei.com>
> 
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
> 
> Miquel
> .
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

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

> 
> Sorry for the mistake. Can you add a fix patch for this?

In any case if the patch turns out to be wrong I'll ask you to either
send a fix or send a v2 and I'll drop v1.

> 在 2021/1/4 18:18, Miquel Raynal 写道:
> > On Wed, 2020-12-30 at 02:17:18 UTC, yangerkun wrote:  
> >> We now support user to set erase page size, and use do_div between len
> >> and erase size to determine the reasonableness for the erase size.
> >> However, do_div is a macro and will overwrite the value of len. Which
> >> results a mtd device with unexcepted size. Fix it by use div_u64_rem.
> >>
> >> Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
> >> Signed-off-by: yangerkun <yangerkun@huawei.com>  
> > 
> > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[yangerkun]

在 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);
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	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;
  	}
-- 

> 
>>
>> Sorry for the mistake. Can you add a fix patch for this?
> 
> In any case if the patch turns out to be wrong I'll ask you to either
> send a fix or send a v2 and I'll drop v1.

Okay, I will send the v2. Sorry again for the mistake.

Thanks,
Kun.

> 
>> 在 2021/1/4 18:18, Miquel Raynal 写道:
>>> On Wed, 2020-12-30 at 02:17:18 UTC, yangerkun wrote:
>>>> We now support user to set erase page size, and use do_div between len
>>>> and erase size to determine the reasonableness for the erase size.
>>>> However, do_div is a macro and will overwrite the value of len. Which
>>>> results a mtd device with unexcepted size. Fix it by use div_u64_rem.
>>>>
>>>> Fixes: ffad560394de ("mtd: phram: Allow the user to set the erase page size.")
>>>> Signed-off-by: yangerkun <yangerkun@huawei.com>
>>>
>>> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
> 
> Thanks,
> Miquèl
> .
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Miquel Raynal]

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/