Date: Tue, 1 Nov 2005 10:08:58 +0100
From: Jens Axboe <axboe@suse.de>
To: Tejun Heo <htejun@gmail.com>
Cc: torvalds@osdl.org, acme@mandriva.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] blk: fix dangling pointer access in __elv_add_request
Message-ID: <20051101090857.GC26049@suse.de>
References: <20051101082349.GA17756@htj.dyndns.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20051101082349.GA17756@htj.dyndns.org>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1197
Lines: 29

On Tue, Nov 01 2005, Tejun Heo wrote:
> cfq's add_req_fn callback may invoke q->request_fn directly and
> depending on low-level driver used and timing, a queued request may be
> finished & deallocated before add_req_fn callback returns.  So,
> __elv_add_request must not access rq after it's passed to add_req_fn
> callback.

It's a generel problem, you may get the queue run at any time regardless
of what the io scheduler is doing. CFQ does run the queue manully
sometimes, but SCSI may do the very same thing for you as well. Given
that SCSI also shortly reenables interrupts in the ->request_fn()
handling, it's quite possible for the request to be completed.

So, as we don't hold a reference to the request, I'd say your patch
looks correct and should be applied right away.

> Jens, does generalizing queue kicking functions and disallowing
> ioscheds from directly calling q->request_fn sound like a good idea?

Yes certainly.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/