| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1422481550.2086.16.camel@HansenPartnership.com>
Date: Wed, 28 Jan 2015 13:45:50 -0800
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Bart Van Assche <bart.vanassche@...disk.com>
Cc: Christoph Hellwig <hch@...radead.org>,
Rusty Russell <rusty@...tcorp.com.au>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: module: fix module_refcount() return when running in a module
exit routine
On Wed, 2015-01-28 at 10:23 +0100, Bart Van Assche wrote:
> On 01/23/15 19:42, James Bottomley wrote:
> > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> > index 08c90a7..31ba254 100644
> > --- a/drivers/scsi/scsi.c
> > +++ b/drivers/scsi/scsi.c
> > @@ -965,6 +965,15 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
> > }
> > EXPORT_SYMBOL(scsi_report_opcode);
> >
> > +static int __scsi_device_get_common(struct scsi_device *sdev)
> > +{
> > + if (sdev->sdev_state == SDEV_DEL)
> > + return -ENXIO;
> > + if (!get_device(&sdev->sdev_gendev))
> > + return -ENXIO;
> > + return 0;
> > +}
> > +
> > /**
> > * scsi_device_get - get an additional reference to a scsi_device
> > * @sdev: device to get a reference to
> > @@ -975,19 +984,45 @@ EXPORT_SYMBOL(scsi_report_opcode);
> > */
> > int scsi_device_get(struct scsi_device *sdev)
> > {
> > - if (sdev->sdev_state == SDEV_DEL)
> > - return -ENXIO;
> > - if (!get_device(&sdev->sdev_gendev))
> > - return -ENXIO;
> > - /* We can fail this if we're doing SCSI operations
> > - * from module exit (like cache flush) */
> > - try_module_get(sdev->host->hostt->module);
> > + int ret;
> >
> > - return 0;
> > + ret = __scsi_device_get_common(sdev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = try_module_get(sdev->host->hostt->module);
> > +
> > + if (ret)
> > + put_device(&sdev->sdev_gendev);
> > +
> > + return ret;
> > }
> > EXPORT_SYMBOL(scsi_device_get);
> >
> > /**
> > + * scsi_device_get_in_module_exit() - get an additional reference to a scsi_device
> > + * @sdev: device to get a reference to
> > + *
> > + * Functions identically to scsi_device_get() except that it unconditionally
> > + * gets the module reference. This allows it to be called from module exit
> > + * routines where scsi_device_get() will fail. This routine is still paired
> > + * with scsi_device_put().
> > + */
> > +int scsi_device_get_in_module_exit(struct scsi_device *sdev)
> > +{
> > + int ret;
> > +
> > + ret = __scsi_device_get_common(sdev);
> > + if (ret)
> > + return ret;
> > +
> > + __module_get(sdev->host->hostt->module);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(scsi_device_get_in_module_exit);
> > +
> > +/**
> > * scsi_device_put - release a reference to a scsi_device
> > * @sdev: device to release a reference on.
> > *
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index ebf35cb6..057604e 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -564,16 +564,22 @@ static int sd_major(int major_idx)
> > }
> > }
> >
> > -static struct scsi_disk *__scsi_disk_get(struct gendisk *disk)
> > +static struct scsi_disk *__scsi_disk_get(struct gendisk *disk, int in_exit)
> > {
> > struct scsi_disk *sdkp = NULL;
> >
> > if (disk->private_data) {
> > + int ret;
> > +
> > sdkp = scsi_disk(disk);
> > - if (scsi_device_get(sdkp->device) == 0)
> > - get_device(&sdkp->dev);
> > + if (in_exit)
> > + ret = scsi_device_get_in_module_exit(sdkp->device);
> > else
> > + ret = scsi_device_get(sdkp->device);
> > + if (unlikely(ret))
> > sdkp = NULL;
> > + else
> > + get_device(&sdkp->dev);
> > }
> > return sdkp;
> > }
> > @@ -583,19 +589,19 @@ static struct scsi_disk *scsi_disk_get(struct gendisk *disk)
> > struct scsi_disk *sdkp;
> >
> > mutex_lock(&sd_ref_mutex);
> > - sdkp = __scsi_disk_get(disk);
> > + sdkp = __scsi_disk_get(disk, 0);
> > mutex_unlock(&sd_ref_mutex);
> > return sdkp;
> > }
> >
> > -static struct scsi_disk *scsi_disk_get_from_dev(struct device *dev)
> > +static struct scsi_disk *scsi_disk_get_from_dev(struct device *dev, int in_exit)
> > {
> > struct scsi_disk *sdkp;
> >
> > mutex_lock(&sd_ref_mutex);
> > sdkp = dev_get_drvdata(dev);
> > if (sdkp)
> > - sdkp = __scsi_disk_get(sdkp->disk);
> > + sdkp = __scsi_disk_get(sdkp->disk, in_exit);
> > mutex_unlock(&sd_ref_mutex);
> > return sdkp;
> > }
> > @@ -1525,7 +1531,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
> >
> > static void sd_rescan(struct device *dev)
> > {
> > - struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
> > + struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev, 0);
> >
> > if (sdkp) {
> > revalidate_disk(sdkp->disk);
> > @@ -3147,7 +3153,7 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
> > */
> > static void sd_shutdown(struct device *dev)
> > {
> > - struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
> > + struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev, 1);
> >
> > if (!sdkp)
> > return; /* this can happen */
> > @@ -3171,7 +3177,7 @@ exit:
> >
> > static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
> > {
> > - struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
> > + struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev, 0);
> > int ret = 0;
> >
> > if (!sdkp)
> > @@ -3213,7 +3219,7 @@ static int sd_suspend_runtime(struct device *dev)
> >
> > static int sd_resume(struct device *dev)
> > {
> > - struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
> > + struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev, 0);
> > int ret = 0;
> >
> > if (!sdkp->device->manage_start_stop)
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index 2e0281e..0bad37c 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -327,6 +327,7 @@ extern int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh);
> > void scsi_attach_vpd(struct scsi_device *sdev);
> >
> > extern int scsi_device_get(struct scsi_device *);
> > +extern int scsi_device_get_in_module_exit(struct scsi_device *);
> > extern void scsi_device_put(struct scsi_device *);
> > extern struct scsi_device *scsi_device_lookup(struct Scsi_Host *,
> > uint, uint, u64);
>
> Hello James,
>
> Is this the latest version of this patch that is available ? I have
> tried to test the above patch. However, I couldn't test the impact of
> this patch on the SRP initiator driver since my test system didn't boot
> anymore with the above patch applied. That test system boots from an ATA
> disk managed by the SCSI subsystem:
> $ lsscsi | head -n1
> [0:0:0:0] disk ATA KINGSTON SH103S3 BBF0 /dev/sda
Not yet, since I knew it would need a bit of testing to identify all the
potential in_exit acquisitions. However, you could help me by
diagnosing the current failure.
Thanks,
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists