From: Calvin Owens <calvinowens@fb.com>
To: Nagalakshmi Nandigama <nagalakshmi.nandigama@avagotech.com>,
Praveen Krishnamoorthy <praveen.krishnamoorthy@avagotech.com>,
Sreekanth Reddy <sreekanth.reddy@avagotech.com>,
Abhijit Mahajan <abhijit.mahajan@avagotech.com>
Cc: <MPT-FusionLinux.pdl@avagotech.com>, <linux-scsi@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <calvinowens@fb.com>,
<kernel-team@fb.com>
Subject: [PATCH 3/6] Fix unsafe sas_device_list usage
Date: Thu, 14 May 2015 20:41:59 -0700 [thread overview]
Message-ID: <1431661322-3097935-4-git-send-email-calvinowens@fb.com> (raw)
In-Reply-To: <1431661322-3097935-1-git-send-email-calvinowens@fb.com>
We cannot iterate over the list without holding a lock for the entire
duration, or we risk corrupting random memory if items are added or
deleted as we iterate.
This refactors code such that it always holds the lock when iterating
on or accessing the sas_device_list.
Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
drivers/scsi/mpt2sas/mpt2sas_scsih.c | 83 +++++++++++++++++++++++++++---------
1 file changed, 62 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index ad6ceb7e..9645055 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -7104,6 +7104,7 @@ _scsih_remove_unresponding_sas_devices(struct MPT2SAS_ADAPTER *ioc)
struct _raid_device *raid_device, *raid_device_next;
struct list_head tmp_list;
unsigned long flags;
+ LIST_HEAD(head);
printk(MPT2SAS_INFO_FMT "removing unresponding devices: start\n",
ioc->name);
@@ -7111,14 +7112,29 @@ _scsih_remove_unresponding_sas_devices(struct MPT2SAS_ADAPTER *ioc)
/* removing unresponding end devices */
printk(MPT2SAS_INFO_FMT "removing unresponding devices: end-devices\n",
ioc->name);
+
+ /*
+ * Iterate, pulling off devices marked as non-responding. We become the
+ * owner for the reference the list had on any object we prune.
+ */
+ spin_lock_irqsave(&ioc->sas_device_lock, flags);
list_for_each_entry_safe(sas_device, sas_device_next,
- &ioc->sas_device_list, list) {
+ &ioc->sas_device_list, list) {
if (!sas_device->responding)
- mpt2sas_device_remove_by_sas_address(ioc,
- sas_device->sas_address);
+ list_move_tail(&sas_device->list, &head);
else
sas_device->responding = 0;
}
+ spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
+
+ /*
+ * Now, uninitialize and remove the unresponding devices we pruned.
+ */
+ list_for_each_entry_safe(sas_device, sas_device_next, &head, list) {
+ _scsih_remove_device(ioc, sas_device);
+ list_del_init(&sas_device->list);
+ sas_device_put(sas_device);
+ }
/* removing unresponding volumes */
if (ioc->ir_firmware) {
@@ -8055,6 +8071,37 @@ _scsih_probe_raid(struct MPT2SAS_ADAPTER *ioc)
}
}
+static struct _sas_device *dequeue_next_sas_device(struct MPT2SAS_ADAPTER *ioc)
+{
+ struct _sas_device *sas_device = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioc->sas_device_lock, flags);
+ if (!list_empty(&ioc->sas_device_init_list)) {
+ sas_device = list_first_entry(&ioc->sas_device_init_list,
+ struct _sas_device, list);
+ list_del_init(&sas_device->list);
+ }
+ spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
+
+ /*
+ * If an item was dequeued, the caller now owns the reference that was
+ * previously owned by the list
+ */
+ return sas_device;
+}
+
+static void sas_device_make_active(struct MPT2SAS_ADAPTER *ioc,
+ struct _sas_device *sas_device)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioc->sas_device_lock, flags);
+ sas_device_get(sas_device);
+ list_add_tail(&sas_device->list, &ioc->sas_device_list);
+ spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
+}
+
/**
* _scsih_probe_sas - reporting sas devices to sas transport
* @ioc: per adapter object
@@ -8064,34 +8111,28 @@ _scsih_probe_raid(struct MPT2SAS_ADAPTER *ioc)
static void
_scsih_probe_sas(struct MPT2SAS_ADAPTER *ioc)
{
- struct _sas_device *sas_device, *next;
- unsigned long flags;
-
- /* SAS Device List */
- list_for_each_entry_safe(sas_device, next, &ioc->sas_device_init_list,
- list) {
+ struct _sas_device *sas_device;
- if (ioc->hide_drives)
- continue;
+ if (ioc->hide_drives)
+ return;
+ while ((sas_device = dequeue_next_sas_device(ioc))) {
if (!mpt2sas_transport_port_add(ioc, sas_device->handle,
- sas_device->sas_address_parent)) {
- list_del(&sas_device->list);
- kfree(sas_device);
+ sas_device->sas_address_parent)) {
+ sas_device_put(sas_device);
continue;
} else if (!sas_device->starget) {
if (!ioc->is_driver_loading) {
mpt2sas_transport_port_remove(ioc,
- sas_device->sas_address,
- sas_device->sas_address_parent);
- list_del(&sas_device->list);
- kfree(sas_device);
+ sas_device->sas_address,
+ sas_device->sas_address_parent);
+ sas_device_put(sas_device);
continue;
}
}
- spin_lock_irqsave(&ioc->sas_device_lock, flags);
- list_move_tail(&sas_device->list, &ioc->sas_device_list);
- spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
+
+ sas_device_make_active(ioc, sas_device);
+ sas_device_put(sas_device);
}
}
--
1.8.1
next prev parent reply other threads:[~2015-05-15 3:42 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-04 15:05 [PATCH] mpt2sas: mpt3sas: Fix memory corruption during initialization Sreekanth Reddy
2015-05-05 15:35 ` Tomas Henzl
2015-05-12 9:38 ` Sreekanth Reddy
2015-05-06 18:48 ` Calvin Owens
2015-05-15 3:41 ` [PATCH 0/6] Fixes for memory corruption in mpt2sas Calvin Owens
2015-05-15 3:41 ` [PATCH 1/6] Add refcount to sas_device struct Calvin Owens
2015-05-15 3:41 ` [PATCH 2/6] Refactor code to use new sas_device refcount Calvin Owens
2015-05-15 3:41 ` Calvin Owens [this message]
2015-05-15 3:42 ` [PATCH 4/6] Add refcount to fw_event_work struct Calvin Owens
2015-05-15 3:42 ` [PATCH 5/6] Refactor code to use new fw_event refcount Calvin Owens
2015-05-15 3:42 ` [PATCH 6/6] Fix unsafe fw_event_list usage Calvin Owens
2015-06-09 3:50 ` [RESEND][PATCH 0/6] Fixes for memory corruption in mpt2sas Calvin Owens
2015-06-09 3:50 ` [PATCH 1/6] Add refcount to sas_device struct Calvin Owens
2015-07-03 15:24 ` Christoph Hellwig
2015-06-09 3:50 ` [PATCH 2/6] Refactor code to use new sas_device refcount Calvin Owens
2015-07-03 15:38 ` Christoph Hellwig
2015-07-12 4:15 ` Calvin Owens
2015-06-09 3:50 ` [PATCH 3/6] Fix unsafe sas_device_list usage Calvin Owens
2015-07-03 16:03 ` Christoph Hellwig
2015-06-09 3:50 ` [PATCH 4/6] Add refcount to fw_event_work struct Calvin Owens
2015-07-03 15:38 ` Christoph Hellwig
2015-06-09 3:50 ` [PATCH 5/6] Refactor code to use new fw_event refcount Calvin Owens
2015-07-03 16:00 ` Christoph Hellwig
2015-07-12 4:13 ` Calvin Owens
2015-06-09 3:50 ` [PATCH 6/6] Fix unsafe fw_event_list usage Calvin Owens
2015-07-03 16:02 ` Christoph Hellwig
2015-07-12 4:20 ` Calvin Owens
2015-07-02 20:15 ` [RESEND][PATCH 0/6] Fixes for memory corruption in mpt2sas Bart Van Assche
2015-07-12 4:24 ` [PATCH 0/2 v2] " Calvin Owens
2015-07-12 4:24 ` [PATCH 1/2] mpt2sas: Refcount sas_device objects and fix unsafe list usage Calvin Owens
2015-07-13 6:52 ` Christoph Hellwig
2015-07-21 7:06 ` Calvin Owens
2015-07-13 15:05 ` Joe Lawrence
2015-07-21 7:04 ` Calvin Owens
2015-07-16 14:57 ` Sreekanth Reddy
2015-07-21 7:03 ` Calvin Owens
2015-07-12 4:24 ` [PATCH 2/2] mpt2sas: Refcount fw_events " Calvin Owens
2015-07-13 6:52 ` Christoph Hellwig
2015-08-01 5:02 ` [PATCH v3 0/2] Fixes for memory corruption in mpt2sas Calvin Owens
2015-08-01 5:02 ` [PATCH v3 1/2] mpt2sas: Refcount sas_device objects and fix unsafe list usage Calvin Owens
2015-08-10 13:15 ` Sreekanth Reddy
2015-08-14 1:43 ` Calvin Owens
2015-08-01 5:02 ` [PATCH v3 2/2] mpt2sas: Refcount fw_events " Calvin Owens
2015-08-14 1:48 ` [PATCH v4 0/2] Fixes for memory corruption in mpt2sas Calvin Owens
2015-08-14 1:48 ` [PATCH v4 1/2] mpt2sas: Refcount sas_device objects and fix unsafe list usage Calvin Owens
2015-08-14 1:48 ` [PATCH v4 2/2] mpt2sas: Refcount fw_events " Calvin Owens
2015-08-25 21:06 ` Nicholas A. Bellinger
2015-09-04 14:35 ` Sreekanth Reddy
2015-08-25 21:03 ` [PATCH v4 1/2] mpt2sas: Refcount sas_device objects " Nicholas A. Bellinger
2015-09-04 14:34 ` Sreekanth Reddy
2015-08-25 21:21 ` [PATCH v4 0/2] Fixes for memory corruption in mpt2sas Nicholas A. Bellinger
2015-07-02 19:22 ` [PATCH 0/6] " Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1431661322-3097935-4-git-send-email-calvinowens@fb.com \
--to=calvinowens@fb.com \
--cc=MPT-FusionLinux.pdl@avagotech.com \
--cc=abhijit.mahajan@avagotech.com \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=nagalakshmi.nandigama@avagotech.com \
--cc=praveen.krishnamoorthy@avagotech.com \
--cc=sreekanth.reddy@avagotech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®