[PATCH] scsi: replace broken specification URL
The t10.org website containing SCSI-2 draft specifications now requires to be from a member company to access the documents. This replaces the now broken link with another public resource where the specifications can be found. Signed-off-by: Michael Opdenacker --- Documentation/DocBook/scsi.tmpl | 2 +- drivers/scsi/scsicam.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/scsi.tmpl b/Documentation/DocBook/scsi.tmpl index 4b9b9b286cea..41ad3086b7a2 100644 --- a/Documentation/DocBook/scsi.tmpl +++ b/Documentation/DocBook/scsi.tmpl @@ -160,7 +160,7 @@ drivers/scsi/scsicam.c - SCSI + SCSI Common Access Method support functions, for use with HDIO_GETGEO, etc. diff --git a/drivers/scsi/scsicam.c b/drivers/scsi/scsicam.c index 910f4a7a3924..8144618fe2de 100644 --- a/drivers/scsi/scsicam.c +++ b/drivers/scsi/scsicam.c @@ -207,7 +207,7 @@ EXPORT_SYMBOL(scsi_partsize); * * WORKINGX3T9.2 * DRAFT792D - * see http://www.t10.org/ftp/t10/drafts/cam/cam-r12b.pdf + * see ftp://ftp.uni-duisburg.de/Hardware/SCSI/cam-r12b.pdf * *Revision 6 * 10-MAR-94 -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] hpsa: change hpsa_passthru_ioctl timeout
Was not alloting for FW Flash times. Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 375a396..030d002 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6524,7 +6524,7 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp) c->SG[0].Ext = cpu_to_le32(HPSA_SG_LAST); /* not chaining */ } rc = hpsa_scsi_do_simple_cmd(h, c, DEFAULT_REPLY_QUEUE, - DEFAULT_TIMEOUT); + NO_TIMEOUT); if (iocommand.buf_size > 0) hpsa_pci_unmap(h->pdev, c, 1, PCI_DMA_BIDIRECTIONAL); check_ioctl_unit_attention(h, c); @@ -6657,7 +6657,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp) c->SG[--i].Ext = cpu_to_le32(HPSA_SG_LAST); } status = hpsa_scsi_do_simple_cmd(h, c, DEFAULT_REPLY_QUEUE, - DEFAULT_TIMEOUT); + NO_TIMEOUT); if (sg_used) hpsa_pci_unmap(h->pdev, c, sg_used, PCI_DMA_BIDIRECTIONAL); check_ioctl_unit_attention(h, c); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] hpsa: correct skipping masked peripherals
The SA controller spins down RAID drive spares.
A REGNEWD event causes an inquiry to be sent to all physical
drives. This causes the SA controller to spin up the spare.
The controller suspends all I/O to a logical volume until
the spare is spun up. The spin-up can take over 50 seconds.
This can result in one or both of the following:
- SML sends down aborts and resets to the logical volume
and can cause the logical volume to be off-lined.
- a negative impact on the logical volume's I/O performance
each time a REGNEWD is triggered.
Reviewed-by: Scott Teel
Reviewed-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/hpsa.c | 79 ---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ff8dcd5..375a396 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4105,6 +4105,70 @@ static int hpsa_set_local_logical_count(struct ctlr_info
*h,
return rc;
}
+static bool hpsa_is_disk_spare(struct ctlr_info *h, u8 *lunaddrbytes)
+{
+ struct bmic_identify_physical_device *id_phys;
+ bool is_spare = false;
+ int rc;
+
+ id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
+ if (!id_phys)
+ return false;
+
+ rc = hpsa_bmic_id_physical_device(h,
+ lunaddrbytes,
+ GET_BMIC_DRIVE_NUMBER(lunaddrbytes),
+ id_phys, sizeof(*id_phys));
+ if (rc == 0)
+ is_spare = (id_phys->more_flags >> 6) & 0x01;
+
+ kfree(id_phys);
+ return is_spare;
+}
+
+#define RPL_DEV_FLAG_NON_DISK 0x1
+#define RPL_DEV_FLAG_UNCONFIG_DISK_REPORTING_SUPPORTED 0x2
+#define RPL_DEV_FLAG_UNCONFIG_DISK 0x4
+
+#define BMIC_DEVICE_TYPE_ENCLOSURE 6
+
+static bool hpsa_skip_device(struct ctlr_info *h, u8 *lunaddrbytes,
+ struct ext_report_lun_entry *rle)
+{
+ u8 device_flags;
+ u8 device_type;
+
+ if (!MASKED_DEVICE(lunaddrbytes))
+ return false;
+
+ device_flags = rle->device_flags;
+ device_type = rle->device_type;
+
+ if (device_flags & RPL_DEV_FLAG_NON_DISK) {
+ if (device_type == BMIC_DEVICE_TYPE_ENCLOSURE)
+ return false;
+ return true;
+ }
+
+ if (!(device_flags & RPL_DEV_FLAG_UNCONFIG_DISK_REPORTING_SUPPORTED))
+ return false;
+
+ if (device_flags & RPL_DEV_FLAG_UNCONFIG_DISK)
+ return false;
+
+ /*
+* Spares may be spun down, we do not want to
+* do an Inquiry to a RAID set spare drive as
+* that would have them spun up, that is a
+* performance hit because I/O to the RAID device
+* stops while the spin up occurs which can take
+* over 50 seconds.
+*/
+ if (hpsa_is_disk_spare(h, lunaddrbytes))
+ return true;
+
+ return false;
+}
static void hpsa_update_scsi_devices(struct ctlr_info *h)
{
@@ -4198,6 +4262,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
u8 *lunaddrbytes, is_OBDR = 0;
int rc = 0;
int phys_dev_index = i - (raid_ctlr_position == 0);
+ bool skip_device = false;
physical_device = i < nphysicals + (raid_ctlr_position == 0);
@@ -4205,11 +4270,15 @@ static void hpsa_update_scsi_devices(struct ctlr_info
*h)
lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,
i, nphysicals, nlogicals, physdev_list, logdev_list);
- /* skip masked non-disk devices */
- if (MASKED_DEVICE(lunaddrbytes) && physical_device &&
- (physdev_list->LUN[phys_dev_index].device_type != 0x06) &&
- (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
- continue;
+ /*
+* Skip over some devices such as a spare.
+*/
+ if (!tmpdevice->external && physical_device) {
+ skip_device = hpsa_skip_device(h, lunaddrbytes,
+ &physdev_list->LUN[phys_dev_index]);
+ if (skip_device)
+ continue;
+ }
/* Get device type, vendor, model, device id */
rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] lib: string: add function strtolower()
On Fri, Jul 01 2016, Markus Mayer wrote:
> Add a function called strtolower() to convert strings to lower case
> in-place, overwriting the original string.
>
> This seems to be a recurring requirement in the kernel that is
> currently being solved by several duplicated implementations doing the
> same thing.
>
> Signed-off-by: Markus Mayer
> ---
> include/linux/string.h | 1 +
> lib/string.c | 14 ++
> 2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..aad605e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
> #endif
> void *memchr_inv(const void *s, int c, size_t n);
> char *strreplace(char *s, char old, char new);
> +char *strtolower(char *s);
>
> extern void kfree_const(const void *x);
>
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..6e3b560 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
> return s;
> }
> EXPORT_SYMBOL(strreplace);
> +
> +char *strtolower(char *s)
> +{
> + char *p;
> +
> +if (unlikely(!s))
> +return NULL;
> +
> + for (p = s; *p; p++)
> + *p = tolower(*p);
> +
> + return s;
> +}
> +EXPORT_SYMBOL(strtolower);
A few suggestions:
- Make the function take separate src and dst parameters, making it explicitly
allowed to pass the same value (but not other kinds of overlap, of
course). That way one can avoid "strcpy(dst, src); strtolower(dst);".
- Drop the NULL check. If someone does "foo->bar = something;
strtolower(foo->bar); put foo in a global data structure...", the
dereference of foo->bar may happen much later. Doing the NULL deref
sooner means it's much easier to find and fix the bug. (Also, other
str* and mem* functions don't usually check for NULL).
- While it's true that strcpy and memcpy by definition return dst, that's
mostly useless. If you want it to return anything, please make it
something that might be used - for example, having stpcpy semantics
(returning a pointer to dst's terminating \0) means a caller might avoid
a strlen call.
- Maybe do strtoupper while you're at it. Quick grepping didn't find any
use for the copy-while-lowercasing, but copy-while-uppercasing can at
least be used in drivers/acpi/acpica/nsrepair2.c,
drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c,
drivers/power/power_supply_sysfs.c along with a bunch of inplace
uppercasing.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] hpsa update
These patches are based on Linus's tree The changes are: - enhanced check for skipping masked devices - corrected hpsa_passthru_ioctl timeouts for fw flash --- Don Brace (2): hpsa: correct skipping masked peripherals hpsa: change hpsa_passthru_ioctl timeout drivers/scsi/hpsa.c | 83 +++ 1 file changed, 76 insertions(+), 7 deletions(-) -- Signature -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sd: Fix crash due to race when removing scsi disk
On Fri, Jul 1, 2016 at 12:36 PM, James Bottomley 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] lib: string: add function strtolower()
On Fri, 01 Jul 2016, Markus Mayer wrote:
> On 1 July 2016 at 03:52, Jani Nikula wrote:
>> On Fri, 01 Jul 2016, Markus Mayer wrote:
>>> Add a function called strtolower() to convert strings to lower case
>>> in-place, overwriting the original string.
>>>
>>> This seems to be a recurring requirement in the kernel that is
>>> currently being solved by several duplicated implementations doing the
>>> same thing.
>>>
>>> Signed-off-by: Markus Mayer
>>> ---
>>> include/linux/string.h | 1 +
>>> lib/string.c | 14 ++
>>> 2 files changed, 15 insertions(+)
>>>
>>> diff --git a/include/linux/string.h b/include/linux/string.h
>>> index 26b6f6a..aad605e 100644
>>> --- a/include/linux/string.h
>>> +++ b/include/linux/string.h
>>> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
>>> #endif
>>> void *memchr_inv(const void *s, int c, size_t n);
>>> char *strreplace(char *s, char old, char new);
>>> +char *strtolower(char *s);
>>>
>>> extern void kfree_const(const void *x);
>>>
>>> diff --git a/lib/string.c b/lib/string.c
>>> index ed83562..6e3b560 100644
>>> --- a/lib/string.c
>>> +++ b/lib/string.c
>>> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>>> return s;
>>> }
>>> EXPORT_SYMBOL(strreplace);
>>> +
>>
>> This needs a kernel-doc comment right here.
>
> Will add it.
>
>>> +char *strtolower(char *s)
>>> +{
>>> + char *p;
>>> +
>>> +if (unlikely(!s))
>>> +return NULL;
>>
>> Using spaces for indentation? See scripts/checkpatch.pl.
>
> Not on purpose. Thanks for spotting it.
>
>>> +
>>> + for (p = s; *p; p++)
>>> + *p = tolower(*p);
>>> +
>>> + return s;
>>
>> Why does it return a value? Could be void?
>
> It could be void, but I thought that would make the function's use
> less flexible. As is, the return value is there if anybody wants it,
> but it can be ignored if it is not needed. Also, it seems customary
> for string functions to be returning the string that was passed in.
>
> I'll change it to void if there are strong opinions leaning that way.
> Personally, I like that it returns a char * better.
I don't have strong opinions on this. Just a general aversion to
returning something redundant. Avoids questions like, does it allocate a
new string, should I use the return value instead of the string I passed
in, should I check the return value or can I ignore it, should I check
both the string I pass in and the return value for != NULL, etc. But I
could be persuaded either way.
BR,
Jani.
>
>> BR,
>> Jani.
>>
>>> +}
>>> +EXPORT_SYMBOL(strtolower);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] lib: string: add function strtolower()
On 1 July 2016 at 03:52, Jani Nikula wrote:
> On Fri, 01 Jul 2016, Markus Mayer wrote:
>> Add a function called strtolower() to convert strings to lower case
>> in-place, overwriting the original string.
>>
>> This seems to be a recurring requirement in the kernel that is
>> currently being solved by several duplicated implementations doing the
>> same thing.
>>
>> Signed-off-by: Markus Mayer
>> ---
>> include/linux/string.h | 1 +
>> lib/string.c | 14 ++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 26b6f6a..aad605e 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
>> #endif
>> void *memchr_inv(const void *s, int c, size_t n);
>> char *strreplace(char *s, char old, char new);
>> +char *strtolower(char *s);
>>
>> extern void kfree_const(const void *x);
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index ed83562..6e3b560 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>> return s;
>> }
>> EXPORT_SYMBOL(strreplace);
>> +
>
> This needs a kernel-doc comment right here.
Will add it.
>> +char *strtolower(char *s)
>> +{
>> + char *p;
>> +
>> +if (unlikely(!s))
>> +return NULL;
>
> Using spaces for indentation? See scripts/checkpatch.pl.
Not on purpose. Thanks for spotting it.
>> +
>> + for (p = s; *p; p++)
>> + *p = tolower(*p);
>> +
>> + return s;
>
> Why does it return a value? Could be void?
It could be void, but I thought that would make the function's use
less flexible. As is, the return value is there if anybody wants it,
but it can be ignored if it is not needed. Also, it seems customary
for string functions to be returning the string that was passed in.
I'll change it to void if there are strong opinions leaning that way.
Personally, I like that it returns a char * better.
> BR,
> Jani.
>
>> +}
>> +EXPORT_SYMBOL(strtolower);
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sd: Fix crash due to race when removing scsi disk
On Fri, 2016-07-01 at 12:14 -0400, Howard Cochran wrote:
> This crash occurred while writing 1 to /sys/block/sda/device/delete
> at
> the same instant that another process was closing the block device:
>
> BUG: unable to handle kernel NULL pointer dereference at 0230
> IP: [] blk_get_backing_dev_info+0xc/0x20
> Oops: [#1] PREEMPT SMP
> Call Trace:
> [] ? __filemap_fdatawrite_range+0x15a/0x180
> [] ? __filemap_fdatawrite_range+0xe5/0x180
> [] filemap_write_and_wait+0x38/0x70
> [] fsync_bdev+0x41/0x50
> [] invalidate_partition+0x1c/0x40
> [] del_gendisk+0xcf/0x1c0
> [] sd_remove+0x53/0xb0
> [] __device_release_driver+0x80/0x120
> [] device_release_driver+0x1d/0x30
> [] bus_remove_device+0xb2/0xf0
> [] device_del+0xec/0x1e0
> [] ? kobject_put+0x58/0xc0
> [] __scsi_remove_device+0xaf/0xc0
> [] scsi_remove_device+0x1f/0x30
> [] sdev_store_delete+0x2b/0x40
> [] ? scsi_remove_device+0x30/0x30
> [] dev_attr_store+0x1f/0x40
>...
> [] SyS_write+0x4c/0xb0
> EIP: [] blk_get_backing_dev_info+0xc/0x20 SS:ESP
> 0068:f5eb9d18
>
> It is caused by this race: Between the time Thread B's instance of
> filemap_write_and_wait() has asked whether there are any pages to
> flush and
> when it it dereferences bdev->disk, Thread A can clear that pointer
> in
> __blkdev_put().
>
> Thread A: Thread B:
> blkdev_close()sdev_store_delete()
> blkdev_put() sd_remove()
> __blkdev_put()del_gendisk()
> mutex_lock(bd_mutex); invalidate_partition()
> sync_blkdev() fsync_bdev()
> filemap_write_and_wait()
> filemap_write_and_wait()
> if (mapping has pages)if (mapping has
> pages)
> deref bdev->disk (OK)
> Set bdev->bd_disk = NULL;
> mutex_unlock(bd_mutex); deref. bdev
> ->bd_disk (BOOM!)
>
> The "dereference bdev->disk" occurs on this sub-chain:
> filemap_write_and_wait()
> __filemap_fdatawrite_range()
> mapping_cap_writeback_dirty()
> inode_to_bdi()
> bdev_get_queue()
> return bdev->disk->queue;
>
> The problem was introduced by de1414a654e6 ("fs: export inode_to_bdi
> and use it in favor of mapping->backing_dev_info"). Before that
> change, mapping_cap_writeback_dirty() directly retrieved the
> backing_dev_info from the mapping rather than looking it up through
> mapping->host->inode_dev->bdev->bd_disk->queue.
Great analysis, thanks.
> This was found while running a stress test on an ARM-based embedded
> system which involved repeatedly shutting down many services
> simultaneously via systemd isolate (thereby making it likely that
> "Thread B" was preempted for awhile just before it dereferenced bdev
> ->bd_disk). I subsequently reproduced this on vanilla Linux 4.6 in
> QEMU/x86.
>
> This patch fixes the race by making sd_remove() hold bd_mutex during
> the call to del_gendisk().
>
> Fixes: de1414a654e6 ("fs: export inode_to_bdi and use it in favor of
> mapping->backing_dev_info")
> Signed-off-by: Howard Cochran
> Cc: Howard Cochran
> Cc: linux-scsi@vger.kernel.org
> Cc: Christoph Hellwig
> Cc: James Bottomley
> Cc: Martin K. Petersen
> ---
> drivers/scsi/sd.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index f52b74c..0f53925 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3126,6 +3126,7 @@ static int sd_remove(struct device *dev)
> {
> struct scsi_disk *sdkp;
> dev_t devt;
> + struct block_device *bdev;
>
> sdkp = dev_get_drvdata(dev);
> devt = disk_devt(sdkp->disk);
> @@ -3134,7 +3135,13 @@ static int sd_remove(struct device *dev)
> async_synchronize_full_domain(&scsi_sd_pm_domain);
> async_synchronize_full_domain(&scsi_sd_probe_domain);
> device_del(&sdkp->dev);
> +
> + bdev = bdget_disk(sdkp->disk, 0);
> + mutex_lock(&bdev->bd_mutex);
> del_gendisk(sdkp->disk);
> + mutex_unlock(&bdev->bd_mutex);
> + bdput(bdev);
> +
> sd_shutdown(dev);
>
> blk_register_region(devt, SD_MINORS, NULL,
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.htm
Re: [PATCH] sd: Fix crash due to race when removing scsi disk
On Fri, Jul 1, 2016 at 12:19 PM, Howard Cochran
wrote:
> On Fri, Jul 1, 2016 at 12:14 PM, Howard Cochran
> wrote:
>> This crash occurred while writing 1 to /sys/block/sda/device/delete at
>> the same instant that another process was closing the block device:
>>
>> BUG: unable to handle kernel NULL pointer dereference at 0230
>> This patch fixes the race by making sd_remove() hold bd_mutex during the
>> call to del_gendisk().
>>
>> Fixes: de1414a654e6 ("fs: export inode_to_bdi and use it in favor of
>> mapping->backing_dev_info")
>> Signed-off-by: Howard Cochran
Here is a method to reproduce this bug:
You need a system with a SATA or other scsi-disk that you are prepared to
overwrite destructively.
Apply this patch, to exaggerate the race window in the kernel. Obviously,
this isn't strictly necessary, but the window is normally small, so it can
be tricky to reproduce otherwise.
[ Begin patch to exaggerate race window ]
>From 336b6ce99adb544f4b475e8f25acfd442504e3dc Mon Sep 17 00:00:00 2001
From: Auto Configured
Date: Thu, 30 Jun 2016 18:04:21 -0400
Subject: [PATCH] HACK: Widen window to expose sd_remove() race
---
mm/filemap.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/mm/filemap.c b/mm/filemap.c
index f2479af..c097591 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include "internal.h"
#define CREATE_TRACE_POINTS
@@ -308,6 +309,11 @@ int __filemap_fdatawrite_range(struct
address_space *mapping, loff_t start,
.range_end = end,
};
+ if (!strcmp(current->comm, "scsi_del_bug")) {
+ printk("BEFORE: 0x%p\n", I_BDEV(mapping->host)->bd_disk);
+ msleep(5000);
+ printk("AFTER : 0x%p\n", I_BDEV(mapping->host)->bd_disk);
+ }
if (!mapping_cap_writeback_dirty(mapping))
return 0;
--
2.4.11
[ End patch ]
Save the following script with the name "scsci_del_bug". This name must
match exactly, because the patch above tests for it (yes, hacky-hacky). Edit
it to change blk_dev_to_overwrite= to the device you want it to overwrite.
Running this script should cause the NULL pointer crash in the kernel.
[ Begin script - Must be named "scsi_del_bug" ]
#!/bin/sh
# WARNING WARNING: THIS SCRIPT WILL OVERWRITE A BLOCK DEVICE!
# Set this to the disk device to overwrite:
blk_dev_to_overwrite=sda
# Tracing mount point (We _rely_ on tracing)
tracedir=/sys/kernel/debug/tracing
# Set up tracing but don't start it yet
echo 0 > $tracedir/tracing_on
echo > $tracedir/trace
echo 1024 > $tracedir/buffer_size_kb
echo 1 > $tracedir/events/syscalls/sys_enter_close/enable
# Create a bunch of dirty disk data (DESTRUCTIVE)
dd if=/dev/zero of=/dev/$blk_dev_to_overwrite bs=1M count=500 &
pid=$!
# Get tracing to say when dd begins to close its output file & wait for it
echo "common_pid == ${pid} && fd == 1" >
$tracedir/events/syscalls/sys_enter_close/filter
echo 1 > $tracedir/tracing_on
read FOO < $tracedir/trace_pipe
echo "Deleting disk device (while dd is still in close())"
echo 1 > /sys/block/sda/device/delete
[ End scsi_del_bug script ]
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sd: Fix crash due to race when removing scsi disk
On Fri, Jul 1, 2016 at 12:14 PM, Howard Cochran
wrote:
> This crash occurred while writing 1 to /sys/block/sda/device/delete at
> the same instant that another process was closing the block device:
>
> BUG: unable to handle kernel NULL pointer dereference at 0230
> IP: [] blk_get_backing_dev_info+0xc/0x20
> Oops: [#1] PREEMPT SMP
> Call Trace:
> [] ? __filemap_fdatawrite_range+0x15a/0x180
> [] ? __filemap_fdatawrite_range+0xe5/0x180
> [] filemap_write_and_wait+0x38/0x70
> [] fsync_bdev+0x41/0x50
> [] invalidate_partition+0x1c/0x40
> [] del_gendisk+0xcf/0x1c0
> [] sd_remove+0x53/0xb0
> [] __device_release_driver+0x80/0x120
> [] device_release_driver+0x1d/0x30
> [] bus_remove_device+0xb2/0xf0
> [] device_del+0xec/0x1e0
> [] ? kobject_put+0x58/0xc0
> [] __scsi_remove_device+0xaf/0xc0
> [] scsi_remove_device+0x1f/0x30
> [] sdev_store_delete+0x2b/0x40
> [] ? scsi_remove_device+0x30/0x30
> [] dev_attr_store+0x1f/0x40
>...
> [] SyS_write+0x4c/0xb0
> EIP: [] blk_get_backing_dev_info+0xc/0x20 SS:ESP 0068:f5eb9d18
>
> It is caused by this race: Between the time Thread B's instance of
> filemap_write_and_wait() has asked whether there are any pages to flush and
> when it it dereferences bdev->disk, Thread A can clear that pointer in
> __blkdev_put().
>
> Thread A: Thread B:
> blkdev_close()sdev_store_delete()
> blkdev_put() sd_remove()
> __blkdev_put()del_gendisk()
> mutex_lock(bd_mutex); invalidate_partition()
> sync_blkdev() fsync_bdev()
> filemap_write_and_wait() filemap_write_and_wait()
> if (mapping has pages)if (mapping has pages)
> deref bdev->disk (OK)
> Set bdev->bd_disk = NULL;
> mutex_unlock(bd_mutex); deref. bdev->bd_disk
> (BOOM!)
>
> The "dereference bdev->disk" occurs on this sub-chain:
> filemap_write_and_wait()
> __filemap_fdatawrite_range()
> mapping_cap_writeback_dirty()
> inode_to_bdi()
> bdev_get_queue()
> return bdev->disk->queue;
>
> The problem was introduced by de1414a654e6 ("fs: export inode_to_bdi and use
> it in favor of mapping->backing_dev_info"). Before that change,
> mapping_cap_writeback_dirty() directly retrieved the backing_dev_info from
> the mapping rather than looking it up through
> mapping->host->inode_dev->bdev->bd_disk->queue.
>
> This was found while running a stress test on an ARM-based embedded system
> which involved repeatedly shutting down many services simultaneously via
> systemd isolate (thereby making it likely that "Thread B" was preempted for
> awhile just before it dereferenced bdev->bd_disk). I subsequently reproduced
> this on vanilla Linux 4.6 in QEMU/x86.
>
> This patch fixes the race by making sd_remove() hold bd_mutex during the
> call to del_gendisk().
>
> Fixes: de1414a654e6 ("fs: export inode_to_bdi and use it in favor of
> mapping->backing_dev_info")
> Signed-off-by: Howard Cochran
> Cc: Howard Cochran
> Cc: linux-scsi@vger.kernel.org
> Cc: Christoph Hellwig
> Cc: James Bottomley
> Cc: Martin K. Petersen
> ---
> drivers/scsi/sd.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index f52b74c..0f53925 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3126,6 +3126,7 @@ static int sd_remove(struct device *dev)
> {
> struct scsi_disk *sdkp;
> dev_t devt;
> + struct block_device *bdev;
>
> sdkp = dev_get_drvdata(dev);
> devt = disk_devt(sdkp->disk);
> @@ -3134,7 +3135,13 @@ static int sd_remove(struct device *dev)
> async_synchronize_full_domain(&scsi_sd_pm_domain);
> async_synchronize_full_domain(&scsi_sd_probe_domain);
> device_del(&sdkp->dev);
> +
> + bdev = bdget_disk(sdkp->disk, 0);
> + mutex_lock(&bdev->bd_mutex);
> del_gendisk(sdkp->disk);
> + mutex_unlock(&bdev->bd_mutex);
> + bdput(bdev);
> +
> sd_shutdown(dev);
>
> blk_register_region(devt, SD_MINORS, NULL,
> --
> 1.9.1
>
Adding to Cc: Corrected email address for James Bottomley.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sd: Fix crash due to race when removing scsi disk
This crash occurred while writing 1 to /sys/block/sda/device/delete at
the same instant that another process was closing the block device:
BUG: unable to handle kernel NULL pointer dereference at 0230
IP: [] blk_get_backing_dev_info+0xc/0x20
Oops: [#1] PREEMPT SMP
Call Trace:
[] ? __filemap_fdatawrite_range+0x15a/0x180
[] ? __filemap_fdatawrite_range+0xe5/0x180
[] filemap_write_and_wait+0x38/0x70
[] fsync_bdev+0x41/0x50
[] invalidate_partition+0x1c/0x40
[] del_gendisk+0xcf/0x1c0
[] sd_remove+0x53/0xb0
[] __device_release_driver+0x80/0x120
[] device_release_driver+0x1d/0x30
[] bus_remove_device+0xb2/0xf0
[] device_del+0xec/0x1e0
[] ? kobject_put+0x58/0xc0
[] __scsi_remove_device+0xaf/0xc0
[] scsi_remove_device+0x1f/0x30
[] sdev_store_delete+0x2b/0x40
[] ? scsi_remove_device+0x30/0x30
[] dev_attr_store+0x1f/0x40
...
[] SyS_write+0x4c/0xb0
EIP: [] blk_get_backing_dev_info+0xc/0x20 SS:ESP 0068:f5eb9d18
It is caused by this race: Between the time Thread B's instance of
filemap_write_and_wait() has asked whether there are any pages to flush and
when it it dereferences bdev->disk, Thread A can clear that pointer in
__blkdev_put().
Thread A: Thread B:
blkdev_close()sdev_store_delete()
blkdev_put() sd_remove()
__blkdev_put()del_gendisk()
mutex_lock(bd_mutex); invalidate_partition()
sync_blkdev() fsync_bdev()
filemap_write_and_wait() filemap_write_and_wait()
if (mapping has pages)if (mapping has pages)
deref bdev->disk (OK)
Set bdev->bd_disk = NULL;
mutex_unlock(bd_mutex); deref. bdev->bd_disk (BOOM!)
The "dereference bdev->disk" occurs on this sub-chain:
filemap_write_and_wait()
__filemap_fdatawrite_range()
mapping_cap_writeback_dirty()
inode_to_bdi()
bdev_get_queue()
return bdev->disk->queue;
The problem was introduced by de1414a654e6 ("fs: export inode_to_bdi and use
it in favor of mapping->backing_dev_info"). Before that change,
mapping_cap_writeback_dirty() directly retrieved the backing_dev_info from
the mapping rather than looking it up through
mapping->host->inode_dev->bdev->bd_disk->queue.
This was found while running a stress test on an ARM-based embedded system
which involved repeatedly shutting down many services simultaneously via
systemd isolate (thereby making it likely that "Thread B" was preempted for
awhile just before it dereferenced bdev->bd_disk). I subsequently reproduced
this on vanilla Linux 4.6 in QEMU/x86.
This patch fixes the race by making sd_remove() hold bd_mutex during the
call to del_gendisk().
Fixes: de1414a654e6 ("fs: export inode_to_bdi and use it in favor of
mapping->backing_dev_info")
Signed-off-by: Howard Cochran
Cc: Howard Cochran
Cc: linux-scsi@vger.kernel.org
Cc: Christoph Hellwig
Cc: James Bottomley
Cc: Martin K. Petersen
---
drivers/scsi/sd.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index f52b74c..0f53925 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3126,6 +3126,7 @@ static int sd_remove(struct device *dev)
{
struct scsi_disk *sdkp;
dev_t devt;
+ struct block_device *bdev;
sdkp = dev_get_drvdata(dev);
devt = disk_devt(sdkp->disk);
@@ -3134,7 +3135,13 @@ static int sd_remove(struct device *dev)
async_synchronize_full_domain(&scsi_sd_pm_domain);
async_synchronize_full_domain(&scsi_sd_probe_domain);
device_del(&sdkp->dev);
+
+ bdev = bdget_disk(sdkp->disk, 0);
+ mutex_lock(&bdev->bd_mutex);
del_gendisk(sdkp->disk);
+ mutex_unlock(&bdev->bd_mutex);
+ bdput(bdev);
+
sd_shutdown(dev);
blk_register_region(devt, SD_MINORS, NULL,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [patch] bfa: clean up some bounds checking
Both the approaches (i.e., using a loop or the strchr()) look fine to me.
Thanks,
Sudarsana
-Original Message-
From: walter harms [mailto:wha...@bfs.de]
Sent: 16 June 2016 17:50
To: Dan Carpenter
Cc: Anil Gurumurthy ; Sudarsana Kalluru
; James E.J. Bottomley ;
Martin K. Petersen ; linux-scsi
; kernel-janit...@vger.kernel.org
Subject: Re: [patch] bfa: clean up some bounds checking
Am 16.06.2016 12:44, schrieb Dan Carpenter:
> This code is supposed to search ->adapter_hwpath[] and replace the
> second colon with a NUL character. Unfortunately, the boundary checks
> that ensure we don't go beyond the end of the buffer have a couple
> problems.
>
> Imagine that the string has no colons. In that case, in the first
> loop, we read one space beyond the end of the buffer and then exit the loop.
> In the next loop, we increment once, read two characters beyond the
> end of the buffer and then exit. Then after the loop we put a NUL
> character two characters past the end of the buffer.
>
> Signed-off-by: Dan Carpenter
> ---
> This is from static analysis and not tested. Caveat emptor.
>
> diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c
> index d1ad020..dfb26f0 100644
> --- a/drivers/scsi/bfa/bfad_bsg.c
> +++ b/drivers/scsi/bfa/bfad_bsg.c
> @@ -106,10 +106,17 @@ bfad_iocmd_ioc_get_info(struct bfad_s *bfad,
> void *cmd)
>
> /* set adapter hw path */
> strcpy(iocmd->adapter_hwpath, bfad->pci_name);
> - for (i = 0; iocmd->adapter_hwpath[i] != ':' && i < BFA_STRING_32; i++)
> - ;
> - for (; iocmd->adapter_hwpath[++i] != ':' && i < BFA_STRING_32; )
> - ;
> + i = -1;
> + while (++i < BFA_STRING_32) {
> + if (iocmd->adapter_hwpath[i] == ':')
> + break;
> + }
> + while (++i < BFA_STRING_32) {
> + if (iocmd->adapter_hwpath[i] == ':')
> + break;
> + }
> + if (i >= BFA_STRING_32)
> + i = BFA_STRING_32 - 1;
> iocmd->adapter_hwpath[i] = '\0';
> iocmd->status = BFA_STATUS_OK;
> return 0;
I do not see the use case but i assume
the idea is to have a string like aa:bb:something and kill everyhing after the
second ':' ?
/*
a few word may help here also inside the code */
second: maybe we can us strchr here ?
s1=strchr(iocmd->adapter_hwpath,':');
if (s1 != NULL ) s1=strchr(s1,":");
re,
wh
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: megaraid_sas driver bug
On Sat, Jun 25, 2016 at 10:43 PM, Geza Lucz wrote:
>
> I don’t know if this is the right place to report this and whether this is
> more like a Centos kernel issue.
>
> Since Centos 6.8, the megaraid_sas driver seems to be somewhat broken –
> which as I understand is a backport of the up-to-date driver to their kernel
> version. It is still partially usable, but as soon as I need to rebuild an
> array, instead of the scan message at the end I get a bunch of kernel error
> messages. There are so many of these that the kernel logger uses too much
> CPU and eventually the log fills up the partition, so the server needs to be
> rebooted. So technically I lose all hotplug functionality.
>
> Probably the same thing happens when the array degrades.
>
> I’m sending this report here, because there is absolutely no mention of
> similar problems on the internet, yet the problem is fully reproducible and
> real.
>
> I’m using Dell Perc 6i at the moment.
>
> Thanks
As we discussed, this is regression and found the proposed fix. Can
you try below patch. ?
---
megaraid_sas_base.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/megaraid_sas_base.c b/megaraid_sas_base.c
index d15e252..bee0895 100755
--- a/megaraid_sas_base.c
+++ b/megaraid_sas_base.c
@@ -5309,6 +5309,12 @@ megasas_get_pd_list(struct megasas_instance *instance)
struct MR_PD_ADDRESS *pd_addr;
dma_addr_t ci_h = 0;
+if (instance->pd_list_not_supported) {
+dev_info(&instance->pdev->dev, "MR_DCMD_PD_LIST_QUERY "
+"not supported by firmware\n");
+return ret;
+}
+
cmd = megasas_get_cmd(instance);
if (!cmd) {
--
We have verified on our local setup, but waiting for more testing by
Test team to make sure it is good to post.
Thanks, Kashyap
>
>
> Jun 25 08:49:29 ns8 kernel: [ 757.358015] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.379016] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.400018] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.421021] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.442017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.463015] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.484021] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.505017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:29 ns8 kernel: [ 757.526021] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.547017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.568021] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.589017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.610017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.631017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.652017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.673016] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.694017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.715016] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.736017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.757017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.778017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.799017] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30 ns8 kernel: [ 757.820018] megaraid_sas :02:00.0: DCMD
> failed/not supported by firmware: megasas_get_pd_list 4115
> Jun 25 08:49:30
Re: [PATCH 1/6] lib: string: add function strtolower()
On Fri, 01 Jul 2016, Markus Mayer wrote:
> Add a function called strtolower() to convert strings to lower case
> in-place, overwriting the original string.
>
> This seems to be a recurring requirement in the kernel that is
> currently being solved by several duplicated implementations doing the
> same thing.
>
> Signed-off-by: Markus Mayer
> ---
> include/linux/string.h | 1 +
> lib/string.c | 14 ++
> 2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..aad605e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
> #endif
> void *memchr_inv(const void *s, int c, size_t n);
> char *strreplace(char *s, char old, char new);
> +char *strtolower(char *s);
>
> extern void kfree_const(const void *x);
>
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..6e3b560 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
> return s;
> }
> EXPORT_SYMBOL(strreplace);
> +
This needs a kernel-doc comment right here.
> +char *strtolower(char *s)
> +{
> + char *p;
> +
> +if (unlikely(!s))
> +return NULL;
Using spaces for indentation? See scripts/checkpatch.pl.
> +
> + for (p = s; *p; p++)
> + *p = tolower(*p);
> +
> + return s;
Why does it return a value? Could be void?
BR,
Jani.
> +}
> +EXPORT_SYMBOL(strtolower);
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] libfc: sanity check cpu number extracted from xid
On Thu, Jun 30, 2016 at 08:32:36AM -0700, Chris Leech wrote:
> In the receive path libfc extracts a cpu number from the ox_id in the
> fiber channel header and uses that to do a per_cpu_ptr conversion.
> If, for some reason, a frame is received with an invalid ox_id,
> per_cpu_ptr will return an invalid pointer and the libfc receive path
> will panic the system trying to use it.
>
> I'm currently looking at such a case, and I don't yet know why a
> cpu number > nr_cpu_ids is appearing in an exchange id. But adding a
> sanity check in libfc prevents a system panic, and seems like good idea
> when dealing with frames coming in from the network.
>
> Signed-off-by: Chris Leech
> ---
> drivers/scsi/libfc/fc_exch.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index 30f9ef0..e72673b 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -908,9 +908,17 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr
> *mp, u16 xid)
> {
> struct fc_exch_pool *pool;
> struct fc_exch *ep = NULL;
> + u16 cpu = xid & fc_cpu_mask;
> +
> + if (cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
> + printk_ratelimited(KERN_ERR
> + "libfc: lookup request for XID = %d, "
> + "indicates invalid CPU %d\n", xid, cpu);
> + return NULL;
> + }
>
> if ((xid >= mp->min_xid) && (xid <= mp->max_xid)) {
> - pool = per_cpu_ptr(mp->pool, xid & fc_cpu_mask);
> + pool = per_cpu_ptr(mp->pool, cpu);
> spin_lock_bh(&pool->lock);
> ep = fc_exch_ptr_get(pool, (xid - mp->min_xid) >> fc_cpu_order);
> if (ep) {
Acked-by: Johannes Thumshirn
@Martin, do you queue the libfc patches as well?
--
Johannes Thumshirn Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
