| Message ID | 1544182066-31528-2-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add NXP pcf85263 real-time clock support | expand |
Thanks for the patch. On 07/12/18 11:27, Biju Das wrote: > Add check in nvmem_device_{read,write}()to ensure that nvmem core never > passes an invalid number of bytes. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > --- > V3-->V4 > * New patch. > --- > drivers/nvmem/core.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > Its better to move checks from bin_attr_nvmem_read()/bin_attr_nvmem_write() into nvmem_reg_read() and nvmem_reg_write(), so its easy to maintain, rather than adding them to each function. > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index d9fd110..db7de33 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -1433,10 +1433,21 @@ int nvmem_device_read(struct nvmem_device *nvmem, > size_t bytes, void *buf) > { > int rc; > + size_t new_bytes; > > if (!nvmem) > return -EINVAL; > > + /* Stop the user from reading */ > + if ((offset >= nvmem->size) || (bytes == 0)) > + return 0; > + > + if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) > + return -EOVERFLOW; > + > + if (new_bytes > nvmem->size) > + bytes = nvmem->size - offset; > + > rc = nvmem_reg_read(nvmem, offset, buf, bytes); > > if (rc) > @@ -1461,16 +1472,29 @@ int nvmem_device_write(struct nvmem_device *nvmem, > size_t bytes, void *buf) > { > int rc; > + size_t new_bytes; > > if (!nvmem) > return -EINVAL; > > + /* Stop the user from writing */ > + if (offset >= nvmem->size) > + return -ENOSPC; > + > + if (bytes == 0) > + return 0; > + > + if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) > + return -EOVERFLOW; > + > + if (new_bytes > nvmem->size) > + bytes = nvmem->size - offset; > + > rc = nvmem_reg_write(nvmem, offset, buf, bytes); > > if (rc) > return rc; > > - Unrelated change! --srini > return bytes; > } > EXPORT_SYMBOL_GPL(nvmem_device_write); >
Hi Srinivas, Thanks for the feedback. > Subject: Re: [PATCH v4 1/5] nvmem: check invalid number of bytes in > nvmem_device_{read,write} > > Thanks for the patch. > > On 07/12/18 11:27, Biju Das wrote: > > Add check in nvmem_device_{read,write}()to ensure that nvmem core > > never passes an invalid number of bytes. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > V3-->V4 > > * New patch. > > --- > > drivers/nvmem/core.c | 26 +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > Its better to move checks from > bin_attr_nvmem_read()/bin_attr_nvmem_write() into nvmem_reg_read() > and nvmem_reg_write(), so its easy to maintain, rather than adding them to > each function. Initially I also thought the same. But there are some RTC drivers which is using nvmem_device interface is not defining "word_size" member. That is the reason we need to add it in separate function. Can you please suggest how to handle the above drivers? > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index > > d9fd110..db7de33 100644 > > --- a/drivers/nvmem/core.c > > +++ b/drivers/nvmem/core.c > > @@ -1433,10 +1433,21 @@ int nvmem_device_read(struct nvmem_device > *nvmem, > > size_t bytes, void *buf) > > { > > int rc; > > +size_t new_bytes; > > > > if (!nvmem) > > return -EINVAL; > > > > +/* Stop the user from reading */ > > +if ((offset >= nvmem->size) || (bytes == 0)) > > +return 0; > > + > > +if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) > > +return -EOVERFLOW; > > + > > +if (new_bytes > nvmem->size) > > +bytes = nvmem->size - offset; > > + > > rc = nvmem_reg_read(nvmem, offset, buf, bytes); > > > > if (rc) > > @@ -1461,16 +1472,29 @@ int nvmem_device_write(struct > nvmem_device *nvmem, > > size_t bytes, void *buf) > > { > > int rc; > > +size_t new_bytes; > > > > if (!nvmem) > > return -EINVAL; > > > > +/* Stop the user from writing */ > > +if (offset >= nvmem->size) > > +return -ENOSPC; > > + > > +if (bytes == 0) > > +return 0; > > + > > +if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) > > +return -EOVERFLOW; > > + > > +if (new_bytes > nvmem->size) > > +bytes = nvmem->size - offset; > > + > > rc = nvmem_reg_write(nvmem, offset, buf, bytes); > > > > if (rc) > > return rc; > > > > - > > Unrelated change! I found an extra space in this function. If it is a problem, I will send V2 for undoing this. 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.
On 10/12/18 12:45, Biju Das wrote: >> Its better to move checks from >> bin_attr_nvmem_read()/bin_attr_nvmem_write() into nvmem_reg_read() >> and nvmem_reg_write(), so its easy to maintain, rather than adding them to >> each function. > Initially I also thought the same. But there are some RTC drivers which is using nvmem_device > interface is not defining "word_size" member. That is the reason we need to add it in separate function. > Can you please suggest how to handle the above drivers? Word size is optional anyway, in the case where word_size is not specified, its considered as one byte word_size. --srini >
Hi Srinivas, Thanks for the feedback. > Subject: Re: [PATCH v4 1/5] nvmem: check invalid number of bytes in > nvmem_device_{read,write} > > > > On 10/12/18 12:45, Biju Das wrote: > >> Its better to move checks from > >> bin_attr_nvmem_read()/bin_attr_nvmem_write() into > nvmem_reg_read() > >> and nvmem_reg_write(), so its easy to maintain, rather than adding > >> them to each function. > > Initially I also thought the same. But there are some RTC drivers > > which is using nvmem_device interface is not defining "word_size" > member. That is the reason we need to add it in separate function. > > Can you please suggest how to handle the above drivers? > > Word size is optional anyway, in the case where word_size is not specified, > its considered as one byte word_size. Will send V2 for this. 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 d9fd110..db7de33 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -1433,10 +1433,21 @@ int nvmem_device_read(struct nvmem_device *nvmem, size_t bytes, void *buf) { int rc; + size_t new_bytes; if (!nvmem) return -EINVAL; + /* Stop the user from reading */ + if ((offset >= nvmem->size) || (bytes == 0)) + return 0; + + if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) + return -EOVERFLOW; + + if (new_bytes > nvmem->size) + bytes = nvmem->size - offset; + rc = nvmem_reg_read(nvmem, offset, buf, bytes); if (rc) @@ -1461,16 +1472,29 @@ int nvmem_device_write(struct nvmem_device *nvmem, size_t bytes, void *buf) { int rc; + size_t new_bytes; if (!nvmem) return -EINVAL; + /* Stop the user from writing */ + if (offset >= nvmem->size) + return -ENOSPC; + + if (bytes == 0) + return 0; + + if (unlikely(check_add_overflow(bytes, offset, &new_bytes))) + return -EOVERFLOW; + + if (new_bytes > nvmem->size) + bytes = nvmem->size - offset; + rc = nvmem_reg_write(nvmem, offset, buf, bytes); if (rc) return rc; - return bytes; } EXPORT_SYMBOL_GPL(nvmem_device_write);
Add check in nvmem_device_{read,write}()to ensure that nvmem core never passes an invalid number of bytes. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V3-->V4 * New patch. --- drivers/nvmem/core.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)