[PATCH V2 0/2] mtd: nandsim: fix error handling

[PATCH V2 0/2] mtd: nandsim: fix error handling

[Sheng Yong]

V2:
Resend the patches against l2-mtd/master.

V1:
These 2 patches fix error handling when nandsim initialization fails.

In alloc_device(), if creating slab memory fails, free_device() will try
to destroy the slab memory without checking if it exists.  PATCH 1 fixes
it.

If something goes wrong in init_nandsim(), it calls free_device() before
returning. However, the caller of init_nandsim() - ns_init_module() - also
does the cleanup by calling free_nandsim(). This causes double free. PATCH
2 fixes it.

Thanks,
Sheng

Sheng Yong (2):
  mtd: nandsim: fix free of NULL pointer
  mtd: nandsim: fix double free

 drivers/mtd/nand/nandsim.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

-- 
1.8.3.4

[PATCH V2 1/2] mtd: nandsim: fix free of NULL pointer

[Sheng Yong]

If allocating ns->nand_pages_slab fails, do not try to destroy it when
cleaning up nandsim resources.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 drivers/mtd/nand/nandsim.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index 52c0c1a..6a74f62 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -649,7 +649,8 @@ static void free_device(struct nandsim *ns)
 				kmem_cache_free(ns->nand_pages_slab,
 						ns->pages[i].byte);
 		}
-		kmem_cache_destroy(ns->nand_pages_slab);
+		if (ns->nand_pages_slab)
+			kmem_cache_destroy(ns->nand_pages_slab);
 		vfree(ns->pages);
 	}
 }
-- 
1.8.3.4

[PATCH V2 2/2] mtd: nandsim: fix double free

[Sheng Yong]

Do not call free_device() in init_nandsim, the caller - ns_init_module -
will take care of that if something goes wrong.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 drivers/mtd/nand/nandsim.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index 6a74f62..92afde9 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -730,8 +730,7 @@ static int init_nandsim(struct mtd_info *mtd)
 	/* Fill the partition_info structure */
 	if (parts_num > ARRAY_SIZE(ns->partitions)) {
 		NS_ERR("too many partitions.\n");
-		ret = -EINVAL;
-		goto error;
+		return -EINVAL;
 	}
 	remains = ns->geom.totsz;
 	next_offset = 0;
@@ -740,8 +739,7 @@ static int init_nandsim(struct mtd_info *mtd)
 
 		if (!part_sz || part_sz > remains) {
 			NS_ERR("bad partition size.\n");
-			ret = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 		ns->partitions[i].name   = get_partition_name(i);
 		if (!ns->partitions[i].name) {
@@ -758,8 +756,7 @@ static int init_nandsim(struct mtd_info *mtd)
 	if (remains) {
 		if (parts_num + 1 > ARRAY_SIZE(ns->partitions)) {
 			NS_ERR("too many partitions.\n");
-			ret = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 		ns->partitions[i].name   = get_partition_name(i);
 		if (!ns->partitions[i].name) {
@@ -793,24 +790,18 @@ static int init_nandsim(struct mtd_info *mtd)
 	printk("options: %#x\n",                ns->options);
 
 	if ((ret = alloc_device(ns)) != 0)
-		goto error;
+		return ret;
 
 	/* Allocate / initialize the internal buffer */
 	ns->buf.byte = kmalloc(ns->geom.pgszoob, GFP_KERNEL);
 	if (!ns->buf.byte) {
 		NS_ERR("init_nandsim: unable to allocate %u bytes for the internal buffer\n",
 			ns->geom.pgszoob);
-		ret = -ENOMEM;
-		goto error;
+		return -ENOMEM;
 	}
 	memset(ns->buf.byte, 0xFF, ns->geom.pgszoob);
 
 	return 0;
-
-error:
-	free_device(ns);
-
-	return ret;
 }
 
 /*
-- 
1.8.3.4

Re: [PATCH V2 0/2] mtd: nandsim: fix error handling

[Brian Norris]

On Thu, Jun 25, 2015 at 12:46:47AM +0000, Sheng Yong wrote:
> V2:
> Resend the patches against l2-mtd/master.

  CC      drivers/mtd/nand/nandsim.o
drivers/mtd/nand/nandsim.c: In function ‘init_nandsim’:
drivers/mtd/nand/nandsim.c:765:4: error: label ‘error’ used but not defined
    goto error;
    ^

The point wasn't to rebase blindly. I got this far already. But you should be
testing (at a minimum, compile testing) your changes!

Brian

Re: [PATCH V2 0/2] mtd: nandsim: fix error handling

[Sheng Yong]

On 6/25/2015 9:24 AM, Brian Norris wrote:
> On Thu, Jun 25, 2015 at 12:46:47AM +0000, Sheng Yong wrote:
>> V2:
>> Resend the patches against l2-mtd/master.
> 
>   CC      drivers/mtd/nand/nandsim.o
> drivers/mtd/nand/nandsim.c: In function ‘init_nandsim’:
> drivers/mtd/nand/nandsim.c:765:4: error: label ‘error’ used but not defined
>     goto error;
>     ^
> 
> The point wasn't to rebase blindly. I got this far already. But you should be
> testing (at a minimum, compile testing) your changes!

Sorry, my bad. I'll keep this in mind. And I'll resend the patch later.

thanks,
Sheng
> 
> Brian
> 
>