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]
Date:   Fri, 23 Sep 2016 06:02:26 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org
Cc:     david.vrabel@...rix.com
Subject: Re: [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list

On 22/09/16 23:02, Boris Ostrovsky wrote:
> On 09/22/2016 04:45 AM, Juergen Gross wrote:
>> The Xen pciback driver has a list of all pci devices it is ready to
>> seize. There is no check whether a to be added entry already exists.
>> While this might be no problem in the common case it might confuse
>> those which consume the list via sysfs.
>>
>> Modify the handling of this list by not adding an entry which already
>> exists. As this will be needed later split out the list handling into
>> a separate function.
>>
>> Signed-off-by: Juergen Gross <jgross@...e.com>
>> ---
>>  drivers/xen/xen-pciback/pci_stub.c | 39 ++++++++++++++++++++++++++++++--------
>>  1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
>> index 79a9e4d..0179333 100644
>> --- a/drivers/xen/xen-pciback/pci_stub.c
>> +++ b/drivers/xen/xen-pciback/pci_stub.c
>> @@ -478,6 +478,36 @@ static int __init pcistub_init_devices_late(void)
>>  	return 0;
>>  }
>>  
>> +static void pcistub_device_id_add_list(struct pcistub_device_id *new,
>> +				       int domain, int bus, unsigned int devfn)
>> +{
>> +	struct pcistub_device_id *pci_dev_id;
>> +	unsigned long flags;
>> +	int found = 0;
>> +
>> +	spin_lock_irqsave(&device_ids_lock, flags);
>> +
>> +	list_for_each_entry(pci_dev_id, &pcistub_device_ids, slot_list) {
>> +		if (pci_dev_id->domain == domain && pci_dev_id->bus == bus &&
>> +		    pci_dev_id->devfn == devfn) {
>> +			found = 1;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!found) {
>> +		new->domain = domain;
>> +		new->bus = bus;
>> +		new->devfn = devfn;
>> +		list_add_tail(&new->slot_list, &pcistub_device_ids);
>> +	}
>> +
>> +	spin_unlock_irqrestore(&device_ids_lock, flags);
>> +
>> +	if (found)
>> +		kfree(new);
> 
> I'd rather free 'new' in the caller (who allocated it) and return
> something like -EEXIST if device is already on the list.

Hmm, I thought of this, but with two callers after the following patch
having to deal with the situation I've chosen this way to do it.

The code is smaller this way.


Juergen

Powered by blists - more mailing lists