From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Roland Dreier <roland@kernel.org>,
Heiko Carstens <heiko.carstens@de.ibm.com>,
Jens Axboe <jaxboe@fusionio.com>,
linux-scsi@vger.kernel.org,
Steffen Maier <maier@linux.vnet.ibm.com>,
"Manvanthara B. Puttashankar" <manvanth@linux.vnet.ibm.com>,
Tarak Reddy <tarak.reddy@in.ibm.com>,
"Seshagiri N. Ippili" <sesh17@linux.vnet.ibm.com>
Subject: Re: [BUG] 2.6.39.1 crash in scsi_dispatch_cmd()
Date: Wed, 06 Jul 2011 09:24:09 -0500 [thread overview]
Message-ID: <1309962249.3282.1.camel@mulgrave> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1107061013440.1995-100000@iolanthe.rowland.org>
On Wed, 2011-07-06 at 10:20 -0400, Alan Stern wrote:
> On Wed, 6 Jul 2011, Roland Dreier wrote:
>
> > Alan Stern's patch looks a bit fishy -- the scsi_free_queue() is moved
> > earlier than the
> >
> > /* cause the request function to reject all I/O requests */
> > sdev->request_queue->queuedata = NULL;
> >
> > which seems to leave a small window where the use-after-free can
> > happen, and it's not clear to me why the scsi_free_queue() has to move
> > at all.
>
> Looks can be deceiving. Although the scsi_free_queue() is higher up in
> the source file, it actually runs later than this code. That's because
> __scsi_remove_device() -- this code -- gets called when the device is
> unregistered from the driver core, whereas
> scsi_device_dev_release_usercontext() -- where the scsi_free_queue() is
> moved to -- gets called when the last reference to the device is
> dropped.
>
> Now, one of the things I'm not sure about (it would nice if James would
> pick up this thread again and comment) is whether queuedata should be
> set to NULL at unregistration time or later on, when the device and the
> queue are about to be freed.
Sorry, higher priority problems at the moment. Sorry about the
->queuedata cockup, was thinking of sdev->request_queue. Moving the
queue free is wrong ... it recently moved to fix another oops. Problem
most likely missing block guards on blk_execute_req() ... no check for
QUEUE_DEAD.
Will be back on Thursday.
James
next prev parent reply other threads:[~2011-07-06 14:24 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-15 11:20 [BUG] 2.6.39.1 crash in scsi_dispatch_cmd() Heiko Carstens
2011-06-16 16:01 ` Heiko Carstens
2011-06-16 16:37 ` James Bottomley
2011-06-16 18:40 ` Heiko Carstens
2011-06-20 15:30 ` Heiko Carstens
2011-07-01 18:07 ` Roland Dreier
2011-07-01 19:04 ` James Bottomley
2011-07-06 0:34 ` Roland Dreier
2011-07-06 6:47 ` Heiko Carstens
2011-07-06 8:06 ` Roland Dreier
2011-07-06 9:25 ` Heiko Carstens
2011-07-06 14:20 ` Alan Stern
2011-07-06 14:24 ` James Bottomley [this message]
2011-07-06 16:30 ` Roland Dreier
2011-07-06 16:53 ` Alan Stern
2011-07-06 18:07 ` Roland Dreier
2011-07-06 18:49 ` Alan Stern
2011-07-07 20:45 ` James Bottomley
2011-07-07 21:07 ` Alan Stern
2011-07-08 17:04 ` James Bottomley
2011-07-08 19:43 ` Alan Stern
2011-07-08 20:41 ` James Bottomley
2011-07-08 22:08 ` Alan Stern
2011-07-08 22:25 ` James Bottomley
2011-07-08 20:47 ` Roland Dreier
2011-07-08 23:04 ` [PATCH] block: Check that queue is alive in blk_insert_cloned_request() Roland Dreier
2011-07-09 9:05 ` Stefan Richter
2011-07-11 22:40 ` Mike Snitzer
2011-07-12 0:52 ` Alan Stern
2011-07-12 1:22 ` Mike Snitzer
2011-07-12 1:46 ` Vivek Goyal
2011-07-12 15:24 ` Alan Stern
2011-07-12 17:10 ` Vivek Goyal
2011-07-12 14:58 ` [PATCH] dm mpath: manage reference on request queue of underlying devices Mike Snitzer
2011-07-12 17:06 ` [PATCH] block: Check that queue is alive in blk_insert_cloned_request() Vivek Goyal
2011-07-12 17:41 ` James Bottomley
2011-07-12 18:02 ` Vivek Goyal
2011-07-12 18:28 ` James Bottomley
2011-07-12 18:54 ` Vivek Goyal
2011-07-12 21:02 ` Alan Stern
2011-07-12 2:09 ` Vivek Goyal
2011-07-06 16:24 ` [BUG] 2.6.39.1 crash in scsi_dispatch_cmd() Roland Dreier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1309962249.3282.1.camel@mulgrave \
--to=james.bottomley@hansenpartnership.com \
--cc=heiko.carstens@de.ibm.com \
--cc=jaxboe@fusionio.com \
--cc=linux-scsi@vger.kernel.org \
--cc=maier@linux.vnet.ibm.com \
--cc=manvanth@linux.vnet.ibm.com \
--cc=roland@kernel.org \
--cc=sesh17@linux.vnet.ibm.com \
--cc=stern@rowland.harvard.edu \
--cc=tarak.reddy@in.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox