* [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
@ 2018-12-11 9:24 Biju Das
2018-12-11 9:55 ` Srinivas Kandagatla
0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2018-12-11 9:24 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Biju Das, Simon Horman, Geert Uytterhoeven, Chris Paterson,
Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
linux-renesas-soc
Add check in nvmem_{read,write}()to ensure that nvmem core never passes
an invalid number of bytes.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
* Moved checks from bin_attr_nvmem_{read/write} into nvmem_reg_{read,write}
* Removed checks from nvmem_device_device_{read,write}()
V2-->V3
* Changed the Error from "ENOSPC" to "EFBIG" on nvmem_reg_write.
---
drivers/nvmem/core.c | 97 ++++++++++++++++++++++++++--------------------------
1 file changed, 49 insertions(+), 48 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index f7301bb..dfbc8ff 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -76,19 +76,60 @@ static struct lock_class_key eeprom_lock_key;
static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
void *val, size_t bytes)
{
- if (nvmem->reg_read)
- return nvmem->reg_read(nvmem->priv, offset, val, bytes);
+ int rc;
+ size_t new_bytes;
+
+ /* Stop the user from reading */
+ if ((offset >= nvmem->size) || (bytes == 0))
+ return 0;
+
+ if ((nvmem->reg_read == NULL) || (bytes < nvmem->word_size))
+ return -EINVAL;
+
+ if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
+ return -EOVERFLOW;
- return -EINVAL;
+ if (new_bytes > nvmem->size)
+ bytes = nvmem->size - offset;
+
+ bytes = round_down(bytes, nvmem->word_size);
+ rc = nvmem->reg_read(nvmem->priv, offset, val, bytes);
+
+ if (rc)
+ return rc;
+
+ return bytes;
}
static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
void *val, size_t bytes)
{
- if (nvmem->reg_write)
- return nvmem->reg_write(nvmem->priv, offset, val, bytes);
+ int rc;
+ size_t new_bytes;
+
+ /* Stop the user from writing */
+ if (offset >= nvmem->size)
+ return -EFBIG;
+
+ if (bytes == 0)
+ return 0;
+
+ if ((nvmem->reg_write == NULL) || (bytes < nvmem->word_size))
+ return -EINVAL;
+
+ if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
+ return -EOVERFLOW;
+
+ if (new_bytes > nvmem->size)
+ bytes = nvmem->size - offset;
+
+ bytes = round_down(bytes, nvmem->word_size);
+ rc = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+
+ if (rc)
+ return rc;
- return -EINVAL;
+ return bytes;
}
static ssize_t type_show(struct device *dev,
@@ -111,33 +152,13 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
char *buf, loff_t pos, size_t count)
{
struct device *dev;
- struct nvmem_device *nvmem;
- int rc;
if (attr->private)
dev = attr->private;
else
dev = container_of(kobj, struct device, kobj);
- nvmem = to_nvmem_device(dev);
-
- /* Stop the user from reading */
- if (pos >= nvmem->size)
- return 0;
-
- if (count < nvmem->word_size)
- return -EINVAL;
-
- if (pos + count > nvmem->size)
- count = nvmem->size - pos;
-
- count = round_down(count, nvmem->word_size);
-
- rc = nvmem_reg_read(nvmem, pos, buf, count);
-
- if (rc)
- return rc;
- return count;
+ return nvmem_reg_read(to_nvmem_device(dev), pos, buf, count);
}
static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
@@ -145,33 +166,13 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
char *buf, loff_t pos, size_t count)
{
struct device *dev;
- struct nvmem_device *nvmem;
- int rc;
if (attr->private)
dev = attr->private;
else
dev = container_of(kobj, struct device, kobj);
- nvmem = to_nvmem_device(dev);
-
- /* Stop the user from writing */
- if (pos >= nvmem->size)
- return -EFBIG;
-
- if (count < nvmem->word_size)
- return -EINVAL;
-
- if (pos + count > nvmem->size)
- count = nvmem->size - pos;
-
- count = round_down(count, nvmem->word_size);
-
- rc = nvmem_reg_write(nvmem, pos, buf, count);
-
- if (rc)
- return rc;
- return count;
+ return nvmem_reg_write(to_nvmem_device(dev), pos, buf, count);
}
/* default read/write permissions */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
2018-12-11 9:24 [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}() Biju Das
@ 2018-12-11 9:55 ` Srinivas Kandagatla
2018-12-11 10:54 ` Biju Das
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas Kandagatla @ 2018-12-11 9:55 UTC (permalink / raw)
To: Biju Das
Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
linux-renesas-soc
After applying this patch I get below errors with W=1 C=1
On 11/12/18 09:24, Biju Das wrote:
> + if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
> + return -EOVERFLOW;
> drivers/nvmem/core.c:82:13: error: incompatible types in comparison
expression (different type sizes)
drivers/nvmem/core.c:82:13: error: incompatible types in comparison
expression (different type sizes)
drivers/nvmem/core.c:113:13: error: incompatible types in comparison
expression (different type sizes)
drivers/nvmem/core.c:113:13: error: incompatible types in comparison
expression (different type sizes)
--srini
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
2018-12-11 9:55 ` Srinivas Kandagatla
@ 2018-12-11 10:54 ` Biju Das
2018-12-11 11:06 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2018-12-11 10:54 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
linux-renesas-soc
Hello Srini,
Thanks for the feedback.
> Subject: Re: [PATCH v3] nvmem: check invalid number of bytes in
> nvmem_{read,write}()
>
>
>
> After applying this patch I get below errors with W=1 C=1
>
> On 11/12/18 09:24, Biju Das wrote:
> > +if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
> > +return -EOVERFLOW;
> > drivers/nvmem/core.c:82:13: error: incompatible types in comparison
> expression (different type sizes)
> drivers/nvmem/core.c:82:13: error: incompatible types in comparison
> expression (different type sizes)
> drivers/nvmem/core.c:113:13: error: incompatible types in comparison
> expression (different type sizes)
> drivers/nvmem/core.c:113:13: error: incompatible types in comparison
> expression (different type sizes)
I was trying with arm32 toolchain and compiler happy. Now tried with Arm64 toolchain, it provides a warning and the below typecast fixed the issue.
if (unlikely(check_add_overflow(bytes, (size_t)offset, &new_bytes)))
Does typecasting to (size_t) fixed the issue in your environment? Please let me know.
regards,
Biju
[https://www2.renesas.eu/media/email/unicef.jpg]
This Christmas, instead of sending out cards, Renesas Electronics Europe have decided to support Unicef with a donation. For further details click here<https://www.unicef.org/> to find out about the valuable work they do, helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a prosperous New Year.
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
2018-12-11 10:54 ` Biju Das
@ 2018-12-11 11:06 ` Geert Uytterhoeven
2018-12-11 11:10 ` Srinivas Kandagatla
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-12-11 11:06 UTC (permalink / raw)
To: Biju Das
Cc: Srinivas Kandagatla, Simon Horman, Geert Uytterhoeven,
Chris Paterson, Alexandre Belloni, Alessandro Zummo,
Fabrizio Castro, linux-rtc, Linux-Renesas
Hi Biju,
On Tue, Dec 11, 2018 at 11:55 AM Biju Das <biju.das@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v3] nvmem: check invalid number of bytes in
> > nvmem_{read,write}()
> >
> > After applying this patch I get below errors with W=1 C=1
> >
> > On 11/12/18 09:24, Biju Das wrote:
> > > +if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
> > > +return -EOVERFLOW;
> > > drivers/nvmem/core.c:82:13: error: incompatible types in comparison
> > expression (different type sizes)
> > drivers/nvmem/core.c:82:13: error: incompatible types in comparison
> > expression (different type sizes)
> > drivers/nvmem/core.c:113:13: error: incompatible types in comparison
> > expression (different type sizes)
> > drivers/nvmem/core.c:113:13: error: incompatible types in comparison
> > expression (different type sizes)
>
> I was trying with arm32 toolchain and compiler happy. Now tried with Arm64 toolchain, it provides a warning and the below typecast fixed the issue.
> if (unlikely(check_add_overflow(bytes, (size_t)offset, &new_bytes)))
>
> Does typecasting to (size_t) fixed the issue in your environment? Please let me know.
That's a side-effect of offset not being loff_t...
I think that should be fixed first, else you will forget about
removing the cast later
("casts are evil").
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
2018-12-11 11:06 ` Geert Uytterhoeven
@ 2018-12-11 11:10 ` Srinivas Kandagatla
2018-12-11 11:27 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas Kandagatla @ 2018-12-11 11:10 UTC (permalink / raw)
To: Geert Uytterhoeven, Biju Das
Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
Linux-Renesas
On 11/12/18 11:06, Geert Uytterhoeven wrote:
>> I was trying with arm32 toolchain and compiler happy. Now tried with Arm64 toolchain, it provides a warning and the below typecast fixed the issue.
>> if (unlikely(check_add_overflow(bytes, (size_t)offset, &new_bytes)))
>>
>> Does typecasting to (size_t) fixed the issue in your environment? Please let me know.
> That's a side-effect of offset not being loff_t...
check_add_overflow will expect all the params to be of same type, so
changing to loff_t will not help too.
> I think that should be fixed first, else you will forget about
> removing the cast later
Yes, I agree.
> ("casts are evil").
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}()
2018-12-11 11:10 ` Srinivas Kandagatla
@ 2018-12-11 11:27 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-12-11 11:27 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Biju Das, Simon Horman, Geert Uytterhoeven, Chris Paterson,
Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
Linux-Renesas
Hi Srinivas,
On Tue, Dec 11, 2018 at 12:10 PM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> On 11/12/18 11:06, Geert Uytterhoeven wrote:
> >> I was trying with arm32 toolchain and compiler happy. Now tried with Arm64 toolchain, it provides a warning and the below typecast fixed the issue.
> >> if (unlikely(check_add_overflow(bytes, (size_t)offset, &new_bytes)))
> >>
> >> Does typecasting to (size_t) fixed the issue in your environment? Please let me know.
> > That's a side-effect of offset not being loff_t...
> check_add_overflow will expect all the params to be of same type, so
> changing to loff_t will not help too.
Right, it has separate checks for signed/unsigned additions, but
both parameter types must have the same signedness, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-11 11:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 9:24 [PATCH v3] nvmem: check invalid number of bytes in nvmem_{read,write}() Biju Das
2018-12-11 9:55 ` Srinivas Kandagatla
2018-12-11 10:54 ` Biju Das
2018-12-11 11:06 ` Geert Uytterhoeven
2018-12-11 11:10 ` Srinivas Kandagatla
2018-12-11 11:27 ` Geert Uytterhoeven
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).