linux-kernel - Re: [PATCH V1] nvmem: core: fix memory abort in cleanup path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7abb79c6-b497-98b3-45ff-44d751f1c781@nvidia.com>
Date:   Thu, 2 Jan 2020 10:51:24 -0800
From:   Bitan Biswas <bbiswas@...dia.com>
To:     Thierry Reding <treding@...dia.com>
CC:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh@...nel.org>, <linux-kernel@...r.kernel.org>,
        Jonathan Hunter <jonathanh@...dia.com>
Subject: Re: [PATCH V1] nvmem: core: fix memory abort in cleanup path


Hi Thierry,

On 1/2/20 4:44 AM, Thierry Reding wrote:
> On Sat, Dec 28, 2019 at 08:02:42PM -0800, Bitan Biswas wrote:
>> nvmem_cell_info_to_nvmem_cell implementation has static
>> allocation of name. nvmem_add_cells_from_of() call may
>> return error and kfree name results in memory abort. Use
>> kasprintf() instead of assigning pointer and prevent kfree crash.
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 9f1ee9c..0fc66e1 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -110,7 +110,7 @@ static int nvmem_cell_info_to_nvmem_cell(struct nvmem_device *nvmem,
>>   	cell->nvmem = nvmem;
>>   	cell->offset = info->offset;
>>   	cell->bytes = info->bytes;
>> -	cell->name = info->name;
>> +	cell->name = kasprintf(GFP_KERNEL, "%s", info->name);

> 
> kstrdup() seems more appropriate here.
Thanks. I shall update the patch as suggested.

> 
> A slightly more efficient way to do this would be to use a combination
> of kstrdup_const() and kfree_const(), which would allow read-only
> strings to be replicated by simple assignment rather than duplication.
> Note that in that case you'd need to carefully replace all kfree() calls
> on cell->name by a kfree_const() to ensure they do the right thing.
kfree(cell->name) is also called for allocations in function 
nvmem_add_cells_from_of() through below call
kasprintf(GFP_KERNEL, "%pOFn", child);

My understanding is kfree_const may not work for above allocation.



-regards,
  Bitan