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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

Boris Brezillon <boris.brezillon@bootlin.com> wrote on Thu, 19 Jul 2018
22:53:50 +0200:

> 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
> 
> Changes in v2:
> - Kill an unneeded 'sets != NULL' test (reported by Dan)
> ---
>  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 */

Applied to nand/next.

Thanks,
Miquèl