* [PATCH] nvmem: core: add sanity check in nvmem_device_read()
@ 2020-08-04 9:13 Bingbu Cao
2020-08-04 9:58 ` Sakari Ailus
0 siblings, 1 reply; 5+ messages in thread
From: Bingbu Cao @ 2020-08-04 9:13 UTC (permalink / raw)
To: srinivas.kandagatla, linux-kernel; +Cc: sakari.ailus, bingbu.cao, bingbu.cao
nvmem_device_read() could be called directly once nvmem device
registered, the sanity check should be done before call
nvmem_reg_read() as cell and sysfs read did now.
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
drivers/nvmem/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 927eb5f6003f..c9a77993f008 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem,
if (!nvmem)
return -EINVAL;
+ if (offset >= nvmem->size || bytes < nvmem->word_size)
+ return 0;
+
+ if (bytes + offset > nvmem->size)
+ bytes = nvmem->size - offset;
+
+ bytes = round_down(bytes, nvmem->word_size);
rc = nvmem_reg_read(nvmem, offset, buf, bytes);
if (rc)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmem: core: add sanity check in nvmem_device_read()
2020-08-04 9:13 [PATCH] nvmem: core: add sanity check in nvmem_device_read() Bingbu Cao
@ 2020-08-04 9:58 ` Sakari Ailus
2020-08-04 10:03 ` Srinivas Kandagatla
0 siblings, 1 reply; 5+ messages in thread
From: Sakari Ailus @ 2020-08-04 9:58 UTC (permalink / raw)
To: Bingbu Cao; +Cc: srinivas.kandagatla, linux-kernel, bingbu.cao
Hi Bingbu,
Thank you for the patch.
On Tue, Aug 04, 2020 at 05:13:56PM +0800, Bingbu Cao wrote:
> nvmem_device_read() could be called directly once nvmem device
> registered, the sanity check should be done before call
> nvmem_reg_read() as cell and sysfs read did now.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
> drivers/nvmem/core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 927eb5f6003f..c9a77993f008 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem,
> if (!nvmem)
> return -EINVAL;
>
> + if (offset >= nvmem->size || bytes < nvmem->word_size)
> + return 0;
> +
> + if (bytes + offset > nvmem->size)
> + bytes = nvmem->size - offset;
The check is relevant for nvmem_device_write(), too.
There are also other ways to access nvmem devices such as nvmem_cell_read
and others alike. Should they be considered as well?
> +
> + bytes = round_down(bytes, nvmem->word_size);
> rc = nvmem_reg_read(nvmem, offset, buf, bytes);
>
> if (rc)
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmem: core: add sanity check in nvmem_device_read()
2020-08-04 9:58 ` Sakari Ailus
@ 2020-08-04 10:03 ` Srinivas Kandagatla
2020-08-04 10:44 ` Bingbu Cao
0 siblings, 1 reply; 5+ messages in thread
From: Srinivas Kandagatla @ 2020-08-04 10:03 UTC (permalink / raw)
To: Sakari Ailus, Bingbu Cao; +Cc: linux-kernel, bingbu.cao
On 04/08/2020 10:58, Sakari Ailus wrote:
> Hi Bingbu,
>
> Thank you for the patch.
>
> On Tue, Aug 04, 2020 at 05:13:56PM +0800, Bingbu Cao wrote:
>> nvmem_device_read() could be called directly once nvmem device
>> registered, the sanity check should be done before call
>> nvmem_reg_read() as cell and sysfs read did now.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> ---
>> drivers/nvmem/core.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 927eb5f6003f..c9a77993f008 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem,
>> if (!nvmem)
>> return -EINVAL;
>>
>> + if (offset >= nvmem->size || bytes < nvmem->word_size)
>> + return 0;
>> +
>> + if (bytes + offset > nvmem->size)
>> + bytes = nvmem->size - offset;
>
> The check is relevant for nvmem_device_write(), too.
>
> There are also other ways to access nvmem devices such as nvmem_cell_read
> and others alike. Should they be considered as well?
We should probably move these sanity checks to a common place like
nvmem_reg_read() and nvmem_reg_write(), so the callers need not
duplicate the same!
--srini
>
>> +
>> + bytes = round_down(bytes, nvmem->word_size);
>> rc = nvmem_reg_read(nvmem, offset, buf, bytes);
>>
>> if (rc)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmem: core: add sanity check in nvmem_device_read()
2020-08-04 10:03 ` Srinivas Kandagatla
@ 2020-08-04 10:44 ` Bingbu Cao
2020-08-04 10:56 ` Sakari Ailus
0 siblings, 1 reply; 5+ messages in thread
From: Bingbu Cao @ 2020-08-04 10:44 UTC (permalink / raw)
To: Srinivas Kandagatla, Sakari Ailus, Bingbu Cao; +Cc: linux-kernel
On 8/4/20 6:03 PM, Srinivas Kandagatla wrote:
>
>
> On 04/08/2020 10:58, Sakari Ailus wrote:
>> Hi Bingbu,
>>
>> Thank you for the patch.
>>
>> On Tue, Aug 04, 2020 at 05:13:56PM +0800, Bingbu Cao wrote:
>>> nvmem_device_read() could be called directly once nvmem device
>>> registered, the sanity check should be done before call
>>> nvmem_reg_read() as cell and sysfs read did now.
>>>
>>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>>> ---
>>> drivers/nvmem/core.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> index 927eb5f6003f..c9a77993f008 100644
>>> --- a/drivers/nvmem/core.c
>>> +++ b/drivers/nvmem/core.c
>>> @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem,
>>> if (!nvmem)
>>> return -EINVAL;
>>> + if (offset >= nvmem->size || bytes < nvmem->word_size)
>>> + return 0;
>>> +
>>> + if (bytes + offset > nvmem->size)
>>> + bytes = nvmem->size - offset;
>>
>> The check is relevant for nvmem_device_write(), too.
>>
>> There are also other ways to access nvmem devices such as nvmem_cell_read
>> and others alike. Should they be considered as well?
>
> We should probably move these sanity checks to a common place like
> nvmem_reg_read() and nvmem_reg_write(), so the callers need not duplicate the same!
>
Srini and Sakari, thanks for your review.
Is it OK just return INVAL with simple check like below?
if (bytes + offset > nvmem->size ||
bytes != round_down(bytes, nvmem->word_size))
return -EINVAL;
> --srini
>
>>
>>> +
>>> + bytes = round_down(bytes, nvmem->word_size);
>>> rc = nvmem_reg_read(nvmem, offset, buf, bytes);
>>> if (rc)
>>
--
Best regards,
Bingbu Cao
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] nvmem: core: add sanity check in nvmem_device_read()
2020-08-04 10:44 ` Bingbu Cao
@ 2020-08-04 10:56 ` Sakari Ailus
0 siblings, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2020-08-04 10:56 UTC (permalink / raw)
To: Bingbu Cao; +Cc: Srinivas Kandagatla, Bingbu Cao, linux-kernel
On Tue, Aug 04, 2020 at 06:44:27PM +0800, Bingbu Cao wrote:
>
> On 8/4/20 6:03 PM, Srinivas Kandagatla wrote:
> >
> >
> > On 04/08/2020 10:58, Sakari Ailus wrote:
> >> Hi Bingbu,
> >>
> >> Thank you for the patch.
> >>
> >> On Tue, Aug 04, 2020 at 05:13:56PM +0800, Bingbu Cao wrote:
> >>> nvmem_device_read() could be called directly once nvmem device
> >>> registered, the sanity check should be done before call
> >>> nvmem_reg_read() as cell and sysfs read did now.
> >>>
> >>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> >>> ---
> >>> drivers/nvmem/core.c | 7 +++++++
> >>> 1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> >>> index 927eb5f6003f..c9a77993f008 100644
> >>> --- a/drivers/nvmem/core.c
> >>> +++ b/drivers/nvmem/core.c
> >>> @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem,
> >>> if (!nvmem)
> >>> return -EINVAL;
> >>> + if (offset >= nvmem->size || bytes < nvmem->word_size)
> >>> + return 0;
> >>> +
> >>> + if (bytes + offset > nvmem->size)
> >>> + bytes = nvmem->size - offset;
> >>
> >> The check is relevant for nvmem_device_write(), too.
> >>
> >> There are also other ways to access nvmem devices such as nvmem_cell_read
> >> and others alike. Should they be considered as well?
> >
> > We should probably move these sanity checks to a common place like
> > nvmem_reg_read() and nvmem_reg_write(), so the callers need not duplicate the same!
> >
> Srini and Sakari, thanks for your review.
>
> Is it OK just return INVAL with simple check like below?
>
> if (bytes + offset > nvmem->size ||
> bytes != round_down(bytes, nvmem->word_size))
> return -EINVAL;
This changes what is currently supported so I'd say no.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-04 10:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04 9:13 [PATCH] nvmem: core: add sanity check in nvmem_device_read() Bingbu Cao
2020-08-04 9:58 ` Sakari Ailus
2020-08-04 10:03 ` Srinivas Kandagatla
2020-08-04 10:44 ` Bingbu Cao
2020-08-04 10:56 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®