mtd: nand: Fix various memory leaks in core
authorBoris Brezillon <boris.brezillon@free-electrons.com>
Fri, 2 Jun 2017 10:18:24 +0000 (12:18 +0200)
committerBoris Brezillon <boris.brezillon@free-electrons.com>
Wed, 23 Aug 2017 14:49:13 +0000 (16:49 +0200)
The nand_scan_ident() function is not expected to allocate resources,
and people are usually not calling nand_cleanup() if something fails
between nand_scan_ident() and nand_scan_tail().

Move all functions that may allocate resource to the nand_scan_tail()
path to prevent such resource leaks.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
drivers/mtd/nand/nand_base.c

index 54d6194fb180c4a4ea16ec8d9db53737dcdfa531..abcb15fba484c807a7f641dcc9ce0e618470153e 100644 (file)
@@ -3856,7 +3856,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
        const struct nand_manufacturer *manufacturer;
        struct mtd_info *mtd = nand_to_mtd(chip);
        int busw;
-       int i, ret;
+       int i;
        u8 *id_data = chip->id.data;
        u8 maf_id, dev_id;
 
@@ -3997,10 +3997,6 @@ ident_done:
        if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
                chip->cmdfunc = nand_command_lp;
 
-       ret = nand_manufacturer_init(chip);
-       if (ret)
-               return ret;
-
        pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
                maf_id, dev_id);
 
@@ -4208,23 +4204,6 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
                return ret;
        }
 
-       /* Initialize the ->data_interface field. */
-       ret = nand_init_data_interface(chip);
-       if (ret)
-               goto err_nand_init;
-
-       /*
-        * Setup the data interface correctly on the chip and controller side.
-        * This explicit call to nand_setup_data_interface() is only required
-        * for the first die, because nand_reset() has been called before
-        * ->data_interface and ->default_onfi_timing_mode were set.
-        * For the other dies, nand_reset() will automatically switch to the
-        * best mode for us.
-        */
-       ret = nand_setup_data_interface(chip, 0);
-       if (ret)
-               goto err_nand_init;
-
        nand_maf_id = chip->id.data[0];
        nand_dev_id = chip->id.data[1];
 
@@ -4254,12 +4233,6 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
        mtd->size = i * chip->chipsize;
 
        return 0;
-
-err_nand_init:
-       /* Free manufacturer priv data. */
-       nand_manufacturer_cleanup(chip);
-
-       return ret;
 }
 EXPORT_SYMBOL(nand_scan_ident);
 
@@ -4646,55 +4619,60 @@ int nand_scan_tail(struct mtd_info *mtd)
        struct nand_chip *chip = mtd_to_nand(mtd);
        struct nand_ecc_ctrl *ecc = &chip->ecc;
        struct nand_buffers *nbuf = NULL;
-       int ret;
+       int ret, i;
 
        /* New bad blocks should be marked in OOB, flash-based BBT, or both */
        if (WARN_ON((chip->bbt_options & NAND_BBT_NO_OOB_BBM) &&
                   !(chip->bbt_options & NAND_BBT_USE_FLASH))) {
-               ret = -EINVAL;
-               goto err_ident;
+               return -EINVAL;
        }
 
        if (invalid_ecc_page_accessors(chip)) {
                pr_err("Invalid ECC page accessors setup\n");
-               ret = -EINVAL;
-               goto err_ident;
+               return -EINVAL;
        }
 
        if (!(chip->options & NAND_OWN_BUFFERS)) {
                nbuf = kzalloc(sizeof(*nbuf), GFP_KERNEL);
-               if (!nbuf) {
-                       ret = -ENOMEM;
-                       goto err_ident;
-               }
+               if (!nbuf)
+                       return -ENOMEM;
 
                nbuf->ecccalc = kmalloc(mtd->oobsize, GFP_KERNEL);
                if (!nbuf->ecccalc) {
                        ret = -ENOMEM;
-                       goto err_free;
+                       goto err_free_nbuf;
                }
 
                nbuf->ecccode = kmalloc(mtd->oobsize, GFP_KERNEL);
                if (!nbuf->ecccode) {
                        ret = -ENOMEM;
-                       goto err_free;
+                       goto err_free_nbuf;
                }
 
                nbuf->databuf = kmalloc(mtd->writesize + mtd->oobsize,
                                        GFP_KERNEL);
                if (!nbuf->databuf) {
                        ret = -ENOMEM;
-                       goto err_free;
+                       goto err_free_nbuf;
                }
 
                chip->buffers = nbuf;
-       } else {
-               if (!chip->buffers) {
-                       ret = -ENOMEM;
-                       goto err_ident;
-               }
+       } else if (!chip->buffers) {
+               return -ENOMEM;
        }
 
+       /*
+        * FIXME: some NAND manufacturer drivers expect the first die to be
+        * selected when manufacturer->init() is called. They should be fixed
+        * to explictly select the relevant die when interacting with the NAND
+        * chip.
+        */
+       chip->select_chip(mtd, 0);
+       ret = nand_manufacturer_init(chip);
+       chip->select_chip(mtd, -1);
+       if (ret)
+               goto err_free_nbuf;
+
        /* Set the internal oob buffer location, just after the page data */
        chip->oob_poi = chip->buffers->databuf + mtd->writesize;
 
@@ -4716,7 +4694,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                        WARN(1, "No oob scheme defined for oobsize %d\n",
                                mtd->oobsize);
                        ret = -EINVAL;
-                       goto err_free;
+                       goto err_nand_manuf_cleanup;
                }
        }
 
@@ -4731,7 +4709,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
                        WARN(1, "No ECC functions supplied; hardware ECC not possible\n");
                        ret = -EINVAL;
-                       goto err_free;
+                       goto err_nand_manuf_cleanup;
                }
                if (!ecc->read_page)
                        ecc->read_page = nand_read_page_hwecc_oob_first;
@@ -4763,7 +4741,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                     ecc->write_page == nand_write_page_hwecc)) {
                        WARN(1, "No ECC functions supplied; hardware ECC not possible\n");
                        ret = -EINVAL;
-                       goto err_free;
+                       goto err_nand_manuf_cleanup;
                }
                /* Use standard syndrome read/write page function? */
                if (!ecc->read_page)
@@ -4783,7 +4761,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                        if (!ecc->strength) {
                                WARN(1, "Driver must set ecc.strength when using hardware ECC\n");
                                ret = -EINVAL;
-                               goto err_free;
+                               goto err_nand_manuf_cleanup;
                        }
                        break;
                }
@@ -4796,7 +4774,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                ret = nand_set_ecc_soft_ops(mtd);
                if (ret) {
                        ret = -EINVAL;
-                       goto err_free;
+                       goto err_nand_manuf_cleanup;
                }
                break;
 
@@ -4804,7 +4782,7 @@ int nand_scan_tail(struct mtd_info *mtd)
                if (!ecc->read_page || !ecc->write_page) {
                        WARN(1, "No ECC functions supplied; on-die ECC not possible\n");
                        ret = -EINVAL;
-                       goto err_free;
+                       goto err_nand_manuf_cleanup;
                }
                if (!ecc->read_oob)
                        ecc->read_oob = nand_read_oob_std;
@@ -4828,7 +4806,7 @@ int nand_scan_tail(struct mtd_info *mtd)
        default:
                WARN(1, "Invalid NAND_ECC_MODE %d\n", ecc->mode);
                ret = -EINVAL;
-               goto err_free;
+               goto err_nand_manuf_cleanup;
        }
 
        /* For many systems, the standard OOB write also works for raw */
@@ -4849,13 +4827,13 @@ int nand_scan_tail(struct mtd_info *mtd)
        if (ecc->steps * ecc->size != mtd->writesize) {
                WARN(1, "Invalid ECC parameters\n");
                ret = -EINVAL;
-               goto err_free;
+               goto err_nand_manuf_cleanup;
        }
        ecc->total = ecc->steps * ecc->bytes;
        if (ecc->total > mtd->oobsize) {
                WARN(1, "Total number of ECC bytes exceeded oobsize\n");
                ret = -EINVAL;
-               goto err_free;
+               goto err_nand_manuf_cleanup;
        }
 
        /*
@@ -4937,6 +4915,21 @@ int nand_scan_tail(struct mtd_info *mtd)
        if (!mtd->bitflip_threshold)
                mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
 
+       /* Initialize the ->data_interface field. */
+       ret = nand_init_data_interface(chip);
+       if (ret)
+               goto err_nand_manuf_cleanup;
+
+       /* Enter fastest possible mode on all dies. */
+       for (i = 0; i < chip->numchips; i++) {
+               chip->select_chip(mtd, i);
+               ret = nand_setup_data_interface(chip, i);
+               chip->select_chip(mtd, -1);
+
+               if (ret)
+                       goto err_nand_data_iface_cleanup;
+       }
+
        /* Check, if we should skip the bad block table scan */
        if (chip->options & NAND_SKIP_BBTSCAN)
                return 0;
@@ -4944,10 +4937,17 @@ int nand_scan_tail(struct mtd_info *mtd)
        /* Build bad block table */
        ret = chip->scan_bbt(mtd);
        if (ret)
-               goto err_free;
+               goto err_nand_data_iface_cleanup;
+
        return 0;
 
-err_free:
+err_nand_data_iface_cleanup:
+       nand_release_data_interface(chip);
+
+err_nand_manuf_cleanup:
+       nand_manufacturer_cleanup(chip);
+
+err_free_nbuf:
        if (nbuf) {
                kfree(nbuf->databuf);
                kfree(nbuf->ecccode);
@@ -4955,12 +4955,6 @@ err_free:
                kfree(nbuf);
        }
 
-err_ident:
-       /* Clean up nand_scan_ident(). */
-
-       /* Free manufacturer priv data. */
-       nand_manufacturer_cleanup(chip);
-
        return ret;
 }
 EXPORT_SYMBOL(nand_scan_tail);