Re: [PATCH 167/190] Revert "gdrom: fix a memory leak bug"

[Peter Rosin]

On 2021-04-27 15:01, Greg KH wrote:
> On Fri, Apr 23, 2021 at 08:20:30AM -0600, Jens Axboe wrote:
>> On 4/22/21 3:29 PM, Peter Rosin wrote:
>>>> This reverts commit 093c48213ee37c3c3ff1cf5ac1aa2a9d8bc66017.
>>>
>>> The reverted patch looks fishy.
>>>
>>> gc.cd_info is kzalloc:ed on probe. In case probe fails after this allocation, the
>>> memory is kfree:d but the variable is NOT zeroed out.
>>>
>>> AFAICT, the above leads to a double-free on exit by the added line.
>>>
>>> I believe gd.cd_info should be kfree:d on remove instead.
>>>
>>> However, might not gc.toc also be kfree:d twice for similar reasons?
>>>
>>> I could easily be mistaken.
>>
>> >From taking a quick look the other day, that's my conclusion too. I
>> don't think the patch is correct, but I don't think the surrounding code
>> is correct right now either.
> 
> Thanks for the review from both of you, I'll keep this commit in the
> tree.
Err, which commit is "this" and what tree are you keeping it in? I
think you mean that you are keeping the revert in your tree with
reverts, and not that you mean that we should keep the original
commit in Linus' tree.

In any case, I'd think that the original memory leak is somewhat
better than the introduced double-free and therefore the revert
should be done.

Cheers,
Peter