iwcm: rem_ref() - don't touch cm_id after deref.
PATCH 1/1 iwcm: rem_ref() - don't touch cm_id after deref.
rem_ref() calls iwcm_deref_id(), which will wake up any blockers on cm_id_priv->destroy_comp if the refcnt hits 0. That will unblock someone in iw_destroy_cm_id() which will free the cmid. If that happens before rem_ref() calls test_bit(IWCM_F_CALLBACK_DESTROY, &cm_id_priv->flags), then the test_bit() will touch freed memory.
The fix is to read the bit first, then deref.
---
drivers/infiniband/core/iwcm.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
drivers/infiniband/core/iwcm.c +9 -2
@@ -181,9 +181,16 @@ static void add_ref(struct iw_cm_id *cm_id)static void rem_ref (struct iw_cm_id *cm_id) {struct iwcm_id_private *cm_id_priv; +int cb_destroy; + cm_id_priv = container_of(cm_id, struct iwcm_id_private, id); -if (iwcm_deref_id (cm_id_priv) && -test_bit (IWCM_F_CALLBACK_DESTROY , &cm_id_priv->flags)) { + +/* +* Test bit before deref in case the cm_id gets freed on another +* thread. +*/ + cb_destroy =test_bit (IWCM_F_CALLBACK_DESTROY , &cm_id_priv->flags); +if (iwcm_deref_id (cm_id_priv) && cb_destroy) {BUG_ON (!list_empty (&cm_id_priv->work_list));free_cm_id (cm_id_priv); }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20131121214014.27527.79925.stgit-T4OLL4TyM9aNDNWfRnPdfg@public.gmane.org
drivers/infiniband/core/iwcm.c static void add_ref(struct iw_cm_id *cm_id) · line 186
Quoted 8 lines · Steve Wise
static void
rem_ref(struct iw_cm_id *cm_id){ struct iwcm_id_private *cm_id_priv;drivers/infiniband/core/iwcm.c +2
-2+int cb_destroy; + cm_id_priv = container_of(cm_id, struct iwcm_id_private, id); -if (iwcm_deref_id (cm_id_priv) && -test_bit (IWCM_F_CALLBACK_DESTROY , &cm_id_priv->flags)) {
Don't we have bigger problems if iwcm_deref_id() returns 1 (meaning this thread is the one dropping the last reference) but then another thread frees the cm_id_priv? For one thing we go on to call free_cm_id() which does kfree(cm_id_priv), which is no good if another thread has already freed it.
Is the point that CALLBACK_DESTROY will never be set in that case? OIC there is a BUG_ON iw_destroy_cm_id, got it.
Looks good.
drivers/infiniband/core/iwcm.c static void add_ref(struct iw_cm_id *cm_id) · line 196
Quoted 10 lines
drivers/infiniband/core/iwcm.c +7
-0+ +/* +* Test bit before deref in case the cm_id gets freed on another +* thread. +*/ + cb_destroy =test_bit (IWCM_F_CALLBACK_DESTROY , &cm_id_priv->flags); +if (iwcm_deref_id (cm_id_priv) && cb_destroy) {BUG_ON (!list_empty (&cm_id_priv->work_list));free_cm_id (cm_id_priv); }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html