Re: [PATCH] vfio/pci: Fix handling of pci use accessor return codes

[Heiner Kallweit <hkallweit1@gmail.com>]

On 25.01.2021 19:11, Cornelia Huck wrote:
> On Sun, 24 Jan 2021 16:35:41 +0100
> Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 
>> The pci user accessors return negative errno's on error.
>>
>> Fixes: f572a960a15e ("vfio/pci: Intel IGD host and LCP bridge config space access")
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/vfio/pci/vfio_pci_igd.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
>> index 53d97f459..e66dfb017 100644
>> --- a/drivers/vfio/pci/vfio_pci_igd.c
>> +++ b/drivers/vfio/pci/vfio_pci_igd.c
>> @@ -127,7 +127,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev,
>>  
>>  		ret = pci_user_read_config_byte(pdev, pos, &val);
>>  		if (ret)
>> -			return pcibios_err_to_errno(ret);
>> +			return ret;
> 
> This is actually not strictly needed, as pcibios_err_to_errno() already
> keeps errors <= 0 unchanged, so more a cleanup than a fix?
> 
I agree. Although I'd argue that the author of the original commit missed
the fact the the user accessors return errno's, and the code just works
by chance, because of the "good will" of pcibios_err_to_errno().
So up to you whether it's worth it to apply this change also to stable.

> Anyway,
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 
>>  
>>  		if (copy_to_user(buf + count - size, &val, 1))
>>  			return -EFAULT;
>