Re: [PATCH] scsi midlayer: fix sdev reuse after free

[James Smart <James.Smart@Emulex.Com>]

FYI - NACK this patch. It passed testing on small systems, but didn't fare
well at all on larger more parallel systems.  Still working on a patch for
this.

-- james s

James Smart wrote:
> The conversion to execute_in_process_context() highlighted a use-after-free
> race condition. Although the sdev was torn down, it remained in the linked
> lists looked at by scan, and allowed scan to reuse the sdev.
> 
> This patch removes the sdev from the lists at the point it tears down the
> sdev.
> 
> -- james s
> 
> Signed-off-by: James Smart <james.smart@emulex.com>
> 
> diff -upNr a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> --- a/drivers/scsi/scsi_sysfs.c	2006-06-14 11:37:09.000000000 -0400
> +++ b/drivers/scsi/scsi_sysfs.c	2006-06-26 14:48:31.000000000 -0400
> @@ -231,8 +231,6 @@ static void scsi_device_dev_release_user
>  
>  	spin_lock_irqsave(sdev->host->host_lock, flags);
>  	starget->reap_ref++;
> -	list_del(&sdev->siblings);
> -	list_del(&sdev->same_target_siblings);
>  	list_del(&sdev->starved_entry);
>  	spin_unlock_irqrestore(sdev->host->host_lock, flags);
>  
> @@ -735,10 +733,15 @@ int scsi_sysfs_add_sdev(struct scsi_devi
>  void __scsi_remove_device(struct scsi_device *sdev)
>  {
>  	struct device *dev = &sdev->sdev_gendev;
> +	unsigned long flags;
>  
>  	if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
>  		return;
>  
> +	spin_lock_irqsave(sdev->host->host_lock, flags);
> +	list_del(&sdev->siblings);
> +	list_del(&sdev->same_target_siblings);
> +	spin_unlock_irqrestore(sdev->host->host_lock, flags);
>  	class_device_unregister(&sdev->sdev_classdev);
>  	transport_remove_device(dev);
>  	device_del(dev);
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>