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,
Justin
