[PATCH 2/2] scsi: Fix RCU handling for VPD pages
From: Alexander Duyck
Date: Thu Jan 21 2016 - 01:35:39 EST
This patch is meant to fix the RCU handling for VPD pages. The original
code had a number of issues including the fact that the local variables
were being declared as __rcu, the RCU variable being directly accessed
outside of the RCU locked region, and the fact that length was not
associated with the data so it would be possible to get a mix and match of
the length for one VPD page with the data from another.
Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
Signed-off-by: Alexander Duyck <aduyck@xxxxxxxxxxxx>
---
drivers/scsi/scsi.c | 52 +++++++++++++++++++++++---------------------
drivers/scsi/scsi_lib.c | 12 +++++-----
drivers/scsi/scsi_sysfs.c | 14 +++++++-----
include/scsi/scsi_device.h | 14 ++++++++----
4 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index ed085e78c893..143b384fd145 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -782,7 +782,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
int vpd_len = SCSI_VPD_PG_LEN;
int pg80_supported = 0;
int pg83_supported = 0;
- unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
+ unsigned char *vpd_buf;
if (sdev->scsi_level < SCSI_3)
return;
@@ -816,58 +816,60 @@ retry_pg0:
vpd_len = SCSI_VPD_PG_LEN;
if (pg80_supported) {
+ struct scsi_vpd_pg *vpd, *orig_vpd;
retry_pg80:
- vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
- if (!vpd_buf)
+ vpd = kmalloc(sizeof(*vpd) + vpd_len, GFP_KERNEL);
+ if (!vpd)
return;
- result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
+ result = scsi_vpd_inquiry(sdev, vpd->buf, 0x80, vpd_len);
if (result < 0) {
- kfree(vpd_buf);
+ kfree(vpd);
return;
}
if (result > vpd_len) {
vpd_len = result;
- kfree(vpd_buf);
+ kfree(vpd);
goto retry_pg80;
}
+ vpd->len = result;
+
mutex_lock(&sdev->inquiry_mutex);
- orig_vpd_buf = sdev->vpd_pg80;
- sdev->vpd_pg80_len = result;
- rcu_assign_pointer(sdev->vpd_pg80, vpd_buf);
+ orig_vpd = rcu_dereference_protected(sdev->vpd_pg80, 1);
+ rcu_assign_pointer(sdev->vpd_pg80, vpd);
mutex_unlock(&sdev->inquiry_mutex);
- synchronize_rcu();
- if (orig_vpd_buf) {
- kfree(orig_vpd_buf);
- orig_vpd_buf = NULL;
- }
+
+ if (orig_vpd)
+ kfree_rcu(orig_vpd, rcu);
vpd_len = SCSI_VPD_PG_LEN;
}
if (pg83_supported) {
+ struct scsi_vpd_pg *vpd, *orig_vpd;
retry_pg83:
- vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
- if (!vpd_buf)
+ vpd = kmalloc(sizeof(*vpd) + vpd_len, GFP_KERNEL);
+ if (!vpd)
return;
- result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
+ result = scsi_vpd_inquiry(sdev, vpd->buf, 0x83, vpd_len);
if (result < 0) {
- kfree(vpd_buf);
+ kfree(vpd);
return;
}
if (result > vpd_len) {
vpd_len = result;
- kfree(vpd_buf);
+ kfree(vpd);
goto retry_pg83;
}
+ vpd->len = result;
+
mutex_lock(&sdev->inquiry_mutex);
- orig_vpd_buf = sdev->vpd_pg83;
- sdev->vpd_pg83_len = result;
- rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
+ orig_vpd = rcu_dereference_protected(sdev->vpd_pg83, 1);
+ rcu_assign_pointer(sdev->vpd_pg83, vpd);
mutex_unlock(&sdev->inquiry_mutex);
- synchronize_rcu();
- if (orig_vpd_buf)
- kfree(orig_vpd_buf);
+
+ if (orig_vpd)
+ kfree_rcu(orig_vpd, rcu);
}
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index fa6b2c4eb7a2..e44f66bc4c90 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3175,7 +3175,7 @@ int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
u8 cur_id_type = 0xff;
u8 cur_id_size = 0;
unsigned char *d, *cur_id_str;
- unsigned char __rcu *vpd_pg83;
+ struct scsi_vpd_pg *vpd_pg83;
int id_size = -EINVAL;
rcu_read_lock();
@@ -3205,8 +3205,8 @@ int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
}
memset(id, 0, id_len);
- d = vpd_pg83 + 4;
- while (d < vpd_pg83 + sdev->vpd_pg83_len) {
+ d = vpd_pg83->buf + 4;
+ while (d < vpd_pg83->buf + vpd_pg83->len) {
/* Skip designators not referring to the LUN */
if ((d[1] & 0x30) != 0x00)
goto next_desig;
@@ -3308,7 +3308,7 @@ EXPORT_SYMBOL(scsi_vpd_lun_id);
int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
{
unsigned char *d;
- unsigned char __rcu *vpd_pg83;
+ struct scsi_vpd_pg *vpd_pg83;
int group_id = -EAGAIN, rel_port = -1;
rcu_read_lock();
@@ -3318,8 +3318,8 @@ int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
return -ENXIO;
}
- d = sdev->vpd_pg83 + 4;
- while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
+ d = vpd_pg83->buf + 4;
+ while (d < vpd_pg83->buf + vpd_pg83->len) {
switch (d[1] & 0xf) {
case 0x4:
/* Relative target port */
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 4f18a851e2c7..061c6ec539dc 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -761,13 +761,15 @@ show_vpd_##_page(struct file *filp, struct kobject *kobj, \
{ \
struct device *dev = container_of(kobj, struct device, kobj); \
struct scsi_device *sdev = to_scsi_device(dev); \
- int ret; \
- if (!sdev->vpd_##_page) \
- return -EINVAL; \
+ struct scsi_vpd_pg *vpd_pg; \
+ ssize_t ret = -EINVAL; \
+ \
rcu_read_lock(); \
- ret = memory_read_from_buffer(buf, count, &off, \
- rcu_dereference(sdev->vpd_##_page), \
- sdev->vpd_##_page##_len); \
+ vpd_pg = rcu_dereference(sdev->vpd_##_page); \
+ if (vpd_pg) \
+ ret = memory_read_from_buffer(buf, count, &off, \
+ vpd_pg->buf, \
+ vpd_pg->len); \
rcu_read_unlock(); \
return ret; \
} \
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index f63a16760ae9..073a411c18ae 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -74,6 +74,13 @@ struct scsi_event {
*/
};
+#define SCSI_VPD_PG_LEN 255
+struct scsi_vpd_pg {
+ struct rcu_head rcu;
+ int len;
+ unsigned char buf[0];
+};
+
struct scsi_device {
struct Scsi_Host *host;
struct request_queue *request_queue;
@@ -116,11 +123,8 @@ struct scsi_device {
const char * model; /* ... after scan; point to static string */
const char * rev; /* ... "nullnullnullnull" before scan */
-#define SCSI_VPD_PG_LEN 255
- int vpd_pg83_len;
- unsigned char __rcu *vpd_pg83;
- int vpd_pg80_len;
- unsigned char __rcu *vpd_pg80;
+ struct scsi_vpd_pg __rcu *vpd_pg80;
+ struct scsi_vpd_pg __rcu *vpd_pg83;
unsigned char current_tag; /* current tag */
struct scsi_target *sdev_target; /* used only for single_lun */