* [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
@ 2020-12-30 2:17 yangerkun
2021-01-04 10:08 ` Miquel Raynal
2021-01-04 10:18 ` Miquel Raynal
0 siblings, 2 replies; 7+ messages in thread
From: yangerkun @ 2020-12-30 2:17 UTC (permalink / raw)
To: zhongguohua1, joern, miquel.raynal, richard, patrick, linux-mtd
Cc: yangerkun, yi.zhang
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/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2020-12-30 2:17 [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup yangerkun
@ 2021-01-04 10:08 ` Miquel Raynal
2021-01-04 10:18 ` Miquel Raynal
1 sibling, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2021-01-04 10:08 UTC (permalink / raw)
To: yangerkun; +Cc: zhongguohua1, yi.zhang, richard, joern, linux-mtd, patrick
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/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2020-12-30 2:17 [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup yangerkun
2021-01-04 10:08 ` Miquel Raynal
@ 2021-01-04 10:18 ` Miquel Raynal
2021-01-25 5:48 ` yangerkun
1 sibling, 1 reply; 7+ messages in thread
From: Miquel Raynal @ 2021-01-04 10:18 UTC (permalink / raw)
To: yangerkun, zhongguohua1, joern, miquel.raynal, richard, patrick,
linux-mtd
Cc: yi.zhang
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/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2021-01-04 10:18 ` Miquel Raynal
@ 2021-01-25 5:48 ` yangerkun
2021-01-25 8:28 ` Miquel Raynal
0 siblings, 1 reply; 7+ messages in thread
From: yangerkun @ 2021-01-25 5:48 UTC (permalink / raw)
To: Miquel Raynal, zhongguohua1, joern, richard, patrick, linux-mtd
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/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2021-01-25 5:48 ` yangerkun
@ 2021-01-25 8:28 ` Miquel Raynal
2021-01-25 8:36 ` yangerkun
0 siblings, 1 reply; 7+ messages in thread
From: Miquel Raynal @ 2021-01-25 8:28 UTC (permalink / raw)
To: yangerkun; +Cc: patrick, zhongguohua1, joern, linux-mtd, richard
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/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2021-01-25 8:28 ` Miquel Raynal
@ 2021-01-25 8:36 ` yangerkun
2021-01-25 8:53 ` Miquel Raynal
0 siblings, 1 reply; 7+ messages in thread
From: yangerkun @ 2021-01-25 8:36 UTC (permalink / raw)
To: Miquel Raynal; +Cc: patrick, zhongguohua1, joern, linux-mtd, richard
在 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/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup
2021-01-25 8:36 ` yangerkun
@ 2021-01-25 8:53 ` Miquel Raynal
0 siblings, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2021-01-25 8:53 UTC (permalink / raw)
To: yangerkun; +Cc: patrick, zhongguohua1, joern, linux-mtd, richard
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/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-25 8:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-30 2:17 [PATCH] mtd: phram: use div_u64_rem to stop overwrite len in phram_setup yangerkun
2021-01-04 10:08 ` Miquel Raynal
2021-01-04 10:18 ` Miquel Raynal
2021-01-25 5:48 ` yangerkun
2021-01-25 8:28 ` Miquel Raynal
2021-01-25 8:36 ` yangerkun
2021-01-25 8:53 ` Miquel Raynal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).