* [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions()
@ 2021-06-17 13:37 Dan Carpenter
2021-06-25 9:50 ` Manivannan Sadhasivam
2021-07-15 22:50 ` Miquel Raynal
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-06-17 13:37 UTC (permalink / raw)
To: Miquel Raynal, Manivannan Sadhasivam
Cc: Richard Weinberger, Vignesh Raghavendra, Boris Brezillon,
Tudor Ambarus, linux-mtd, kernel-janitors
Check for whether of_property_count_elems_of_size() returns a negative
error code.
Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/mtd/nand/raw/nand_base.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 57a583149cc0..cbba46432e39 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5231,8 +5231,8 @@ static int of_get_nand_secure_regions(struct nand_chip *chip)
int nr_elem, i, j;
nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64));
- if (!nr_elem)
- return 0;
+ if (nr_elem <= 0)
+ return nr_elem;
chip->nr_secure_regions = nr_elem / 2;
chip->secure_regions = kcalloc(chip->nr_secure_regions, sizeof(*chip->secure_regions),
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() 2021-06-17 13:37 [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() Dan Carpenter @ 2021-06-25 9:50 ` Manivannan Sadhasivam 2021-07-15 22:50 ` Miquel Raynal 1 sibling, 0 replies; 6+ messages in thread From: Manivannan Sadhasivam @ 2021-06-25 9:50 UTC (permalink / raw) To: Dan Carpenter Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Boris Brezillon, Tudor Ambarus, linux-mtd, kernel-janitors On Thu, Jun 17, 2021 at 04:37:25PM +0300, Dan Carpenter wrote: > Check for whether of_property_count_elems_of_size() returns a negative > error code. > > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> Thanks, Mani > --- > drivers/mtd/nand/raw/nand_base.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 57a583149cc0..cbba46432e39 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -5231,8 +5231,8 @@ static int of_get_nand_secure_regions(struct nand_chip *chip) > int nr_elem, i, j; > > nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64)); > - if (!nr_elem) > - return 0; > + if (nr_elem <= 0) > + return nr_elem; > > chip->nr_secure_regions = nr_elem / 2; > chip->secure_regions = kcalloc(chip->nr_secure_regions, sizeof(*chip->secure_regions), > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() 2021-06-17 13:37 [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() Dan Carpenter 2021-06-25 9:50 ` Manivannan Sadhasivam @ 2021-07-15 22:50 ` Miquel Raynal 2021-07-24 14:27 ` Martin Kaiser 1 sibling, 1 reply; 6+ messages in thread From: Miquel Raynal @ 2021-07-15 22:50 UTC (permalink / raw) To: Dan Carpenter, Miquel Raynal, Manivannan Sadhasivam Cc: Richard Weinberger, Vignesh Raghavendra, Boris Brezillon, Tudor Ambarus, linux-mtd, kernel-janitors On Thu, 2021-06-17 at 13:37:25 UTC, Dan Carpenter wrote: > Check for whether of_property_count_elems_of_size() returns a negative > error code. > > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks. Miquel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() 2021-07-15 22:50 ` Miquel Raynal @ 2021-07-24 14:27 ` Martin Kaiser 2021-07-24 16:03 ` Manivannan Sadhasivam 0 siblings, 1 reply; 6+ messages in thread From: Martin Kaiser @ 2021-07-24 14:27 UTC (permalink / raw) To: Miquel Raynal Cc: Dan Carpenter, Manivannan Sadhasivam, Richard Weinberger, Vignesh Raghavendra, Boris Brezillon, Tudor Ambarus, linux-mtd, kernel-janitors Hi all, Thus wrote Miquel Raynal (miquel.raynal@bootlin.com): > On Thu, 2021-06-17 at 13:37:25 UTC, Dan Carpenter wrote: > > Check for whether of_property_count_elems_of_size() returns a negative > > error code. > > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks. I'm running linux-next on an imx25 system with the following flash chip [ 1.997539] nand: device found, Manufacturer ID: 0x98, Chip ID: 0xaa [ 2.004134] nand: Toshiba NAND 256MiB 1,8V 8-bit [ 2.008917] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128 The system is using the drivers/mtd/nand/raw/mxc_nand.c driver. Since this commit appeared in linux-next, mxc_nand's probe function fails with -EINVAL, taking this path mxcnd_probe nand_scan nand_scan_with_ids nand_scan_tail of_get_nand_secure_regions nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64)); returns -EINVAL as there's no secure-regions property in my device tree. We should certainly handle negative error codes before we calculate chip->nr_secure_regions = nr_elem / 2 but a missing secure-regions property is a valid case and should not make the probe fail. If the property exists, but the device-tree entry is incorrect and of_property_count_elems_of_size returns -ENODATA, we might print a warning and ignore the entry. What do you think? Thanks, Martin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() 2021-07-24 14:27 ` Martin Kaiser @ 2021-07-24 16:03 ` Manivannan Sadhasivam 2021-07-26 7:58 ` Miquel Raynal 0 siblings, 1 reply; 6+ messages in thread From: Manivannan Sadhasivam @ 2021-07-24 16:03 UTC (permalink / raw) To: Martin Kaiser Cc: Miquel Raynal, Dan Carpenter, Richard Weinberger, Vignesh Raghavendra, Boris Brezillon, Tudor Ambarus, linux-mtd, kernel-janitors On Sat, Jul 24, 2021 at 04:27:30PM +0200, Martin Kaiser wrote: > Hi all, > > Thus wrote Miquel Raynal (miquel.raynal@bootlin.com): > > > On Thu, 2021-06-17 at 13:37:25 UTC, Dan Carpenter wrote: > > > Check for whether of_property_count_elems_of_size() returns a negative > > > error code. > > > > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> > > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks. > > I'm running linux-next on an imx25 system with the following flash chip > > [ 1.997539] nand: device found, Manufacturer ID: 0x98, Chip ID: 0xaa > [ 2.004134] nand: Toshiba NAND 256MiB 1,8V 8-bit > [ 2.008917] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128 > > The system is using the drivers/mtd/nand/raw/mxc_nand.c driver. > > Since this commit appeared in linux-next, mxc_nand's probe function fails > with -EINVAL, taking this path > > mxcnd_probe > nand_scan > nand_scan_with_ids > nand_scan_tail > of_get_nand_secure_regions > > nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64)); > returns -EINVAL as there's no secure-regions property in my device tree. > Doh! Sorry for missing this. > We should certainly handle negative error codes before we calculate > chip->nr_secure_regions = nr_elem / 2 > but a missing secure-regions property is a valid case and should not make > the probe fail. > Absolutely! > If the property exists, but the device-tree entry is incorrect > and of_property_count_elems_of_size returns -ENODATA, we might print a > warning and ignore the entry. > Hmm, I think it is best to error out in this case as the user has got DT wrong. > What do you think? > Since of_property_count_elems_of_size() returns -EINVAL if the length is not a multiple of sizeof(u64), we can't just ignore -EINVAL. So I think we can just check for the existence of the property before invoking of_get_nand_secure_regions(). Miquel, what do you think? Thanks, Mani > Thanks, > > Martin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() 2021-07-24 16:03 ` Manivannan Sadhasivam @ 2021-07-26 7:58 ` Miquel Raynal 0 siblings, 0 replies; 6+ messages in thread From: Miquel Raynal @ 2021-07-26 7:58 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Martin Kaiser, Dan Carpenter, Richard Weinberger, Vignesh Raghavendra, Boris Brezillon, Tudor Ambarus, linux-mtd, kernel-janitors Hi Mani, Manivannan Sadhasivam <mani@kernel.org> wrote on Sat, 24 Jul 2021 21:33:08 +0530: > On Sat, Jul 24, 2021 at 04:27:30PM +0200, Martin Kaiser wrote: > > Hi all, > > > > Thus wrote Miquel Raynal (miquel.raynal@bootlin.com): > > > > > On Thu, 2021-06-17 at 13:37:25 UTC, Dan Carpenter wrote: > > > > Check for whether of_property_count_elems_of_size() returns a negative > > > > error code. > > > > > > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory") > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> > > > > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks. > > > > I'm running linux-next on an imx25 system with the following flash chip > > > > [ 1.997539] nand: device found, Manufacturer ID: 0x98, Chip ID: 0xaa > > [ 2.004134] nand: Toshiba NAND 256MiB 1,8V 8-bit > > [ 2.008917] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128 > > > > The system is using the drivers/mtd/nand/raw/mxc_nand.c driver. > > > > Since this commit appeared in linux-next, mxc_nand's probe function fails > > with -EINVAL, taking this path > > > > mxcnd_probe > > nand_scan > > nand_scan_with_ids > > nand_scan_tail > > of_get_nand_secure_regions > > > > nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64)); > > returns -EINVAL as there's no secure-regions property in my device tree. > > > > Doh! Sorry for missing this. > > > We should certainly handle negative error codes before we calculate > > chip->nr_secure_regions = nr_elem / 2 > > but a missing secure-regions property is a valid case and should not make > > the probe fail. > > > > Absolutely! > > > If the property exists, but the device-tree entry is incorrect > > and of_property_count_elems_of_size returns -ENODATA, we might print a > > warning and ignore the entry. > > > > Hmm, I think it is best to error out in this case as the user has got DT wrong. > > > What do you think? > > > > Since of_property_count_elems_of_size() returns -EINVAL if the length is not > a multiple of sizeof(u64), we can't just ignore -EINVAL. > > So I think we can just check for the existence of the property before invoking > of_get_nand_secure_regions(). Miquel, what do you think? Yes please add this check and we should be good. Thanks, Miquèl ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-26 7:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-17 13:37 [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions() Dan Carpenter 2021-06-25 9:50 ` Manivannan Sadhasivam 2021-07-15 22:50 ` Miquel Raynal 2021-07-24 14:27 ` Martin Kaiser 2021-07-24 16:03 ` Manivannan Sadhasivam 2021-07-26 7:58 ` Miquel Raynal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).