[PATCH] zram: avoid calling bio_endio() before io complete
When the bio is given as parameter, zram calls the bio_endio() with given bio after zram io finished. However in the case of partial writing, the __zram_bvec_read() calls the bio_endio() with the given bio regardless of the write operation. In other words, the bio_endio() may be called at an unwanted time. So this patch avoids calling bio_endio() with given bio before writing is completed. Signed-off-by: Huijin Park --- drivers/block/zram/zram_drv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 4879595..b3d3ba2 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1226,7 +1226,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, if (!page) return -ENOMEM; - ret = __zram_bvec_read(zram, page, index, bio, true); + ret = __zram_bvec_read(zram, page, index, NULL, true); if (ret) goto out; -- 1.7.9.5
Re: [PATCH,1/2] genhd: avoid overflow of sectors in disk_stats
Hi Omar Sandoval,
On Sat, Dec 1, 2018 at 4:56 AM Omar Sandoval wrote:
>
> On Fri, Nov 30, 2018 at 04:32:40AM -0500, Huijin Park wrote:
> > From: "huijin.park"
> >
> > This patch changes the 'sectors' type to an u64.
> > In 32 bit system, the 'sectors' can accumulate up to about 2TiB.
> > If a 32 bit system makes i/o over 2TiB while running,
> > the 'sectors' will overflow.
> > As a result, the part_stat_read(sectors), the diskstats in proc and
> > the (lifetime|session)_write_kbytes in sysfs return invalid statistic.
>
> What about parsers which expect it to be an unsigned long? E.g., iostat:
> https://github.com/sysstat/sysstat/blob/v12.1.1/rd_stats.c#L736
>
> At least with glibc, scanf seems to truncate sanely, but this appears to
> be undefined.
The glibc APIs such as scanf and strtoul set return value and errno
to ULONG_MAX and ERANGE in overflow case.
I think ULONG_MAX and ERANGE are better than reset to zero because
of overflow. At least, application can notice overflow with errno.
Besides nowadays, a 32 bit is not enough size to show
an i/o accumulated size.
I met a problem like below. So I suggested this patch.
sh-3.2# mount | grep p19
/dev/mmcblk0p19 on /ext4_dir type ext4 (rw,relatime,data=ordered)
sh-3.2# cat /sys/fs/ext4/mmcblk0p19/session_write_kbytes
2147467268
sh-3.2# cat /sys/fs/ext4/mmcblk0p19/lifetime_write_kbytes
2147568561
sh-3.2# dd if=/dev/zero of=/ext4_dir/temp.bin bs=1M count=20 oflag=sync
20+0 records in
20+0 records out
20971520 bytes (21 MB) copied,
0.621939 s, 33.7 MB/s
sh-3.2# cat /sys/fs/ext4/mmcblk0p19/session_write_kbytes
4524
sh-3.2# cat /sys/fs/ext4/mmcblk0p19/lifetime_write_kbytes
105817
>
> > Signed-off-by: huijin.park
> > ---
> > block/genhd.c |6 +++---
> > include/linux/genhd.h |2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/block/genhd.c b/block/genhd.c
> > index 0145bcb..7518dcd 100644
> > --- a/block/genhd.c
> > +++ b/block/genhd.c
> > @@ -1343,10 +1343,10 @@ static int diskstats_show(struct seq_file *seqf,
> > void *v)
> > part_stat_unlock();
> > part_in_flight(gp->queue, hd, inflight);
> > seq_printf(seqf, "%4d %7d %s "
> > -"%lu %lu %lu %u "
> > -"%lu %lu %lu %u "
> > +"%lu %lu %llu %u "
> > +"%lu %lu %llu %u "
> > "%u %u %u "
> > -"%lu %lu %lu %u\n",
> > +"%lu %lu %llu %u\n",
> > MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
> > disk_name(gp, hd->partno, buf),
> > part_stat_read(hd, ios[STAT_READ]),
> > diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> > index 0c5ee17..5bf86f9 100644
> > --- a/include/linux/genhd.h
> > +++ b/include/linux/genhd.h
> > @@ -84,7 +84,7 @@ struct partition {
> >
> > struct disk_stats {
> > u64 nsecs[NR_STAT_GROUPS];
> > - unsigned long sectors[NR_STAT_GROUPS];
> > + u64 sectors[NR_STAT_GROUPS];
> > unsigned long ios[NR_STAT_GROUPS];
> > unsigned long merges[NR_STAT_GROUPS];
> > unsigned long io_ticks;
> > --
> > 1.7.9.5
> >
Thanks,
Huijin
[PATCH v3] mtd: change len type from signed to unsigned type
From: "huijin.park"
Callers of erase_write() always pass an unsigned int.
So this patch avoids a cast to an int.
Signed-off-by: huijin.park
Reviewed-by: Miquel Raynal
---
Changes since v1:
* Made commit message more clear.
Changes since v2:
* Made commit message more clear.
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH v3] mtd: change len type from signed to unsigned type
From: "huijin.park"
Callers of erase_write() always pass an unsigned int.
So this patch avoids a cast to an int.
Signed-off-by: huijin.park
Reviewed-by: Miquel Raynal
---
Changes since v1:
* Made commit message more clear.
Changes since v2:
* Made commit message more clear.
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
Re: [PATCH v2] mtd: change len type from signed to unsigned type
Hi Miquèl,
On Thu, Nov 15, 2018 at 6:29 PM Miquel Raynal wrote:
>
> Hi Huijin,
>
> Huijin Park wrote on Thu, 15 Nov 2018
> 00:07:10 -0500:
>
> > From: "huijin.park"
> >
> > This patch casts the "len" parameter to an unsigned int.
> > The callers of erase_write() pass the "len" parameter as unsigned int.
>
> Indeed. Perhaps it is worth backporting this patch to a stable releases?
>
It doesn't need backporting.
Because this patch is for enforcing code correctness.
> >
> > Signed-off-by: huijin.park
> > ---
> > drivers/mtd/mtdblock.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> > index a5b1933..b2d5ed1 100644
> > --- a/drivers/mtd/mtdblock.c
> > +++ b/drivers/mtd/mtdblock.c
> > @@ -56,7 +56,7 @@ struct mtdblk_dev {
> > */
> >
> > static int erase_write (struct mtd_info *mtd, unsigned long pos,
> > - int len, const char *buf)
> > + unsigned int len, const char *buf)
> > {
> > struct erase_info erase;
> > size_t retlen;
>
> Reviewed-by: Miquel Raynal
>
>
> Thanks,
> Miquèl
Thanks,
Huijin
Re: [PATCH v2] mtd: change len type from signed to unsigned type
Hi Miquèl,
On Thu, Nov 15, 2018 at 6:29 PM Miquel Raynal wrote:
>
> Hi Huijin,
>
> Huijin Park wrote on Thu, 15 Nov 2018
> 00:07:10 -0500:
>
> > From: "huijin.park"
> >
> > This patch casts the "len" parameter to an unsigned int.
> > The callers of erase_write() pass the "len" parameter as unsigned int.
>
> Indeed. Perhaps it is worth backporting this patch to a stable releases?
>
It doesn't need backporting.
Because this patch is for enforcing code correctness.
> >
> > Signed-off-by: huijin.park
> > ---
> > drivers/mtd/mtdblock.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> > index a5b1933..b2d5ed1 100644
> > --- a/drivers/mtd/mtdblock.c
> > +++ b/drivers/mtd/mtdblock.c
> > @@ -56,7 +56,7 @@ struct mtdblk_dev {
> > */
> >
> > static int erase_write (struct mtd_info *mtd, unsigned long pos,
> > - int len, const char *buf)
> > + unsigned int len, const char *buf)
> > {
> > struct erase_info erase;
> > size_t retlen;
>
> Reviewed-by: Miquel Raynal
>
>
> Thanks,
> Miquèl
Thanks,
Huijin
[PATCH v3] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" to an u64. Signed-off-by: huijin.park --- Changes since v1: * Made commit subject and message more clear. Changes since v2: * Adding the cast to only the first operand. --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..59a1075 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH v3] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" to an u64. Signed-off-by: huijin.park --- Changes since v1: * Made commit subject and message more clear. Changes since v2: * Adding the cast to only the first operand. --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..59a1075 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH v2] mtd: change len type from signed to unsigned type
From: "huijin.park"
This patch casts the "len" parameter to an unsigned int.
The callers of erase_write() pass the "len" parameter as unsigned int.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH v2] mtd: change len type from signed to unsigned type
From: "huijin.park"
This patch casts the "len" parameter to an unsigned int.
The callers of erase_write() pass the "len" parameter as unsigned int.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH v2] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" and "info->n_sectors" to an u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH v2] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" and "info->n_sectors" to an u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" and "info->n_sectors" to an u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH] mtd: spi-nor: cast to u64 to avoid uint overflows
From: "huijin.park" The "params->size" is defined as "u64". And "info->sector_size" and "info->n_sectors" are defined as unsigned int and u16. Thus, u64 data might have strange data(loss data) if the result overflows an unsigned int. This patch casts "info->sector_size" and "info->n_sectors" to an u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH] mtd: change len type from signed to unsigned type
From: "huijin.park"
This patch casts the "len" parameter to an unsigned int.
The callers of erase_write() pass the "len" parameter as unsigned int.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH] mtd: change len type from signed to unsigned type
From: "huijin.park"
This patch casts the "len" parameter to an unsigned int.
The callers of erase_write() pass the "len" parameter as unsigned int.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
Re: [PATCH] mtd: change len type from signed to unsigned type
Hi Boris,
On Wed, Oct 31, 2018 at 11:02 PM Boris Brezillon
wrote:
>
> Hi Huijin,
>
> On Thu, 23 Aug 2018 04:43:39 -0400
> Huijin Park wrote:
>
> > From: "huijin.park"
> >
> > assign of a signed value which has type 'int' to a variable of
> > a bigger unsigned integer type 'uint64_t'.
>
> Why are you mentioning u64? AFAICT, the len passed to erase_write() is
> always an unsigned int.
It's my mistake.
Messages about u64 are not related to this patch.
>
> > this is ok most of the time, but can lead to unexpectedly large
> > resulting value if the original signed value is negative.
> > in addtion, the callers of the erase_write() pass the len parameter
>
> ^In addition,
>
> > as unsigned type.
> >
> > Signed-off-by: huijin.park
> > ---
> > drivers/mtd/mtdblock.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> > index a5b1933..b2d5ed1 100644
> > --- a/drivers/mtd/mtdblock.c
> > +++ b/drivers/mtd/mtdblock.c
> > @@ -56,7 +56,7 @@ struct mtdblk_dev {
> > */
> >
> > static int erase_write (struct mtd_info *mtd, unsigned long pos,
> > - int len, const char *buf)
> > + unsigned int len, const char *buf)
>
> The diff looks good, but the commit message is not clear at all. Can
> you reword it?
>
> Thanks,
>
> Boris
>
> > {
> > struct erase_info erase;
> > size_t retlen;
>
I will send again patch after reword the commit message.
Thanks & best regards,
Huijin
Re: [PATCH] mtd: change len type from signed to unsigned type
Hi Boris,
On Wed, Oct 31, 2018 at 11:02 PM Boris Brezillon
wrote:
>
> Hi Huijin,
>
> On Thu, 23 Aug 2018 04:43:39 -0400
> Huijin Park wrote:
>
> > From: "huijin.park"
> >
> > assign of a signed value which has type 'int' to a variable of
> > a bigger unsigned integer type 'uint64_t'.
>
> Why are you mentioning u64? AFAICT, the len passed to erase_write() is
> always an unsigned int.
It's my mistake.
Messages about u64 are not related to this patch.
>
> > this is ok most of the time, but can lead to unexpectedly large
> > resulting value if the original signed value is negative.
> > in addtion, the callers of the erase_write() pass the len parameter
>
> ^In addition,
>
> > as unsigned type.
> >
> > Signed-off-by: huijin.park
> > ---
> > drivers/mtd/mtdblock.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> > index a5b1933..b2d5ed1 100644
> > --- a/drivers/mtd/mtdblock.c
> > +++ b/drivers/mtd/mtdblock.c
> > @@ -56,7 +56,7 @@ struct mtdblk_dev {
> > */
> >
> > static int erase_write (struct mtd_info *mtd, unsigned long pos,
> > - int len, const char *buf)
> > + unsigned int len, const char *buf)
>
> The diff looks good, but the commit message is not clear at all. Can
> you reword it?
>
> Thanks,
>
> Boris
>
> > {
> > struct erase_info erase;
> > size_t retlen;
>
I will send again patch after reword the commit message.
Thanks & best regards,
Huijin
Re: [PATCH] mtd: cast to u64 to avoid unexpected error
Hi Boris Thanks for review. I will send patch again. On Wed, Oct 31, 2018 at 10:55 PM Boris Brezillon wrote: > > Hi Huijin, > > Subject prefix should be "mtd: spi-nor: ...", and please replace > "unexpected error" by "unsigned int overflows". > > On Thu, 23 Aug 2018 03:28:02 -0400 > Huijin Park wrote: > > > From: "huijin.park" > > > > the params->size is defined as "u64" > > and, "info->sector_size" and "info->n_sectors" is defined as unsgined and > > u16 > > ^ are ^ unsigned > > > thus, u64 data might have strange data(loss data) if data is overflow. > > ^ if the result > overflows an unsigned int. > > > this patch cast it to u64. > > ^casts info->sector_size to an u64. > > > > > Signed-off-by: huijin.park > > --- > > drivers/mtd/spi-nor/spi-nor.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > > index d9c368c..527f281 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, > > memset(params, 0, sizeof(*params)); > > > > /* Set SPI NOR sizes. */ > > - params->size = info->sector_size * info->n_sectors; > > + params->size = (u64)info->sector_size * (u64)info->n_sectors; > > ^ this cast is not needed. > > BTW, I doubt we'll ever have to deal with NORs that are more than 4GB, > but making static code analysis tools happy and enforcing code > correctness is important too. > I agree that enforcing code correctness is important as your said too. > > params->page_size = info->page_size; > > > > /* (Fast) Read settings. */ > > Regards, > > Boris Thanks & best regards, Huijin
Re: [PATCH] mtd: cast to u64 to avoid unexpected error
Hi Boris Thanks for review. I will send patch again. On Wed, Oct 31, 2018 at 10:55 PM Boris Brezillon wrote: > > Hi Huijin, > > Subject prefix should be "mtd: spi-nor: ...", and please replace > "unexpected error" by "unsigned int overflows". > > On Thu, 23 Aug 2018 03:28:02 -0400 > Huijin Park wrote: > > > From: "huijin.park" > > > > the params->size is defined as "u64" > > and, "info->sector_size" and "info->n_sectors" is defined as unsgined and > > u16 > > ^ are ^ unsigned > > > thus, u64 data might have strange data(loss data) if data is overflow. > > ^ if the result > overflows an unsigned int. > > > this patch cast it to u64. > > ^casts info->sector_size to an u64. > > > > > Signed-off-by: huijin.park > > --- > > drivers/mtd/spi-nor/spi-nor.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > > index d9c368c..527f281 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, > > memset(params, 0, sizeof(*params)); > > > > /* Set SPI NOR sizes. */ > > - params->size = info->sector_size * info->n_sectors; > > + params->size = (u64)info->sector_size * (u64)info->n_sectors; > > ^ this cast is not needed. > > BTW, I doubt we'll ever have to deal with NORs that are more than 4GB, > but making static code analysis tools happy and enforcing code > correctness is important too. > I agree that enforcing code correctness is important as your said too. > > params->page_size = info->page_size; > > > > /* (Fast) Read settings. */ > > Regards, > > Boris Thanks & best regards, Huijin
[PATCH] mtd: change len type from signed to unsigned type
From: "huijin.park"
assign of a signed value which has type 'int' to a variable of
a bigger unsigned integer type 'uint64_t'.
this is ok most of the time, but can lead to unexpectedly large
resulting value if the original signed value is negative.
in addtion, the callers of the erase_write() pass the len parameter
as unsigned type.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH] mtd: change len type from signed to unsigned type
From: "huijin.park"
assign of a signed value which has type 'int' to a variable of
a bigger unsigned integer type 'uint64_t'.
this is ok most of the time, but can lead to unexpectedly large
resulting value if the original signed value is negative.
in addtion, the callers of the erase_write() pass the len parameter
as unsigned type.
Signed-off-by: huijin.park
---
drivers/mtd/mtdblock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index a5b1933..b2d5ed1 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -56,7 +56,7 @@ struct mtdblk_dev {
*/
static int erase_write (struct mtd_info *mtd, unsigned long pos,
- int len, const char *buf)
+ unsigned int len, const char *buf)
{
struct erase_info erase;
size_t retlen;
--
1.7.9.5
[PATCH] mtd: cast to u64 to avoid unexpected error
From: "huijin.park" the params->size is defined as "u64" and, "info->sector_size" and "info->n_sectors" is defined as unsgined and u16 thus, u64 data might have strange data(loss data) if data is overflow. this patch cast it to u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
[PATCH] mtd: cast to u64 to avoid unexpected error
From: "huijin.park" the params->size is defined as "u64" and, "info->sector_size" and "info->n_sectors" is defined as unsgined and u16 thus, u64 data might have strange data(loss data) if data is overflow. this patch cast it to u64. Signed-off-by: huijin.park --- drivers/mtd/spi-nor/spi-nor.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index d9c368c..527f281 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2459,7 +2459,7 @@ static int spi_nor_init_params(struct spi_nor *nor, memset(params, 0, sizeof(*params)); /* Set SPI NOR sizes. */ - params->size = info->sector_size * info->n_sectors; + params->size = (u64)info->sector_size * (u64)info->n_sectors; params->page_size = info->page_size; /* (Fast) Read settings. */ -- 1.7.9.5
