Re: [PATCH] sd: Fix crash due to race when removing scsi disk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 1, 2016 at 12:36 PM, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 2016-07-01 at 12:14 -0400, Howard Cochran wrote:
>>

>> This patch fixes the race by making sd_remove() hold bd_mutex during
>> the call to del_gendisk().
>
> OK, so this can't be the proper fix because it's a layering violation.
>  You can see this if you consider what happens to any other block
> device doing the same thing: they would oops in the same way, implying
> that this fix would have to be replicated to every other block device
> remove path.  Instead place to fix it should be somewhere inside the
> block subsystem.  My suspicion is that it should probably be in
> del_gendisk() because that will fix every device, but the block people
> should think more about the problem (linux-block cc added).
>
> James

James,

I originally attempted to fix this by grabbing bd_mutex inside
del_gendisk(). However, I surveyed the kernel and found that some other
block drivers already hold bd_mutex when calling del_gendisk(), so that
would deadlock. For example: blkfront_closing() and blkfront_remove() in
drivers/block/xen-blkfront.c

Some of them ensure that there can be no dirty blocks in the same code
path prior to calling del_gendisk() (e.g. zram_remove()).

Other drivers (e.g. loop) appear to only allow removal via an ioctl(),
which, by definition, means that you have an bd_openers, and therefore
cannot have cleared the bdev->bd_disk pointer.

In fact, it appeared that sd.c may have the only "out of band" way to
remove a device via its "delete" node, but now I see that oasdblk.c has
a "remove" node that appears susceptible to this problem.  Another may
be mg_disk.c

OK, so indeed, a more general solution is needed. But taking bd_mutex
inside del_gendisk() does not appear to be it. Perhaps making
mapping_cap_writeback_dirty() not have to dereference bdev->bd_disk
would be better (as it didn't prior to de1414a654e6)?

Howard
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux