diff mbox

[3/3] scsi_dh_alua: do not call BUG_ON when updating port group

Message ID 20170428130626.32162-4-mwilck@suse.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Martin Wilck April 28, 2017, 1:06 p.m. UTC 
alua_rtpg() can race with alua_bus_detach(). The assertion that
alua_dh_data *h->sdev must be non-NULL is not guaranteed because
alua_bus_detach sets this field to NULL before removing the entry
from the port group's dh_list.

This happens when a device is about to be removed, so don't BUG out
but continue silently.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Bart Van Assche April 28, 2017, 7:58 p.m. UTC | #1 
On Fri, 2017-04-28 at 15:06 +0200, Martin Wilck wrote:
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 501855bde633..274fb49d0801 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -652,9 +652,13 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  					rcu_read_lock();
>  					list_for_each_entry_rcu(h,
>  						&tmp_pg->dh_list, node) {
> -						/* h->sdev should always be valid */
> -						BUG_ON(!h->sdev);
> -						h->sdev->access_state = desc[0];
> +						/*
> +						 * We might be racing with
> +						 * alua_bus_detach here
> +						 */
> +						if (h->sdev)
> +							h->sdev->access_state =
> +								desc[0];
>  					}
>  					rcu_read_unlock();
>  				}

Hello Hannes and Martin,

What will happen if h->sdev is cleared after it has been tested and before
it is dereferenced? Additionally, even if h->sdev would be cached, can the
following sequence of events happen?
* alua_rtpg() tests h->sdev.
* alua_bus_detach() clears h->sdev.
* h->sdev is freed.
* alua_rtpg() dereferences h->sdev.

Thanks,

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 501855bde633..274fb49d0801 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -652,9 +652,13 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 					rcu_read_lock();
 					list_for_each_entry_rcu(h,
 						&tmp_pg->dh_list, node) {
-						/* h->sdev should always be valid */
-						BUG_ON(!h->sdev);
-						h->sdev->access_state = desc[0];
+						/*
+						 * We might be racing with
+						 * alua_bus_detach here
+						 */
+						if (h->sdev)
+							h->sdev->access_state =
+								desc[0];
 					}
 					rcu_read_unlock();
 				}
@@ -694,7 +698,8 @@  static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 			pg->expiry = 0;
 			rcu_read_lock();
 			list_for_each_entry_rcu(h, &pg->dh_list, node) {
-				BUG_ON(!h->sdev);
+				if (!h->sdev)
+					continue;
 				h->sdev->access_state =
 					(pg->state & SCSI_ACCESS_STATE_MASK);
 				if (pg->pref)