[PATCH] scsi: libsas: Fix use-after-free in sas_suspend_devices()

[PATCH] scsi: libsas: Fix use-after-free in sas_suspend_devices()

[Wentao Liang]

sas_suspend_devices() walks port->dev_list with list_for_each_entry()
while sas_notify_lldd_dev_gone() may drop the last reference and free
the current device, so the next iteration dereferences freed memory.

Use list_for_each_entry_safe() to keep the next entry around.

Fixes: 303694eeee5e ("[SCSI] libsas: suspend / resume support")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/scsi/libsas/sas_discover.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index b07062db50b2..91045966612e 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -231,7 +231,7 @@ static void sas_probe_devices(struct asd_sas_port *port)
 static void sas_suspend_devices(struct work_struct *work)
 {
 	struct asd_sas_phy *phy;
-	struct domain_device *dev;
+	struct domain_device *dev, *n;
 	struct sas_discovery_event *ev = to_sas_discovery_event(work);
 	struct asd_sas_port *port = ev->port;
 	struct Scsi_Host *shost = port->ha->shost;
@@ -245,7 +245,7 @@ static void sas_suspend_devices(struct work_struct *work)
 	 * suspension, we force the issue here to keep the reference
 	 * counts aligned
 	 */
-	list_for_each_entry(dev, &port->dev_list, dev_list_node)
+	list_for_each_entry_safe(dev, n, &port->dev_list, dev_list_node)
 		sas_notify_lldd_dev_gone(dev);
 
 	/* we are suspending, so we know events are disabled and
-- 
2.34.1

Re: [PATCH] scsi: libsas: Fix use-after-free in sas_suspend_devices()

[John Garry]

On 9/17/26 15:38, Wentao Liang wrote:
> sas_suspend_devices() walks port->dev_list with list_for_each_entry()
> while sas_notify_lldd_dev_gone() may drop the last reference 

The low-level device driver should not keep (nor put) a reference to the 
domain device.

Notice how sas_put_device() is in sas_internal.h ?

> and free
> the current device, so the next iteration dereferences freed memory.
> 
> Use list_for_each_entry_safe() to keep the next entry around.
> 
> Fixes: 303694eeee5e ("[SCSI] libsas: suspend / resume support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   drivers/scsi/libsas/sas_discover.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> index b07062db50b2..91045966612e 100644
> --- a/drivers/scsi/libsas/sas_discover.c
> +++ b/drivers/scsi/libsas/sas_discover.c
> @@ -231,7 +231,7 @@ static void sas_probe_devices(struct asd_sas_port *port)
>   static void sas_suspend_devices(struct work_struct *work)
>   {
>   	struct asd_sas_phy *phy;
> -	struct domain_device *dev;
> +	struct domain_device *dev, *n;
>   	struct sas_discovery_event *ev = to_sas_discovery_event(work);
>   	struct asd_sas_port *port = ev->port;
>   	struct Scsi_Host *shost = port->ha->shost;
> @@ -245,7 +245,7 @@ static void sas_suspend_devices(struct work_struct *work)
>   	 * suspension, we force the issue here to keep the reference
>   	 * counts aligned
>   	 */
> -	list_for_each_entry(dev, &port->dev_list, dev_list_node)
> +	list_for_each_entry_safe(dev, n, &port->dev_list, dev_list_node)
>   		sas_notify_lldd_dev_gone(dev);
>   
>   	/* we are suspending, so we know events are disabled and