linux-kernel - Re: [PATCH v2 7/9] PCI: endpoint: Add the function number as argument to EPC ops

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <22c6a835-86e1-4145-e20b-aa74444dfd94@free-electrons.com>
Date:   Fri, 29 Dec 2017 21:30:00 +0100
From:   Cyrille Pitchen <cyrille.pitchen@...e-electrons.com>
To:     Kishon Vijay Abraham I <kishon@...com>, bhelgaas@...gle.com,
        lorenzo.pieralisi@....com, linux-pci@...r.kernel.org
Cc:     adouglas@...ence.com, stelford@...ence.com, dgary@...ence.com,
        kgopi@...ence.com, eandrews@...ence.com,
        thomas.petazzoni@...e-electrons.com, sureshp@...ence.com,
        nsekhar@...com, linux-kernel@...r.kernel.org, robh@...nel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v2 7/9] PCI: endpoint: Add the function number as argument
 to EPC ops

Hi Kishon,

Le 29/12/2017 à 10:23, Kishon Vijay Abraham I a écrit :
> Hi,
> 
> On Monday 18 December 2017 11:46 PM, Cyrille Pitchen wrote:
>> This patch updates the prototype of most handlers from 'struct
>> pci_epc_ops' so the EPC library can now support multi-function devices.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@...e-electrons.com>
>> ---
>>  drivers/pci/dwc/pcie-designware-ep.c          | 20 +++++----
>>  drivers/pci/endpoint/functions/pci-epf-test.c | 41 ++++++++++--------
>>  drivers/pci/endpoint/pci-epc-core.c           | 62 ++++++++++++++++-----------
>>  include/linux/pci-epc.h                       | 43 +++++++++++--------
>>  4 files changed, 96 insertions(+), 70 deletions(-)
>>
[...]
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index cd7d4788b94d..77420364a728 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -141,25 +141,26 @@ EXPORT_SYMBOL_GPL(pci_epc_start);
>>  /**
>>   * pci_epc_raise_irq() - interrupt the host system
>>   * @epc: the EPC device which has to interrupt the host
>> + * @func_no: the endpoint function number in the EPC device
>>   * @type: specify the type of interrupt; legacy or MSI
>>   * @interrupt_num: the MSI interrupt number
>>   *
>>   * Invoke to raise an MSI or legacy interrupt
>>   */
>> -int pci_epc_raise_irq(struct pci_epc *epc, enum pci_epc_irq_type type,
>> -		      u8 interrupt_num)
>> +int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no,
>> +		      enum pci_epc_irq_type type, u8 interrupt_num)
>>  {
>>  	int ret;
>>  	unsigned long flags;
>>  
>> -	if (IS_ERR(epc))
>> +	if (IS_ERR(epc) || func_no > BAR_5)
> 
> why is function number compared with BAR? here and everywhere below..
>

Oops! sorry for that. Actually it's a silly mistake, thanks for
catching it!

I plan to fix it replacing with something like this:

s/func_no > BAR_5/func_no >= epc->max_functions/

Then I guess if we already test with IS_ERR(), I should now replace
IS_ERR() by IS_ERR_OR_NULL() before dereferencing epc when reading
max_functions.

Best regards,

Cyrille
 
> Thanks
> Kishon
> 


-- 
Cyrille Pitchen, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com