[PATCH] mtd: rawnand: s3c2410: Error out when ->nrsets < 0 or ->sets == NULL

[PATCH] mtd: rawnand: s3c2410: Error out when ->nrsets < 0 or ->sets == NULL

[Boris Brezillon]

All of the code in the probe path assumes ->sets != NULL and
->nrsets > 0. Error out if that's not the case to avoid dereferencing a
NULL pointer.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Hello,

I intentionally did not Cc stable because nobody complained so far.
Also didn't add a Fixes tag because it's hard to tell when people
started to dereferencing ->sets without checking its value.

Regards,

Boris
---
 drivers/mtd/nand/raw/s3c2410.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c
index 10d81f367d26..5a4a68790653 100644
--- a/drivers/mtd/nand/raw/s3c2410.c
+++ b/drivers/mtd/nand/raw/s3c2410.c
@@ -1134,8 +1134,13 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
 
 	dev_dbg(&pdev->dev, "mapped registers at %p\n", info->regs);
 
-	sets = (plat != NULL) ? plat->sets : NULL;
-	nr_sets = (plat != NULL) ? plat->nr_sets : 1;
+	if (!plat->sets || plat->nr_sets < 1) {
+		err = -EINVAL;
+		goto exit_error;
+	}
+
+	sets = plat->sets;
+	nr_sets = plat->nr_sets;
 
 	info->mtd_count = nr_sets;
 
@@ -1152,7 +1157,7 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
 
 	nmtd = info->mtds;
 
-	for (setno = 0; setno < nr_sets; setno++, nmtd++) {
+	for (setno = 0; setno < nr_sets; setno++, nmtd++, sets++) {
 		struct mtd_info *mtd = nand_to_mtd(&nmtd->chip);
 
 		pr_debug("initialising set %d (%p, info %p)\n",
@@ -1174,9 +1179,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
 			goto exit_error;
 
 		s3c2410_nand_add_partition(info, nmtd, sets);
-
-		if (sets != NULL)
-			sets++;
 	}
 
 	/* initialise the hardware */
-- 
2.14.1

Re: [PATCH] mtd: rawnand: s3c2410: Error out when ->nrsets < 0 or ->sets == NULL

[Dan Carpenter]

On Thu, Jul 19, 2018 at 11:41:37AM +0200, Boris Brezillon wrote:
> @@ -1152,7 +1157,7 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>  
>  	nmtd = info->mtds;
>  
> -	for (setno = 0; setno < nr_sets; setno++, nmtd++) {
> +	for (setno = 0; setno < nr_sets; setno++, nmtd++, sets++) {
>  		struct mtd_info *mtd = nand_to_mtd(&nmtd->chip);
>  
>  		pr_debug("initialising set %d (%p, info %p)\n",
> @@ -1174,9 +1179,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)

Can you also remove the NULL check from int the middle of the loop:

		err = nand_scan_ident(mtd, (sets) ? sets->nr_chips : 1, NULL);
                                            ^^^^

>  			goto exit_error;
>  
>  		s3c2410_nand_add_partition(info, nmtd, sets);
> -
> -		if (sets != NULL)
> -			sets++;
>  	}

regards,
dan carpenter

Re: [PATCH] mtd: rawnand: s3c2410: Error out when ->nrsets < 0 or ->sets == NULL

[Boris Brezillon]

On Thu, 19 Jul 2018 13:30:47 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Thu, Jul 19, 2018 at 11:41:37AM +0200, Boris Brezillon wrote:
> > @@ -1152,7 +1157,7 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  
> >  	nmtd = info->mtds;
> >  
> > -	for (setno = 0; setno < nr_sets; setno++, nmtd++) {
> > +	for (setno = 0; setno < nr_sets; setno++, nmtd++, sets++) {
> >  		struct mtd_info *mtd = nand_to_mtd(&nmtd->chip);
> >  
> >  		pr_debug("initialising set %d (%p, info %p)\n",
> > @@ -1174,9 +1179,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)  
> 
> Can you also remove the NULL check from int the middle of the loop:
> 
> 		err = nand_scan_ident(mtd, (sets) ? sets->nr_chips : 1, NULL);

Sure.

>                                             ^^^^
> 
> >  			goto exit_error;
> >  
> >  		s3c2410_nand_add_partition(info, nmtd, sets);
> > -
> > -		if (sets != NULL)
> > -			sets++;
> >  	}  
> 
> regards,
> dan carpenter
>