Bart Van Assche wrote:

+
+       switch (event->event) {
+       case IB_EVENT_PORT_ERR:
+               list_for_each_entry_safe(host, tmp_host,
+                                        &srp_dev->dev_list, list) {
+                       if (event->element.port_num == host->port) {
+                               spin_lock(&host->target_lock);

Can srp_remove_work() be executed concurrently with
srp_event_handler() ? In that case the above code isn't safe and the
spin_lock(&host->target_lock) should be moved to just before
list_for_each_entry_safe(). The current implementation can trigger
reading deallocated memory.

+                               list_for_each_entry_safe(target, tmp_target,
+                                                        &host->target_list, 
list) {
+                                       unsigned long flags;
+
+                                       
spin_lock_irqsave(target->scsi_host->host_lock,
+                                                         flags);
+                                       if (!target->qp_in_error &&
+                                           target->state == SRP_TARGET_LIVE)
+                                               srp_qp_err_add_timer(target,
+                                                                    
srp_dev_loss_tmo);
+                                       
spin_unlock_irqrestore(target->scsi_host->host_lock,
+                                                              flags);
+                               }
+                               spin_unlock(&host->target_lock);
+                       }
+               }
+               break;
+       case IB_EVENT_PORT_ACTIVE:
+       case IB_EVENT_LID_CHANGE:
+       case IB_EVENT_PKEY_CHANGE:
+       case IB_EVENT_SM_CHANGE:
+               list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list,
+                                        list) {
+                       if (event->element.port_num == host->port) {
+                               spin_lock(&host->target_lock);

A remark similar to the above: this code isn't safe against concurrent
calls of srp_remove_one().

I did not pay attention to the races created by unloading module (same for patch 3/4). I'll rework patches and address these races.

thanks,
-vu
--
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

Reply via email to