Re: [PATCH] mmc: prevent dangling block device from accessing stalequeues

[Pierre Ossman]

On Thu, 04 Jun 2009 21:00:42 +0200
Stefan Bader <stefan.bader@xxxxxxxxxxxxx> wrote:

> Pierre Ossman wrote:
> > 
> > You seem to have dug a bit further than I've had time for. Do you have
> > anything substantial to back this up:
> > 
> >> +	/*
> >> +	 * Calling blk_cleanup_queue() would be too soon here. As long as
> >> +	 * the gendisk has a reference to it and is not released we should
> >> +	 * keep the queue. It has been shutdown and will not accept any new
> >> +	 * requests, so that should be safe.
> >> +	 */
> > 
> 
> This is mostly based on the debug output. But it seems hard to get around of it 
> without having a way to increment the refcount of the queue. It is probably not 
> the most common use case to remove a device while it is mounted.
> Hm, not sure this is what you wanted to know... On the launchpad report there 
> are logs which I took with lots of printk's enabled. This shows that after 
> resume the queue receives a request from mmcblk0 (which no longer exists) but 
> uses the same pointer as mmcblk1 which was just created.
> 

I was hoping you had dug around in the block layer and had some idea
why gendisk requires someone else to keep the queue around for it. Is
it just a simple case of a missing reference, or is there some
architectural problem?

> > This part from the launchpad report also seems incredibly broken:
> > 
> >> What makes the whole thing a disaster is the fact that the block device queue objects are taken from a slub cache. Which means on resume, the newly created block device will get the same queue object as the old one, initializes it and
> >> after the tasks have been resumed, ext3 feels obliged to write out the invalidated superblocks (still not sure why it goes for sector 0) which will happily migrate to the new block device and cause confusion.
> 
> I don't think that part is that much broken. It is more a unfortunate result of 
> the previous events. Maybe the part of ext3 writing to sector 0 is a bit 
> worrying as I would only expect it to update the mount information which I hink 
> is somewhere around sector 10.
> 

The incredibly broken part is how requests for the old queue wind up on
the new queue. Such a thing should never be possible.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

Attachment: signature.asc
Description: PGP signature