* [PATCH 0/2] mtd: nandsim: fix error handling
@ 2015-06-17 8:45 Sheng Yong
2015-06-17 8:45 ` [PATCH 1/2] mtd: nandsim: fix free of NULL pointer Sheng Yong
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Sheng Yong @ 2015-06-17 8:45 UTC (permalink / raw)
To: computersforpeace, dwmw2; +Cc: richard, linux-mtd
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] mtd: nandsim: fix free of NULL pointer
2015-06-17 8:45 [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
@ 2015-06-17 8:45 ` Sheng Yong
2015-06-17 8:48 ` Richard Weinberger
2015-06-17 8:45 ` [PATCH 2/2] mtd: nandsim: fix double free Sheng Yong
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Sheng Yong @ 2015-06-17 8:45 UTC (permalink / raw)
To: computersforpeace, dwmw2; +Cc: richard, linux-mtd
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 cb38f3d..33e4064 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -646,7 +646,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] mtd: nandsim: fix double free
2015-06-17 8:45 [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
2015-06-17 8:45 ` [PATCH 1/2] mtd: nandsim: fix free of NULL pointer Sheng Yong
@ 2015-06-17 8:45 ` Sheng Yong
2015-06-23 1:03 ` [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
2015-06-23 21:59 ` Brian Norris
3 siblings, 0 replies; 11+ messages in thread
From: Sheng Yong @ 2015-06-17 8:45 UTC (permalink / raw)
To: computersforpeace, dwmw2; +Cc: richard, linux-mtd
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 33e4064..9c655f1 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);
ns->partitions[i].offset = next_offset;
@@ -753,8 +751,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);
ns->partitions[i].offset = next_offset;
@@ -789,24 +786,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mtd: nandsim: fix free of NULL pointer
2015-06-17 8:45 ` [PATCH 1/2] mtd: nandsim: fix free of NULL pointer Sheng Yong
@ 2015-06-17 8:48 ` Richard Weinberger
2015-06-17 9:00 ` Richard Weinberger
2015-06-17 9:03 ` Sheng Yong
0 siblings, 2 replies; 11+ messages in thread
From: Richard Weinberger @ 2015-06-17 8:48 UTC (permalink / raw)
To: Sheng Yong, computersforpeace, dwmw2; +Cc: linux-mtd
Am 17.06.2015 um 10:45 schrieb 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 cb38f3d..33e4064 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -646,7 +646,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);
It is perfectly fine to free a NULL pointer.
Thanks,
//richard
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mtd: nandsim: fix free of NULL pointer
2015-06-17 8:48 ` Richard Weinberger
@ 2015-06-17 9:00 ` Richard Weinberger
2015-06-17 9:03 ` Sheng Yong
1 sibling, 0 replies; 11+ messages in thread
From: Richard Weinberger @ 2015-06-17 9:00 UTC (permalink / raw)
To: Sheng Yong, computersforpeace, dwmw2; +Cc: linux-mtd
Am 17.06.2015 um 10:48 schrieb Richard Weinberger:
> Am 17.06.2015 um 10:45 schrieb 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 cb38f3d..33e4064 100644
>> --- a/drivers/mtd/nand/nandsim.c
>> +++ b/drivers/mtd/nand/nandsim.c
>> @@ -646,7 +646,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);
>
> It is perfectly fine to free a NULL pointer.
Ignore that. /me needs more coffee. ;)
Thanks,
//richard
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mtd: nandsim: fix free of NULL pointer
2015-06-17 8:48 ` Richard Weinberger
2015-06-17 9:00 ` Richard Weinberger
@ 2015-06-17 9:03 ` Sheng Yong
2015-06-17 9:05 ` Richard Weinberger
1 sibling, 1 reply; 11+ messages in thread
From: Sheng Yong @ 2015-06-17 9:03 UTC (permalink / raw)
To: Richard Weinberger, computersforpeace, dwmw2; +Cc: linux-mtd
On 6/17/2015 4:48 PM, Richard Weinberger wrote:
> Am 17.06.2015 um 10:45 schrieb 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 cb38f3d..33e4064 100644
>> --- a/drivers/mtd/nand/nandsim.c
>> +++ b/drivers/mtd/nand/nandsim.c
>> @@ -646,7 +646,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);
>
> It is perfectly fine to free a NULL pointer.
OK, then maybe the double free is not a serious problem, besides we just
get a message "Trying to vfree() nonexistent vm area" or the like. But
kmem_cache_destroy() will access ns->nand_pages_slab, and ns->nand_pages_slab
is NULL. This will crash the kernel. :)
thanks,
Sheng
>
> Thanks,
> //richard
>
> .
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mtd: nandsim: fix free of NULL pointer
2015-06-17 9:03 ` Sheng Yong
@ 2015-06-17 9:05 ` Richard Weinberger
0 siblings, 0 replies; 11+ messages in thread
From: Richard Weinberger @ 2015-06-17 9:05 UTC (permalink / raw)
To: Sheng Yong, computersforpeace, dwmw2; +Cc: linux-mtd
Am 17.06.2015 um 11:03 schrieb Sheng Yong:
>
>
> On 6/17/2015 4:48 PM, Richard Weinberger wrote:
>> Am 17.06.2015 um 10:45 schrieb 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 cb38f3d..33e4064 100644
>>> --- a/drivers/mtd/nand/nandsim.c
>>> +++ b/drivers/mtd/nand/nandsim.c
>>> @@ -646,7 +646,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);
>>
>> It is perfectly fine to free a NULL pointer.
> OK, then maybe the double free is not a serious problem, besides we just
> get a message "Trying to vfree() nonexistent vm area" or the like. But
> kmem_cache_destroy() will access ns->nand_pages_slab, and ns->nand_pages_slab
> is NULL. This will crash the kernel. :)
Please see my other may, I was wrong. :)
Thanks,
//richard
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] mtd: nandsim: fix error handling
2015-06-17 8:45 [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
2015-06-17 8:45 ` [PATCH 1/2] mtd: nandsim: fix free of NULL pointer Sheng Yong
2015-06-17 8:45 ` [PATCH 2/2] mtd: nandsim: fix double free Sheng Yong
@ 2015-06-23 1:03 ` Sheng Yong
2015-06-23 6:26 ` Richard Weinberger
2015-06-23 21:59 ` Brian Norris
3 siblings, 1 reply; 11+ messages in thread
From: Sheng Yong @ 2015-06-23 1:03 UTC (permalink / raw)
To: computersforpeace, dwmw2; +Cc: richard, linux-mtd
Ping. Any comments are appreciated :)
thanks,
Sheng
On 6/17/2015 4:45 PM, Sheng Yong wrote:
> 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(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] mtd: nandsim: fix error handling
2015-06-23 1:03 ` [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
@ 2015-06-23 6:26 ` Richard Weinberger
2015-06-24 0:53 ` Sheng Yong
0 siblings, 1 reply; 11+ messages in thread
From: Richard Weinberger @ 2015-06-23 6:26 UTC (permalink / raw)
To: Sheng Yong, computersforpeace, dwmw2; +Cc: linux-mtd
Am 23.06.2015 um 03:03 schrieb Sheng Yong:
> Ping. Any comments are appreciated :)
>
> thanks,
> Sheng
>
> On 6/17/2015 4:45 PM, Sheng Yong wrote:
>> 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
Both patches look good to me.
Thanks,
//richard
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] mtd: nandsim: fix error handling
2015-06-17 8:45 [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
` (2 preceding siblings ...)
2015-06-23 1:03 ` [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
@ 2015-06-23 21:59 ` Brian Norris
3 siblings, 0 replies; 11+ messages in thread
From: Brian Norris @ 2015-06-23 21:59 UTC (permalink / raw)
To: Sheng Yong; +Cc: linux-mtd, dwmw2, richard
On Wed, Jun 17, 2015 at 08:45:11AM +0000, Sheng Yong wrote:
> 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(-)
This series doesn't apply cleanly to -next. Please rebase on either
linux-next.git or l2-mtd.git:
http://www.linux-mtd.infradead.org/source.html
Thanks,
Brian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] mtd: nandsim: fix error handling
2015-06-23 6:26 ` Richard Weinberger
@ 2015-06-24 0:53 ` Sheng Yong
0 siblings, 0 replies; 11+ messages in thread
From: Sheng Yong @ 2015-06-24 0:53 UTC (permalink / raw)
To: Richard Weinberger, computersforpeace, dwmw2; +Cc: linux-mtd
On 6/23/2015 2:26 PM, Richard Weinberger wrote:
> Am 23.06.2015 um 03:03 schrieb Sheng Yong:
>> Ping. Any comments are appreciated :)
>>
>> thanks,
>> Sheng
>>
>> On 6/17/2015 4:45 PM, Sheng Yong wrote:
>>> 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
>
> Both patches look good to me.
Hi, Richard
Can I add your reviewed-by?
thanks,
Sheng
>
> Thanks,
> //richard
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-06-24 0:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 8:45 [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
2015-06-17 8:45 ` [PATCH 1/2] mtd: nandsim: fix free of NULL pointer Sheng Yong
2015-06-17 8:48 ` Richard Weinberger
2015-06-17 9:00 ` Richard Weinberger
2015-06-17 9:03 ` Sheng Yong
2015-06-17 9:05 ` Richard Weinberger
2015-06-17 8:45 ` [PATCH 2/2] mtd: nandsim: fix double free Sheng Yong
2015-06-23 1:03 ` [PATCH 0/2] mtd: nandsim: fix error handling Sheng Yong
2015-06-23 6:26 ` Richard Weinberger
2015-06-24 0:53 ` Sheng Yong
2015-06-23 21:59 ` Brian Norris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.