Skip to contentSkip to conversation

mtd: rawnand: Add a check in of_get_nand_secure_regions()

PATCH Applied Dan Carpenter 4 participants 6 messages Jun 17, 2021 – Jul 26, 2021

Subsystems (3)
Kernel janitors Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 mtd: rawnand: Add a check in of_get_nand_secure_regions()

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 <[email protected]>
---
 drivers/mtd/nand/raw/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/nand_base.c +2 -2
index 57a583149cc0..cbba46432e39 100644
@@ -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),
Signature · 1 line
2.30.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Dan Carpenter

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 <[email protected]>
Reviewed-by Manivannan Sadhasivam <[email protected]>

Thanks,
Mani

Quoted 22 lines
---
 drivers/mtd/nand/raw/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/nand_base.c +2 -2
index 57a583149cc0..cbba46432e39 100644
@@ -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),
Signature · 1 line
2.30.2
#3 lore raw

Hi all,

Thus wrote Miquel Raynal ([email protected]):

Quoted 9 lines · Dan Carpenter
Quoted 6 lines

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 <[email protected]>
Reviewed-by Manivannan Sadhasivam <[email protected]>

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

#4 lore raw
Quoted 25 lines · Martin Kaiser

Hi all,

Thus wrote Miquel Raynal ([email protected]):

Quoted 9 lines · Dan Carpenter
Quoted 6 lines

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 <[email protected]>
Reviewed-by Manivannan Sadhasivam <[email protected]>

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

Quoted 9 lines

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.

Quoted 5 lines

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!

Quoted 4 lines

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.

Quoted 2 lines

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

Quoted 3 lines

Thanks,

Martin

maintainer
#5 lore raw

Hi Mani,

Manivannan Sadhasivam <[email protected]> wrote on Sat, 24 Jul 2021 21:33:08 +0530:

Quoted 53 lines · Martin Kaiser
Quoted 25 lines

Hi all,

Thus wrote Miquel Raynal ([email protected]):

Quoted 9 lines · Dan Carpenter
Quoted 6 lines

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 <[email protected]>
Reviewed-by Manivannan Sadhasivam <[email protected]>

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

Quoted 9 lines

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.

Quoted 5 lines

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!

Quoted 4 lines

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.

Quoted 8 lines
Quoted 2 lines

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