Re: [PATCH] mtd: cfi_cmdset_0002: don't free cfi->cfiq in error path of cfi_amdstd_setup()

[Vignesh Raghavendra <vigneshr@xxxxxx>]

Hi Hou,

On 16/10/19 1:17 AM, Richard Weinberger wrote:
> On Tue, Oct 8, 2019 at 4:29 AM Hou Tao <houtao1@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>>  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@xxxxxx>
> 

-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/