lkml.org 
[lkml]   [2012]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Kernel crashing on eject SD card
Hello,

On Thu, Feb 16, 2012 at 10:26:38AM +0900, Jun'ichi Nomura wrote:
> >> +int invalidate_partitions(struct gendisk *disk, struct block_device *bdev)
> >> +{
> >> + int res;
> >> +
> >> + res = drop_partitions(disk, bdev);
> >> + if (res)
> >> + return res;
> >> +
> >
> > Hmmm... shouldn't we have set_capacity(disk, 0) here?
>
> Added.
> I wasn't sure whether I should leave it to drivers.

The problem is that we shouldn't call into drivers without first
opening the device, so....
> But it seems capacity 0 for ENOMEDIUM device is reasonable.

Yeah, I *think* it should be okay.

> >> + check_disk_size_change(disk, bdev);
> >> + bdev->bd_invalidated = 0;
> >> + /* tell userspace that the media / partition table may have changed */
> >> + kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
> >
> > Also, we really shouldn't be generating KOBJ_CHANGE after every
> > -ENOMEDIUM open. This can easily lead to infinite loop. We should
> > generate this iff we actually dropped partitions && modified the size.
>
> invalidate_partitions() is called only when bd_invalidated is set.
> So KOBJ_CHANGE is not raised for every ENOMEDIUM open.

Ah, okay.

> I put it explicit in the function to make it safer for
> possible misuse.
>
> How about this?
>
> ---------------------------------------------------------
> Do not call drivers when invalidating partitions for -ENOMEDIUM
>
> When a scsi driver returns -ENOMEDIUM for open(),
> __blkdev_get() calls rescan_partitions(), which ends up calling
> sd_revalidate_disk() without getting a refcount of scsi_device.
>
> That could lead to oops like this:
>
> process A process B
> ----------------------------------------------
> sys_open
> __blkdev_get
> sd_open
> returns -ENOMEDIUM
> scsi_remove_device
> <scsi_device torn down>
> rescan_partitions
> sd_revalidate_disk
> <oops>
>
> Oopses are reported here:
> http://marc.info/?l=linux-scsi&m=132388619710052
>
> This patch separates the partition invalidation from rescan_partitions()
> and use it for -ENOMEDIUM case.

Yeah, this looks good to me.

Thank you.

--
tejun


\
 
 \ /
  Last update: 2012-02-16 17:39    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site