On 2/2/2012 1:03 PM, Neil Horman wrote:
> 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
> ---
[...]
>
> @@ -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);
I think you mean list_entry_rcu here?
> + 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;
> }
I know your working up a v3 so I'll take a look at that when I see it.
Thanks,
John
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
