Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo <tj@kernel.org>]

Hello,

On Tue, Sep 11, 2012 at 08:42:28AM +0200, Bart Van Assche wrote:
> Good question. As far as I can see calling request_queue.request_fn() is
> fine as long as the caller holds a reference on the queue. If e.g.
> scsi_request_fn() would get invoked after blk_drain_queue() finished it
> will return immediately because it was invoked with an empty request
> queue. So we should be fine as long as all blk_run_queue() callers
> either hold a reference on the request queue itself or on the sdev that
> owns the request queue. As far as I can see if patch
> http://marc.info/?l=linux-scsi&m=134453905402413 gets accepted then all
> callers in the SCSI core of blk_run_queue() will hold a (direct or
> indirect) reference on the request_queue before invoking blk_run_queue()
> or __blk_run_queue().

It's been quite a while since I really looked through the code and I'm
feeling a bit dense but what you describe seems like a two-pronged
approach where the drain stalling, when properly done, should be
enough.

The problem at hand IIUC is ->request_fn() being invoked when
request_queue itself is alive but the underlying driver is gone.  We
already make sure that a new request is not queued once drain is
complete but there's no guarantee about calling into ->request_fn()
and this is what you want to fix, right?

I think this is something which the block layer proper should handle
correctly and expose sane interface.  ie. if the caller has
request_queue reference, it should be safe to call __blk_run_queue()
no matter what.  As long as SCSI follows proper shutdown procedure, it
shouldn't need to worry about this.

Am I hopelessly confused somewhere?

Thanks.

-- 
tejun