| Message ID | 1544455701-5720-1-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2] nvmem: check invalid number of bytes in nvmem_{read,write}() | expand |
On 10/12/18 15:28, Biju Das wrote: > + /* Stop the user from writing */ > + if (offset >= nvmem->size) > + return -ENOSPC; > + This should be "return -EFBIG" to make utilities like dd or sh happy. Look at 38b0774c0598c ("nvmem: core: return EFBIG on out-of-range write") for more info. other than that, patch looks fine to me! --srini
Hi Srini, Thanks for the feedback. > Subject: Re: [PATCH v2] nvmem: check invalid number of bytes in > nvmem_{read,write}() > > > > On 10/12/18 15:28, Biju Das wrote: > > +/* Stop the user from writing */ > > +if (offset >= nvmem->size) > > +return -ENOSPC; > > + > This should be "return -EFBIG" to make utilities like dd or sh happy. > > Look at 38b0774c0598c ("nvmem: core: return EFBIG on out-of-range > write") for more info. return -ENOSPC also make dd or sh happy. Please let me know which is appropriate by the below results. Based on your feedback, If needed, I can send a patch with "return -EFBIG" With -ENOSPC ------------ dd if=1.bin of=/sys/bus/nvmem/devices/pcf85x63-0/nvmem bs=1 count=2 dd: writing '/sys/bus/nvmem/devices/pcf85x63-0/nvmem': No space left on device 2+0 records in 1+0 records out echo "1234" > /sys/bus/nvmem/devices/pcf85x63-0/nvmem -sh: echo: write error: No space left on device with -EFBIG ------------ dd if=1.bin of=/sys/bus/nvmem/devices/pcf85x63-0/nvmem bs=1 count dd: writing '/sys/bus/nvmem/devices/pcf85x63-0/nvmem': File too large 2+0 records in 1+0 records out echo "1234" > /sys/bus/nvmem/devices/pcf85x63-0/nvmem -sh: echo: write error: File too large 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.
On 11/12/18 08:16, Biju Das wrote: > Hi Srini, > > Thanks for the feedback. > >> Subject: Re: [PATCH v2] nvmem: check invalid number of bytes in >> nvmem_{read,write}() >> >> >> >> On 10/12/18 15:28, Biju Das wrote: >>> +/* Stop the user from writing */ >>> +if (offset >= nvmem->size) >>> +return -ENOSPC; >>> + >> This should be "return -EFBIG" to make utilities like dd or sh happy. >> >> Look at 38b0774c0598c ("nvmem: core: return EFBIG on out-of-range >> write") for more info. > > return -ENOSPC also make dd or sh happy. Please let me know which is appropriate by the below results. Based on your feedback, If needed, I can send a patch with "return -EFBIG" > Lets stay with -EFBIG for now! given the fact that it was already in the old code! --srini > With -ENOSPC > ------------ > dd if=1.bin of=/sys/bus/nvmem/devices/pcf85x63-0/nvmem bs=1 count=2 > dd: writing '/sys/bus/nvmem/devices/pcf85x63-0/nvmem': No space left on device > 2+0 records in > 1+0 records out > > echo "1234" > /sys/bus/nvmem/devices/pcf85x63-0/nvmem > -sh: echo: write error: No space left on device > > with -EFBIG > ------------ > dd if=1.bin of=/sys/bus/nvmem/devices/pcf85x63-0/nvmem bs=1 count > dd: writing '/sys/bus/nvmem/devices/pcf85x63-0/nvmem': File too large > 2+0 records in > 1+0 records out > > echo "1234" > /sys/bus/nvmem/devices/pcf85x63-0/nvmem > -sh: echo: write error: File too large > > 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. >
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 -ENOSPC; + + 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 */
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_devicenvmem_device_{read,write}()_{read,write} --- drivers/nvmem/core.c | 97 ++++++++++++++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 48 deletions(-)