linux-kernel - Re: [PATCH] scsi: fix missing lock protection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <03b1b927-fb33-46e8-b38d-08e986f45672@acm.org>
Date: Thu, 20 Feb 2025 11:01:46 +0800
From: Chaohai Chen <wdhh66@....com>
To: wdhh66@....com,
	martin.petersen@...cle.com
Cc: James.Bottomley@...senPartnership.com,
	linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH] scsi: fix missing lock protection

From: Bart Van Assche <bvanassche@....org>

On 2/19/25 12:11 AM, Chaohai Chen wrote:
>> async_scan_lock is designed to protect the scanning_hosts list,
>> but there is no protection here.
>> 
>> Signed-off-by: Chaohai Chen <wdhh66@....com>
>> ---
>>   drivers/scsi/scsi_scan.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>> index 087fcbfc9aaa..9a90e6ba5603 100644
>> --- a/drivers/scsi/scsi_scan.c
>> +++ b/drivers/scsi/scsi_scan.c
>> @@ -151,8 +151,12 @@ int scsi_complete_async_scans(void)
>>   	struct async_scan_data *data;
>>   
>>   	do {
>> -		if (list_empty(&scanning_hosts))
>> +		spin_lock(&async_scan_lock);
>> +		if (list_empty(&scanning_hosts)) {
>> +			spin_unlock(&async_scan_lock);
>>   			return 0;
>> +		}
>> +		spin_unlock(&async_scan_lock);
>>   		/* If we can't get memory immediately, that's OK.  Just
>>   		 * sleep a little.  Even if we never get memory, the async
>>   		 * scans will finish eventually.
>
>Has it been considered to use scoped_guard() as in the untested patch
>below?
>
>Thanks,
>
>Bart.
>
>
>diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>index 087fcbfc9aaa..efc90571ab47 100644
>--- a/drivers/scsi/scsi_scan.c
>+++ b/drivers/scsi/scsi_scan.c
>@@ -151,8 +151,9 @@ int scsi_complete_async_scans(void)
>  	struct async_scan_data *data;
>
>  	do {
>-		if (list_empty(&scanning_hosts))
>-			return 0;
>+		scoped_guard(spinlock, &async_scan_lock)
>+			if (list_empty(&scanning_hosts))
>+				return 0;
>  		/* If we can't get memory immediately, that's OK.  Just
>  		 * sleep a little.  Even if we never get memory, the async
>  		 * scans will finish eventually.
>
>

Of course, it looks nice, thanks.

Chaohai