[Open-FCoE] [PATCH 2/2] libfc: fixed some more possible fc_exch_reset() races
michaelc at cs.wisc.edu
Tue Sep 9 15:43:58 UTC 2008
Vasu Dev wrote:
> @@ -553,6 +553,7 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp, u16 xid)
> fc_seq_alloc(ep, ep->seq_id++);
> + fc_exch_hold(ep); /* hold for exch in mp */
> * update exchange
> @@ -567,7 +568,12 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp, u16 xid)
> setup_timer(&ep->ex_timer, fc_exch_timeout, (unsigned long)ep);
> - fc_exch_hold(ep); /* hold for caller */
> + /*
> + * Hold exch lock for caller to prevent
> + * fc_exch_reset() from releasing exch
> + * while caller is still working on exch.
> + */
> + spin_lock_bh(&ep->ex_lock);
I think we need to move the fc_exch_hold and ep initialization (code
after the "update exchange" comment in fc_exch_alloc to before where add
the ep to the mp->ex_list.
I am thinking about if fc_exch_mgr_reset runs right after fc_exch_alloc
mp->exches[xid - min_xid] = ep;
fc_exch_mgr_reset will see the ep in the ex_list and call fc_exch_reset,
but the ep is not fully setup at this time, so when fc_exch_reset tries
to grab the ex_lock we should get an error about using a spin lock that
is not inited.
Do we also need to add a check for FC_EX_RST_CLEANUP in fc_exch_alloc
after we have grabbed the ex_lock in case after fc_exch_alloc dropped
the em_lock, fc_exch_mgr_reset had grabbed the em_lock, seen the ep in
the ex_list and called fc_exch_reset on it?
More information about the devel