Re: [PATCH] fix NULL-pointer dereference on scsi_run_queue

From: James Bottomley
Date: Thu Aug 02 2012 - 05:34:57 EST


On Thu, 2012-08-02 at 18:28 +0900, Chanho Min wrote:
> On Thu, Aug 2, 2012 at 5:57 PM, James Bottomley
> <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Thu, 2012-08-02 at 17:41 +0900, Chanho Min wrote:
> >> This patch is to fix a oops from a torn down device. When
> >> scsi_run_queue process starved queues, scsi_request_fn can race with
> >> scsi_remove_device. In this case, rarely, scsi_request_fn release the
> >> last reference and set sdev->request_queue to NULL. It result in
> >> NULL-pointer dereference when spin_unlock is tried with (NULL)->
> >> queue_lock. We need to add an extra reference to the device on both
> >> sides of the __blk_run_queue to hold reference until scsi_request_fn
> >> is finished.
> >
> > You need a recent kernel with this patch:
> >
> > commit 940f5d47e2f2e1fa00443921a0abf4822335b54d
> > Author: Bart Van Assche <bvanassche@xxxxxxx>
> > Date: Fri Jun 29 15:34:26 2012 +0000
> >
> > [SCSI] Avoid dangling pointer in scsi_requeue_command()
> >
> > James
> It is different from my case. This is occured inside scsi_run_queue
> and on processing starved_list.
> Another sdev is obtained from starved_list.

Does it occur with that patch applied?

If it does, the likely fix would be to take a copy of the queue ... but
I'd like to understand why first. An active command has an automatic
reference to the sdev_gendev, so it shouldn't be the normal case. This
was broken by unprep because it releases the command from the queue and
drops the reference. We may have another case like unjprep, but in that
case, we need to find it ... trying to add extra get/put_device() calls
will paper over the problem.

James


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