Re: [PATCH] ata: libata-scsi: fix sloppy result type of ata_ioc32()

[Sergey Shtylyov <s.shtylyov@omp.ru>]

On 6/20/22 2:12 AM, Damien Le Moal wrote:

>> While ata_ioc32() returns *int*, its result gets assigned to and compared
>> with the *unsigned long* variable 'val' in ata_sas_scsi_ioctl(), its only
>> caller, which implies a problematic implicit cast -- fix that by returning
>> *unsigned long* instead.
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> ---
>> This patch is against the 'for-next' branch of Damien's 'libata.git' repo.
>>
>>  drivers/ata/libata-scsi.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: libata/drivers/ata/libata-scsi.c
>> ===================================================================
>> --- libata.orig/drivers/ata/libata-scsi.c
>> +++ libata/drivers/ata/libata-scsi.c
>> @@ -539,7 +539,7 @@ int ata_task_ioctl(struct scsi_device *s
>>  	return rc;
>>  }
>>  
>> -static int ata_ioc32(struct ata_port *ap)
>> +static unsigned long ata_ioc32(struct ata_port *ap)
>>  {
>>  	if (ap->flags & ATA_FLAG_PIO_DMA)
>>  		return 1;

> Actually, this should be a bool I think and the ioctl code cleaned to use

   By the ioctl code you mean ata_sas_scsi_ioctl()?

> that type since the val argument of the ioctl is also used as a bool.

   As for HDIO_SET_32BIT, that's prolly OK but what to do with HDIO_GET_32BIT
(it calls put_user() on *unsigned long*)?

MBR, Sergey