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

Sergey Shtylyov
2022-06-20T20:26:59Z

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