Re: [PATCH] scsi: simplify registration of scsi host sysfs attributes

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 2021/11/16 11:18, Bart Van Assche wrote:
> On 11/15/21 1:29 AM, Damien Le Moal wrote:
>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>> index 8049b00b6766..c3b6812aac5b 100644
>> --- a/drivers/scsi/hosts.c
>> +++ b/drivers/scsi/hosts.c
>> @@ -359,6 +359,7 @@ static void scsi_host_dev_release(struct device *dev)
>>   static struct device_type scsi_host_type = {
>>   	.name =		"scsi_host",
>>   	.release =	scsi_host_dev_release,
>> +	.groups =	scsi_sysfs_shost_attr_groups,
>>   };
> 
> Many SCSI LLDs use class_to_shost() to convert a device pointer into a SCSI host
> pointer. This patch makes the use of that macro very confusing since the SCSI
> host class is no longer involved in attribute registration.

OK. But at least I think we should fix this:

WARN_ON_ONCE(j >= ARRAY_SIZE(shost->shost_dev_attr_groups));

to change it into:

if (WARN_ON_ONCE(j >= ARRAY_SIZE(shost->shost_dev_attr_groups)))
	shost->shost_dev_attr_groups[j] = NULL;

to guarantee that the attribute groups array is NULL terminated, as it should
be. This will ensure that we do not end up with a kernel crash when a buggy
driver is loaded. No ?

> 
> Thanks,
> 
> Bart.
> 


-- 
Damien Le Moal
Western Digital Research