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
