Print this page
Running stmfadm remove-hg-member caused a NULL pointer dereference panic in stmf_remove_lu_from_session
Reviewed by: Dan Fields <dan.fields@nexenta.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Running stmfadm remove-hg-member caused a NULL pointer dereference panic in stmf_remove_lu_from_session
Reviewed by: Dan Fields <dan.fields@nexenta.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
SUP-770 deadlock between thread acquiring iss->iss_lockp in stmf_task_free() and thread holding sl->sl_pgr->pgr_lock from sbd_pgr_remove_it_handle()
Reviewed by: Rick McNeal <rick.mcneal@nexenta.com>
@@ -45,12 +45,13 @@
void stmf_update_sessions_per_ve(stmf_view_entry_t *ve,
stmf_lu_t *lu, int action);
void stmf_add_lus_to_session_per_vemap(stmf_i_local_port_t *ilport,
stmf_i_scsi_session_t *iss, stmf_lun_map_t *vemap);
stmf_id_data_t *stmf_lookup_group_for_host(uint8_t *ident, uint16_t ident_size);
-stmf_status_t stmf_add_ent_to_map(stmf_lun_map_t *sm, void *ent, uint8_t *lun);
-stmf_status_t stmf_remove_ent_from_map(stmf_lun_map_t *sm, uint8_t *lun);
+static stmf_status_t stmf_add_ent_to_map(stmf_lun_map_t *sm, void *ent,
+ uint8_t *lun);
+static stmf_status_t stmf_remove_ent_from_map(stmf_lun_map_t *sm, uint8_t *lun);
uint16_t stmf_get_next_free_lun(stmf_lun_map_t *sm, uint8_t *lun);
stmf_status_t stmf_add_tg(uint8_t *tg_name, uint16_t tg_name_size,
int allow_special, uint32_t *err_detail);
stmf_status_t stmf_add_hg(uint8_t *hg_name, uint16_t hg_name_size,
int allow_special, uint32_t *err_detail);
@@ -177,10 +178,11 @@
}
}
/*
* Create luns map for session based on the view
+ * iss_lockp is held
*/
stmf_status_t
stmf_session_create_lun_map(stmf_i_local_port_t *ilport,
stmf_i_scsi_session_t *iss)
{
@@ -234,56 +236,12 @@
return (STMF_SUCCESS);
}
/*
- * destroy lun map for session
- */
-/* ARGSUSED */
-stmf_status_t
-stmf_session_destroy_lun_map(stmf_i_local_port_t *ilport,
- stmf_i_scsi_session_t *iss)
-{
- stmf_lun_map_t *sm;
- stmf_i_lu_t *ilu;
- uint16_t n;
- stmf_lun_map_ent_t *ent;
-
- ASSERT(mutex_owned(&stmf_state.stmf_lock));
- /*
- * to avoid conflict with updating session's map,
- * which only grab stmf_lock
- */
- sm = iss->iss_sm;
- iss->iss_sm = NULL;
- iss->iss_hg = NULL;
- if (sm->lm_nentries) {
- for (n = 0; n < sm->lm_nentries; n++) {
- if ((ent = (stmf_lun_map_ent_t *)sm->lm_plus[n])
- != NULL) {
- if (ent->ent_itl_datap) {
- stmf_do_itl_dereg(ent->ent_lu,
- ent->ent_itl_datap,
- STMF_ITL_REASON_IT_NEXUS_LOSS);
- }
- ilu = (stmf_i_lu_t *)
- ent->ent_lu->lu_stmf_private;
- atomic_dec_32(&ilu->ilu_ref_cnt);
- kmem_free(sm->lm_plus[n],
- sizeof (stmf_lun_map_ent_t));
- }
- }
- kmem_free(sm->lm_plus,
- sizeof (stmf_lun_map_ent_t *) * sm->lm_nentries);
- }
-
- kmem_free(sm, sizeof (*sm));
- return (STMF_SUCCESS);
-}
-
-/*
* Expects the session lock to be held.
+ * iss_lockp is held
*/
stmf_xfer_data_t *
stmf_session_prepare_report_lun_data(stmf_lun_map_t *sm)
{
stmf_xfer_data_t *xd;
@@ -388,12 +346,13 @@
break;
}
}
/*
* add lu to a session, stmf_lock is already held
+ * iss_lockp/ilport_lock already held
*/
-stmf_status_t
+static stmf_status_t
stmf_add_lu_to_session(stmf_i_local_port_t *ilport,
stmf_i_scsi_session_t *iss,
stmf_lu_t *lu,
uint8_t *lu_nbr)
{
@@ -432,17 +391,15 @@
return (STMF_SUCCESS);
}
/*
* remvoe lu from a session, stmf_lock is already held
+ * iss_lockp held
*/
-/* ARGSUSED */
-stmf_status_t
-stmf_remove_lu_from_session(stmf_i_local_port_t *ilport,
- stmf_i_scsi_session_t *iss,
- stmf_lu_t *lu,
- uint8_t *lu_nbr)
+static void
+stmf_remove_lu_from_session(stmf_i_scsi_session_t *iss,
+ stmf_lu_t *lu, uint8_t *lu_nbr)
{
stmf_status_t ret;
stmf_i_lu_t *ilu;
stmf_lun_map_t *sm = iss->iss_sm;
stmf_lun_map_ent_t *lun_map_ent;
@@ -449,11 +406,14 @@
uint16_t luNbr =
((uint16_t)lu_nbr[1] | (((uint16_t)(lu_nbr[0] & 0x3F)) << 8));
ASSERT(mutex_owned(&stmf_state.stmf_lock));
lun_map_ent = stmf_get_ent_from_map(sm, luNbr);
- ASSERT(lun_map_ent && lun_map_ent->ent_lu == lu);
+ ASSERT(lun_map_ent->ent_lu == lu);
+ if (lun_map_ent == NULL) {
+ return;
+ }
ilu = (stmf_i_lu_t *)lu->lu_stmf_private;
ret = stmf_remove_ent_from_map(sm, lu_nbr);
ASSERT(ret == STMF_SUCCESS);
@@ -462,11 +422,10 @@
if (lun_map_ent->ent_itl_datap) {
stmf_do_itl_dereg(lu, lun_map_ent->ent_itl_datap,
STMF_ITL_REASON_USER_REQUEST);
}
kmem_free((void *)lun_map_ent, sizeof (stmf_lun_map_ent_t));
- return (STMF_SUCCESS);
}
/*
* add or remove lu from all related sessions based on view entry,
* action is 0 for delete, 1 for add
@@ -517,12 +476,12 @@
iss = iss->iss_next) {
if (!all_hg && iss->iss_hg != ve->ve_hg)
continue;
/* This host belongs to the host group */
if (action == 0) { /* to remove */
- (void) stmf_remove_lu_from_session(ilport, iss,
- lu_to_add, ve->ve_lun);
+ stmf_remove_lu_from_session(iss, lu_to_add,
+ ve->ve_lun);
if (ilu_tmp->ilu_ref_cnt == 0) {
rw_exit(&ilport->ilport_lock);
return;
}
} else {
@@ -547,11 +506,10 @@
stmf_i_lu_t *ilu;
stmf_view_entry_t *ve;
uint32_t i;
ASSERT(mutex_owned(&stmf_state.stmf_lock));
-
for (i = 0; i < vemap->lm_nentries; i++) {
ve = (stmf_view_entry_t *)vemap->lm_plus[i];
if (!ve)
continue;
ilu = (stmf_i_lu_t *)ve->ve_luid->id_pt_to_object;
@@ -560,14 +518,16 @@
(void) stmf_add_lu_to_session(ilport, iss, lu,
ve->ve_lun);
}
}
}
-/* remove luns in view entry map from a session */
+/*
+ * remove luns in view entry map from a session
+ * iss_lockp held
+ */
void
-stmf_remove_lus_from_session_per_vemap(stmf_i_local_port_t *ilport,
- stmf_i_scsi_session_t *iss,
+stmf_remove_lus_from_session_per_vemap(stmf_i_scsi_session_t *iss,
stmf_lun_map_t *vemap)
{
stmf_lu_t *lu;
stmf_i_lu_t *ilu;
stmf_view_entry_t *ve;
@@ -580,12 +540,11 @@
if (!ve)
continue;
ilu = (stmf_i_lu_t *)ve->ve_luid->id_pt_to_object;
if (ilu && ilu->ilu_state == STMF_STATE_ONLINE) {
lu = ilu->ilu_lu;
- (void) stmf_remove_lu_from_session(ilport, iss, lu,
- ve->ve_lun);
+ stmf_remove_lu_from_session(iss, lu, ve->ve_lun);
}
}
}
stmf_id_data_t *
@@ -708,10 +667,11 @@
/*
* The refcnts of objects in a view entry are updated when then entry
* is successfully added. ve_map is just another representation of the
* view enrtries in a LU. Duplicating or merging a ve map does not
* affect any refcnts.
+ * stmf_state.stmf_lock held
*/
stmf_lun_map_t *
stmf_duplicate_ve_map(stmf_lun_map_t *src)
{
stmf_lun_map_t *dst;
@@ -740,10 +700,14 @@
kmem_free(dst->lm_plus, dst->lm_nentries * sizeof (void *));
}
kmem_free(dst, sizeof (*dst));
}
+/*
+ * stmf_state.stmf_lock held. Operations are stmf global in nature and
+ * not session level.
+ */
int
stmf_merge_ve_map(stmf_lun_map_t *src, stmf_lun_map_t *dst,
stmf_lun_map_t **pp_ret_map, stmf_merge_flags_t mf)
{
int i;
@@ -1064,11 +1028,15 @@
stmf_free_id(luid);
}
return (ret);
}
-stmf_status_t
+/*
+ * protected by stmf_state.stmf_lock when working on global lun map.
+ * iss_lockp when working at the session level.
+ */
+static stmf_status_t
stmf_add_ent_to_map(stmf_lun_map_t *lm, void *ent, uint8_t *lun)
{
uint16_t n;
if (((lun[0] & 0xc0) >> 6) != 0)
return (STMF_FAILURE);
@@ -1096,11 +1064,15 @@
goto try_again_to_add;
}
}
-stmf_status_t
+/*
+ * iss_lockp held when working on a session.
+ * stmf_state.stmf_lock is held when working on the global views.
+ */
+static stmf_status_t
stmf_remove_ent_from_map(stmf_lun_map_t *lm, uint8_t *lun)
{
uint16_t n, i;
uint8_t lutype = (lun[0] & 0xc0) >> 6;
if (lutype != 0)
@@ -1133,10 +1105,13 @@
}
return (STMF_SUCCESS);
}
+/*
+ * stmf_state.stmf_lock held
+ */
uint16_t
stmf_get_next_free_lun(stmf_lun_map_t *sm, uint8_t *lun)
{
uint16_t luNbr;
@@ -1156,10 +1131,14 @@
}
return (luNbr);
}
+/*
+ * stmf_state.stmf_lock is held when working on global view map
+ * iss_lockp (RW_WRITER) is held when working on session map.
+ */
void *
stmf_get_ent_from_map(stmf_lun_map_t *sm, uint16_t lun_num)
{
if ((lun_num & 0xC000) == 0) {
if (sm->lm_nentries > lun_num)
@@ -1490,19 +1469,21 @@
entry_ident, entry_size);
if (iss) {
stmf_id_data_t *tgid;
iss->iss_hg = (void *)id_grp;
tgid = ilport->ilport_tg;
+ rw_enter(iss->iss_lockp, RW_WRITER);
if (tgid) {
vemap = stmf_get_ve_map_per_ids(tgid, id_grp);
if (vemap)
stmf_add_lus_to_session_per_vemap(
ilport, iss, vemap);
}
if (vemap_alltgt)
stmf_add_lus_to_session_per_vemap(ilport,
iss, vemap_alltgt);
+ rw_exit(iss->iss_lockp);
}
}
return (0);
}
@@ -1575,21 +1556,23 @@
continue;
iss = stmf_lookup_session_for_hostident(ilport,
entry_ident, entry_size);
if (iss) {
stmf_id_data_t *tgid;
+ rw_enter(iss->iss_lockp, RW_WRITER);
iss->iss_hg = NULL;
tgid = ilport->ilport_tg;
if (tgid) {
vemap = stmf_get_ve_map_per_ids(tgid, id_grp);
if (vemap)
stmf_remove_lus_from_session_per_vemap(
- ilport, iss, vemap);
+ iss, vemap);
}
if (vemap_alltgt)
- stmf_remove_lus_from_session_per_vemap(ilport,
- iss, vemap_alltgt);
+ stmf_remove_lus_from_session_per_vemap(iss,
+ vemap_alltgt);
+ rw_exit(iss->iss_lockp);
}
}
return (0);
}