fix for kernel BUG at drivers/scsi/device_handler/scsi_dh_alua.c:662!

fix for kernel BUG at drivers/scsi/device_handler/scsi_dh_alua.c:662!

[Brian Bunker]

Hello all,

It looks like I sent this email to the wrong mailing list according to RedHat so I am sending it here:

>From my earlier post:

https://www.redhat.com/archives/dm-devel/2020-September/msg00083.html


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);
 }

Thanks,
Brian

Brian Bunker
SW Eng
brian@purestorage.com

Re: fix for kernel BUG at drivers/scsi/device_handler/scsi_dh_alua.c:662!

[Christoph Hellwig]

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?

fix for kernel BUG at drivers/scsi/device_handler/scsi_dh_alua.c:662!

[Brian Bunker]

Hello all,

From my earlier post:
https://www.redhat.com/archives/dm-devel/2020-September/msg00083.html

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);
 }

Thanks,
Brian

Brian Bunker
SW Eng
brian@purestorage.com