Re: [PATCH] block: fix memleak of bio integrity data

[Jens Axboe <axboe@kernel.dk>]

On 12/5/19 11:30 AM, Justin Tee wrote:
> On 12/5/2019 12:09 AM, Ming Lei wrote:
>> On Thu, Dec 05, 2019 at 03:49:32PM +0800, Ming Lei wrote:
>>> On Wed, Dec 04, 2019 at 09:41:24PM -0800, Justin Tee wrote:
>>>> Hi Ming,
>>>>
>>>> I understand the patch, but I have a concern.
>>>>
>>>> Is it possible to come across a double-free?  from bio_endio ->
>>>> bio_integrity_endio -> __bio_integrity_endio -> bio_integrity_free;  And
>>>> then, resuming in bio_endio -> bio_uninit -> bio_integrity_free;.  Maybe
>>>> it's even possible queue_work  bio_integrity_verify_fn was scheduled and
>>>> called bio_integrity_free from there as well.  So, should also remove
>>>> bio_integrity_free from bio_integrity_verify_fn and __bio_integrity_endio
>>>> routines?
>>>
>>> Yeah, double-free could be caused for READ between bio_integrity_verify_fn()
>>> and bio_uninit().
>>
>> ooops, the above race doesn't exist because __bio_integrity_endio()
>> returns false and bio_endio() won't call bio_uninit(). And bio_uninit()
>> is only called from bio_endio() when bio_integrity_verify_fn() exits.
>>
>> Also we can't remove the bio_integrity_free() from bio_integrity_verify_fn(),
>> otherwise this bio may never be ended because bio_integrity_endio() will
>> schedule the verify_fn again if bio_integrity_verify_fn() won't clear
>> REQ_INTEGRITY.
>>
>> So bio_integrity_free() is always called serially, and the flag of REQ_INTEGRITY
>> guarantees that it is only freed once.
>>
>> I think there isn't such double free you mentioned.
>>
>> Thanks,
>> Ming
>>
>>
>>
> 
> Right agreed, the REQ_INTEGRITY flag is what is guaranteeing freeing 
> only once.
> 
> Thanks for clearing this up.  I'm good with the patch.

Thanks all - I have applied the patch, and re-instated Justin's
signed-off-by.

-- 
Jens Axboe