linux-kernel - Re: [PATCH V2 2/8] vfio/pci: Remove negative check on unsigned vector

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <460fdc5f-7613-5164-0247-254939cedc71@intel.com>
Date:   Thu, 30 Mar 2023 15:32:20 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     <jgg@...dia.com>, <yishaih@...dia.com>,
        <shameerali.kolothum.thodi@...wei.com>, <kevin.tian@...el.com>,
        <tglx@...utronix.de>, <darwi@...utronix.de>, <kvm@...r.kernel.org>,
        <dave.jiang@...el.com>, <jing2.liu@...el.com>,
        <ashok.raj@...el.com>, <fenghua.yu@...el.com>,
        <tom.zanussi@...ux.intel.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 2/8] vfio/pci: Remove negative check on unsigned vector

Hi Alex,

On 3/30/2023 1:26 PM, Alex Williamson wrote:
> On Tue, 28 Mar 2023 14:53:29 -0700
> Reinette Chatre <reinette.chatre@...el.com> wrote:
...

>> @@ -399,7 +399,8 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev,
>>  static int vfio_msi_set_block(struct vfio_pci_core_device *vdev, unsigned start,
>>  			      unsigned count, int32_t *fds, bool msix)
>>  {
>> -	int i, j, ret = 0;
>> +	int i, ret = 0;
>> +	unsigned int j;
>>  
>>  	if (start >= vdev->num_ctx || start + count > vdev->num_ctx)
>>  		return -EINVAL;
> 
> Unfortunately this turns the unwind portion of the function into an
> infinite loop in the common case when @start is zero:
> 
>                 for (--j; j >= (int)start; j--)
>                         vfio_msi_set_vector_signal(vdev, j, -1, msix);
> 
> 

Thank you very much for catching this. It is not clear to me how you
would prefer to resolve this. Would you prefer that the vector parameter
in vfio_msi_set_vector_signal() continue to be an int and this patch be
dropped and the "if (vector < 0)" check remains (option A)? Or, alternatively,
I see two other possible solutions where the vector parameter in
vfio_msi_set_vector_signal() becomes an unsigned int and the above snippet
could be one of:

option B:
vfio_msi_set_block()
{
	int i, j, ret = 0;

	...
		for (--j; j >= (int)start; j--)
			vfio_msi_set_vector_signal(vdev, (unsigned int)j, -1, msix);
}

option C:
vfio_msi_set_block()
{
	int i, ret = 0;
	unsigned int j;

	...
		for (--j; j >= start && j < start + count; j--)
			vfio_msi_set_vector_signal(vdev, j, -1, msix);
}

What would you prefer?

Reinette