* [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()
@ 2019-10-08 2:36 Hou Tao
2019-10-14 12:23 ` Hou Tao
2019-10-15 19:47 ` Richard Weinberger
0 siblings, 2 replies; 5+ messages in thread
From: Hou Tao @ 2019-10-08 2:36 UTC (permalink / raw)
To: linux-mtd
Cc: vigneshr, richard, marek.vasut, miquel.raynal, computersforpeace, dwmw2
Else there may be a double-free problem, because cfi->cfiq will
be freed by mtd_do_chip_probe() if both the two invocations of
check_cmd_set() return failure.
Also check cfi_intelext_setup() & cfi_staa_setup() to find out
that cfi->cfiq is not freed as well in these functions.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
drivers/mtd/chips/cfi_cmdset_0002.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index cf8c8be40a9c..7eaa4b523197 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -785,7 +785,6 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
kfree(mtd->eraseregions);
kfree(mtd);
kfree(cfi->cmdset_priv);
- kfree(cfi->cfiq);
return NULL;
}
--
2.22.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()
2019-10-08 2:36 [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup() Hou Tao
@ 2019-10-14 12:23 ` Hou Tao
2019-10-15 19:47 ` Richard Weinberger
1 sibling, 0 replies; 5+ messages in thread
From: Hou Tao @ 2019-10-14 12:23 UTC (permalink / raw)
To: linux-mtd, vigneshr, richard, marek.vasut, miquel.raynal,
computersforpeace
Cc: dwmw2
ping ?
On 2019/10/8 10:36, Hou Tao wrote:
> Else there may be a double-free problem, because cfi->cfiq will
> be freed by mtd_do_chip_probe() if both the two invocations of
> check_cmd_set() return failure.
>
> Also check cfi_intelext_setup() & cfi_staa_setup() to find out
> that cfi->cfiq is not freed as well in these functions.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> drivers/mtd/chips/cfi_cmdset_0002.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index cf8c8be40a9c..7eaa4b523197 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -785,7 +785,6 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
> kfree(mtd->eraseregions);
> kfree(mtd);
> kfree(cfi->cmdset_priv);
> - kfree(cfi->cfiq);
> return NULL;
> }
>
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()
2019-10-08 2:36 [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup() Hou Tao
2019-10-14 12:23 ` Hou Tao
@ 2019-10-15 19:47 ` Richard Weinberger
2019-10-16 5:38 ` Vignesh Raghavendra
1 sibling, 1 reply; 5+ messages in thread
From: Richard Weinberger @ 2019-10-15 19:47 UTC (permalink / raw)
To: Hou Tao
Cc: Vignesh Raghavendra, Richard Weinberger, Marek Vasut, linux-mtd,
Miquel Raynal, Brian Norris, David Woodhouse
On Tue, Oct 8, 2019 at 4:29 AM Hou Tao <houtao1@huawei.com> wrote:
>
> Else there may be a double-free problem, because cfi->cfiq will
> be freed by mtd_do_chip_probe() if both the two invocations of
> check_cmd_set() return failure.
>
> Also check cfi_intelext_setup() & cfi_staa_setup() to find out
> that cfi->cfiq is not freed as well in these functions.
This sentence does not make sense to me.
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> drivers/mtd/chips/cfi_cmdset_0002.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index cf8c8be40a9c..7eaa4b523197 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -785,7 +785,6 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
> kfree(mtd->eraseregions);
> kfree(mtd);
> kfree(cfi->cmdset_priv);
> - kfree(cfi->cfiq);
> return NULL;
> }
Other than that,
Reviewed-by: Richard Weinberger <richard@nod.at>
--
Thanks,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()
2019-10-15 19:47 ` Richard Weinberger
@ 2019-10-16 5:38 ` Vignesh Raghavendra
2019-10-16 6:43 ` Hou Tao
0 siblings, 1 reply; 5+ messages in thread
From: Vignesh Raghavendra @ 2019-10-16 5:38 UTC (permalink / raw)
To: Hou Tao
Cc: Richard Weinberger, Richard Weinberger, Marek Vasut, linux-mtd,
Miquel Raynal, Brian Norris, David Woodhouse
Hi Hou,
On 16/10/19 1:17 AM, Richard Weinberger wrote:
> On Tue, Oct 8, 2019 at 4:29 AM Hou Tao <houtao1@huawei.com> wrote:
>>
>> Else there may be a double-free problem, because cfi->cfiq will
>> be freed by mtd_do_chip_probe() if both the two invocations of
>> check_cmd_set() return failure.
>>
>> Also check cfi_intelext_setup() & cfi_staa_setup() to find out
>> that cfi->cfiq is not freed as well in these functions.
>
I guess you are trying to imply cfi_amdstd_setup() equivalents in
cfi_cmdset_0001.c (cfi_intelext_setup()) and cfi_cmdset_0020.c
(cfi_staa_setup()) dont't call kfree(cfi->cfiq). So cfi_amdstd_setup()
should not be freeing that pointer either?
This reference to other drivers in commit msg is quite confusing. My
suggestion would be to drop above line.
Let me know if that sound good. I will drop the it while applying.
> This sentence does not make sense to me.
>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> drivers/mtd/chips/cfi_cmdset_0002.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>> index cf8c8be40a9c..7eaa4b523197 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -785,7 +785,6 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
>> kfree(mtd->eraseregions);
>> kfree(mtd);
>> kfree(cfi->cmdset_priv);
>> - kfree(cfi->cfiq);
>> return NULL;
>> }
>
> Other than that,
> Reviewed-by: Richard Weinberger <richard@nod.at>
>
--
Regards
Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()
2019-10-16 5:38 ` Vignesh Raghavendra
@ 2019-10-16 6:43 ` Hou Tao
0 siblings, 0 replies; 5+ messages in thread
From: Hou Tao @ 2019-10-16 6:43 UTC (permalink / raw)
To: Vignesh Raghavendra
Cc: Richard Weinberger, Richard Weinberger, Marek Vasut, linux-mtd,
Miquel Raynal, Brian Norris, David Woodhouse
Hi,
On 2019/10/16 13:38, Vignesh Raghavendra wrote:
> Hi Hou,
>
> On 16/10/19 1:17 AM, Richard Weinberger wrote:
>> On Tue, Oct 8, 2019 at 4:29 AM Hou Tao <houtao1@huawei.com> wrote:
>>>
>>> Else there may be a double-free problem, because cfi->cfiq will
>>> be freed by mtd_do_chip_probe() if both the two invocations of
>>> check_cmd_set() return failure.
>>>
>>> Also check cfi_intelext_setup() & cfi_staa_setup() to find out
>>> that cfi->cfiq is not freed as well in these functions.
>>
>
> I guess you are trying to imply cfi_amdstd_setup() equivalents in
> cfi_cmdset_0001.c (cfi_intelext_setup()) and cfi_cmdset_0020.c
> (cfi_staa_setup()) dont't call kfree(cfi->cfiq). So cfi_amdstd_setup()
> should not be freeing that pointer either?
>
No. The lines are used to illustrate the same mistake doesn't happen
in cfi_cmdset_0001 & 0020 instead of cfi_cmdstd_setup() needs to do
the same thing as others cmdset.
> This reference to other drivers in commit msg is quite confusing. My
> suggestion would be to drop above line.
>
It's OK for me if you drop these line.
> Let me know if that sound good. I will drop the it while applying.
>
>
Regards,
Tao
>> This sentence does not make sense to me.
>>
>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>> ---
>>> drivers/mtd/chips/cfi_cmdset_0002.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>>> index cf8c8be40a9c..7eaa4b523197 100644
>>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>>> @@ -785,7 +785,6 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
>>> kfree(mtd->eraseregions);
>>> kfree(mtd);
>>> kfree(cfi->cmdset_priv);
>>> - kfree(cfi->cfiq);
>>> return NULL;
>>> }
>>
>> Other than that,
>> Reviewed-by: Richard Weinberger <richard@nod.at>
>>
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-16 6:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08 2:36 [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup() Hou Tao
2019-10-14 12:23 ` Hou Tao
2019-10-15 19:47 ` Richard Weinberger
2019-10-16 5:38 ` Vignesh Raghavendra
2019-10-16 6:43 ` Hou Tao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).