On Sun, May 17, 2015 at 08:51:01AM +0300, Haggai Eran wrote: > @@ -212,6 +212,8 @@ struct cm_id_private { > spinlock_t lock; /* Do not acquire inside cm.lock */ > struct completion comp; > atomic_t refcount; > + /* Number of clients sharing this ib_cm_id. Only valid for listeners. */ > + atomic_t sharecount; No need for this atomic, hold the lock The use of the atomic looks racy: > + if (!atomic_dec_and_test(&cm_id_priv->sharecount)) { > + /* The id is still shared. */ > + return; > + } Might race with this: > + if (atomic_inc_return(&cm_id_priv->sharecount) == 1) { > + /* This ID is already being destroyed */ > + atomic_dec(&cm_id_priv->sharecount); > + goto new_id; > + } > + Resulting in use-after-free of cm_id_priv->sharecount Don't try and be clever with atomics, it is almost always wrong. The share count should be 'listen_sharecount' because it *only* works for listen. The above test in cm_destroy_id should only be in the listen branch of the if. > + * Create a new listening ib_cm_id and listen on the given service ID. > + * > + * If there's an existing ID listening on that same device and service ID, > + * return it. > + * .. Callers should call cm_destroy_id when done with the listen .. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html