On Wed, Sep 09, 2020 at 10:08:55AM -0700, Brian Bunker wrote:
> Would it be better to move the unsetting the address of sdev to NULL lower? This would protect
> against the crash we see when the alua_rtpg function tries to access the sdev address
> that has been set to NULL in alua_bus_detach by another thread.
>
> --- a/linux-5.4.17/drivers/scsi/device_handler/scsi_dh_alua.c 2020-07-29 22:48:30.000000000 -0600
> +++ b/linux-5.4.17/drivers/scsi/device_handler/scsi_dh_alua.c 2020-09-07 13:38:23.771575702 -0600
> @@ -1146,15 +1146,15 @@
>
> spin_lock(&h->pg_lock);
> pg = rcu_dereference_protected(h->pg, lockdep_is_held(&h->pg_lock));
> - rcu_assign_pointer(h->pg, NULL);
> - h->sdev = NULL;
> - spin_unlock(&h->pg_lock);
> if (pg) {
> spin_lock_irq(&pg->lock);
> list_del_rcu(&h->node);
> spin_unlock_irq(&pg->lock);
> kref_put(&pg->kref, release_port_group);
> }
> + rcu_assign_pointer(h->pg, NULL);
> + h->sdev = NULL;
> + spin_unlock(&h->pg_lock);
> sdev->handler_data = NULL;
> kfree(h);
I don't think we can call the kref_put inside ->pg_lock. But I think
doing the list del early as in you patch, but keeping the put after
the unlock looks sensible. Can you submit a properly formatted patch
with a commit log and signoff for that?
