fc_port->ema_list is being accessed in multiple parallel contexts with no
locking around it leading to potential corruption. Whats more the fc_find_ema
function was manually accessing ema_list.prev without checking for a list_empty
condition. The ema_list could easily be emtpy if an transation came in to the
foce stack with an UNKNOWN xid prior to our adding a ema anchor to the list
(dev_add_pack gets called before said add occurs).
Fix it by adding an ema_lock to list accesses. Make it an rcu lock so we don't
get Warnings when we try to tx a frame with a spinlock held
Change notes:
v2) Removed pool->lock and ex_lock changes that had incorectly crept into this
commit
Signed-off-by: Neil Horman <nhor...@tuxdriver.com>
CC: kiran.pa...@intel.com
CC: bprak...@broadcom.com
CC: robert.w.l...@intel.com
---
drivers/scsi/libfc/fc_exch.c | 82 ++++++++++++++++++++++++++++++++---------
include/scsi/libfc.h | 2 +
2 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 630291f..3c03c87 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -130,6 +130,7 @@ struct fc_exch_mgr_anchor {
struct list_head ema_list;
struct fc_exch_mgr *mp;
bool (*match)(struct fc_frame *);
+ struct rcu_head rcu;
};
static void fc_exch_rrq(struct fc_exch *);
@@ -801,9 +802,13 @@ static inline struct fc_exch *fc_exch_alloc(struct
fc_lport *lport,
{
struct fc_exch_mgr_anchor *ema;
- list_for_each_entry(ema, &lport->ema_list, ema_list)
- if (!ema->match || ema->match(fp))
+ rcu_read_lock();
+ list_for_each_entry_rcu(ema, &lport->ema_list, ema_list)
+ if (!ema->match || ema->match(fp)) {
+ rcu_read_unlock();
return fc_exch_em_alloc(lport, ema->mp);
+ }
+ rcu_read_unlock();
return NULL;
}
@@ -1329,10 +1334,13 @@ static struct fc_seq *fc_seq_assign(struct fc_lport
*lport, struct fc_frame *fp)
WARN_ON(fr_seq(fp));
fr_seq(fp) = NULL;
- list_for_each_entry(ema, &lport->ema_list, ema_list)
+ rcu_read_lock();
+ list_for_each_entry_rcu(ema, &lport->ema_list, ema_list)
if ((!ema->match || ema->match(fp)) &&
fc_seq_lookup_recip(lport, ema->mp, fp) == FC_RJT_NONE)
break;
+
+ rcu_read_unlock();
return fr_seq(fp);
}
@@ -1817,12 +1825,14 @@ void fc_exch_mgr_reset(struct fc_lport *lport, u32 sid,
u32 did)
struct fc_exch_mgr_anchor *ema;
unsigned int cpu;
- list_for_each_entry(ema, &lport->ema_list, ema_list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(ema, &lport->ema_list, ema_list) {
for_each_possible_cpu(cpu)
fc_exch_pool_reset(lport,
per_cpu_ptr(ema->mp->pool, cpu),
sid, did);
}
+ rcu_read_unlock();
}
EXPORT_SYMBOL(fc_exch_mgr_reset);
@@ -1836,11 +1846,17 @@ EXPORT_SYMBOL(fc_exch_mgr_reset);
static struct fc_exch *fc_exch_lookup(struct fc_lport *lport, u32 xid)
{
struct fc_exch_mgr_anchor *ema;
+ struct fc_exch *exch = NULL;
- list_for_each_entry(ema, &lport->ema_list, ema_list)
- if (ema->mp->min_xid <= xid && xid <= ema->mp->max_xid)
- return fc_exch_find(ema->mp, xid);
- return NULL;
+ rcu_read_lock();
+ list_for_each_entry_rcu(ema, &lport->ema_list, ema_list) {
+ if (ema->mp->min_xid <= xid && xid <= ema->mp->max_xid) {
+ exch = fc_exch_find(ema->mp, xid);
+ break;
+ }
+ }
+ rcu_read_unlock();
+ return exch;
}
/**
@@ -2166,6 +2182,7 @@ struct fc_exch_mgr_anchor *fc_exch_mgr_add(struct
fc_lport *lport,
bool (*match)(struct fc_frame *))
{
struct fc_exch_mgr_anchor *ema;
+ unsigned long flags;
ema = kmalloc(sizeof(*ema), GFP_ATOMIC);
if (!ema)
@@ -2174,8 +2191,10 @@ struct fc_exch_mgr_anchor *fc_exch_mgr_add(struct
fc_lport *lport,
ema->mp = mp;
ema->match = match;
/* add EM anchor to EM anchors list */
- list_add_tail(&ema->ema_list, &lport->ema_list);
+ spin_lock_irqsave(&lport->ema_lock, flags);
+ list_add_tail_rcu(&ema->ema_list, &lport->ema_list);
kref_get(&mp->kref);
+ spin_unlock_irqrestore(&lport->ema_lock, flags);
return ema;
}
EXPORT_SYMBOL(fc_exch_mgr_add);
@@ -2193,16 +2212,26 @@ static void fc_exch_mgr_destroy(struct kref *kref)
kfree(mp);
}
+static void del_ema_rcu_cb(struct rcu_head *head)
+{
+ struct fc_exch_mgr_anchor *ema;
+
+ ema = container_of(head, struct fc_exch_mgr_anchor, rcu);
+
+ kref_put(&ema->mp->kref, fc_exch_mgr_destroy);
+ kfree(ema);
+}
+
/**
* fc_exch_mgr_del() - Delete an EM from a local port's list
* @ema: The exchange manager anchor identifying the EM to be deleted
+ * NOTE: Assumes that the ema_lock for the list this anchor is on is held
*/
void fc_exch_mgr_del(struct fc_exch_mgr_anchor *ema)
{
/* remove EM anchor from EM anchors list */
- list_del(&ema->ema_list);
- kref_put(&ema->mp->kref, fc_exch_mgr_destroy);
- kfree(ema);
+ list_del_rcu(&ema->ema_list);
+ call_rcu(&ema->rcu, del_ema_rcu_cb);
}
EXPORT_SYMBOL(fc_exch_mgr_del);
@@ -2214,15 +2243,21 @@ EXPORT_SYMBOL(fc_exch_mgr_del);
int fc_exch_mgr_list_clone(struct fc_lport *src, struct fc_lport *dst)
{
struct fc_exch_mgr_anchor *ema, *tmp;
+ unsigned long flags;
+ spin_lock_irqsave(&src->ema_lock, flags);
list_for_each_entry(ema, &src->ema_list, ema_list) {
if (!fc_exch_mgr_add(dst, ema->mp, ema->match))
goto err;
}
+ spin_unlock_irqrestore(&src->ema_lock, flags);
return 0;
err:
+ spin_unlock_irqrestore(&src->ema_lock, flags);
+ spin_lock_irqsave(&dst->ema_lock, flags);
list_for_each_entry_safe(ema, tmp, &dst->ema_list, ema_list)
fc_exch_mgr_del(ema);
+ spin_unlock_irqrestore(&dst->ema_lock, flags);
return -ENOMEM;
}
EXPORT_SYMBOL(fc_exch_mgr_list_clone);
@@ -2322,10 +2357,13 @@ EXPORT_SYMBOL(fc_exch_mgr_alloc);
void fc_exch_mgr_free(struct fc_lport *lport)
{
struct fc_exch_mgr_anchor *ema, *next;
+ unsigned long flags;
flush_workqueue(fc_exch_workqueue);
+ spin_lock_irqsave(&lport->ema_lock, flags);
list_for_each_entry_safe(ema, next, &lport->ema_list, ema_list)
fc_exch_mgr_del(ema);
+ spin_unlock_irqrestore(&lport->ema_lock, flags);
}
EXPORT_SYMBOL(fc_exch_mgr_free);
@@ -2340,24 +2378,32 @@ static struct fc_exch_mgr_anchor *fc_find_ema(u32 f_ctl,
struct fc_lport *lport,
struct fc_frame_header *fh)
{
- struct fc_exch_mgr_anchor *ema;
u16 xid;
+ struct fc_exch_mgr_anchor *ema = NULL;
+ rcu_read_lock();
if (f_ctl & FC_FC_EX_CTX)
xid = ntohs(fh->fh_ox_id);
else {
xid = ntohs(fh->fh_rx_id);
- if (xid == FC_XID_UNKNOWN)
- return list_entry(lport->ema_list.prev,
+ if (xid == FC_XID_UNKNOWN) {
+
+ ema = list_empty(&lport->ema_list) ?
+ NULL :
+ list_entry(lport->ema_list.prev,
typeof(*ema), ema_list);
+ goto out;
+ }
}
- list_for_each_entry(ema, &lport->ema_list, ema_list) {
+ list_for_each_entry_rcu(ema, &lport->ema_list, ema_list) {
if ((xid >= ema->mp->min_xid) &&
(xid <= ema->mp->max_xid))
- return ema;
+ goto out;
}
- return NULL;
+out:
+ rcu_read_unlock();
+ return ema;
}
/**
* fc_exch_recv() - Handler for received frames
diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
index 8f9dfba..5627d99 100644
--- a/include/scsi/libfc.h
+++ b/include/scsi/libfc.h
@@ -850,6 +850,7 @@ struct fc_lport {
/* Associations */
struct Scsi_Host *host;
struct list_head ema_list;
+ spinlock_t ema_lock;
struct fc_rport_priv *dns_rdata;
struct fc_rport_priv *ms_rdata;
struct fc_rport_priv *ptp_rdata;
@@ -1024,6 +1025,7 @@ libfc_host_alloc(struct scsi_host_template *sht, int
priv_size)
lport = shost_priv(shost);
lport->host = shost;
INIT_LIST_HEAD(&lport->ema_list);
+ spin_lock_init(&lport->ema_lock);
INIT_LIST_HEAD(&lport->vports);
return lport;
}
--
1.7.7.6
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
