Linux-RTC Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}()
@ 2018-12-10 15:28 Biju Das
  2018-12-10 18:34 ` Srinivas Kandagatla
  0 siblings, 1 reply; 4+ messages in thread
From: Biju Das @ 2018-12-10 15:28 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_devicenvmem_device_{read,write}()_{read,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 -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 */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}()
  2018-12-10 15:28 [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}() Biju Das
@ 2018-12-10 18:34 ` Srinivas Kandagatla
  2018-12-11  8:16   ` Biju Das
  0 siblings, 1 reply; 4+ messages in thread
From: Srinivas Kandagatla @ 2018-12-10 18:34 UTC (permalink / raw)
  To: Biju Das
  Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
	Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
	linux-renesas-soc



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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}()
  2018-12-10 18:34 ` Srinivas Kandagatla
@ 2018-12-11  8:16   ` Biju Das
  2018-12-11  9:31     ` Srinivas Kandagatla
  0 siblings, 1 reply; 4+ messages in thread
From: Biju Das @ 2018-12-11  8:16 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
	Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
	linux-renesas-soc

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}()
  2018-12-11  8:16   ` Biju Das
@ 2018-12-11  9:31     ` Srinivas Kandagatla
  0 siblings, 0 replies; 4+ messages in thread
From: Srinivas Kandagatla @ 2018-12-11  9:31 UTC (permalink / raw)
  To: Biju Das
  Cc: Simon Horman, Geert Uytterhoeven, Chris Paterson,
	Alexandre Belloni, Alessandro Zummo, Fabrizio Castro, linux-rtc,
	linux-renesas-soc



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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 15:28 [PATCH v2] nvmem: check invalid number of bytes in nvmem_{read,write}() Biju Das
2018-12-10 18:34 ` Srinivas Kandagatla
2018-12-11  8:16   ` Biju Das
2018-12-11  9:31     ` Srinivas Kandagatla

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org linux-rtc@archiver.kernel.org
	public-inbox-index linux-rtc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox