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