Re: [PATCH] PowerNV/PCI: Fix NULL PCI controller

[Mike Qiu <qiudayu@linux.vnet.ibm.com>]

于 2013/4/22 14:36, Benjamin Herrenschmidt 写道:
> On Mon, 2013-04-22 at 02:13 -0400, Mike Qiu wrote:
>> In pnv_pci_read_config() or pnv_pci_write_config(), we never check if
>> the PCI controller is valid before converting that into platform
>> dependent one, this is very dangerous.
>>
>> To avoid this potential risks, the patch check PCI controller first
>> before use it.
> I don't think there's any remote possibility of that happening, is
> there ?
Yes, I agree, I don't exactly mean that it maybe happen, but the code
try to check the pci_controller pointer and the way it try is useless,
because if this happens, the system will crash before check: try to access
the "NULL" pointer.

My patch just makes the code more stable and robust.

Anyway, I think it's better to remove the check code as it is useless, as it
will shows that this "NULL" pci_controller pointer may happen...

Thanks
Mike
> If it does, maybe it warrants a WARN_ON...
>
> Ben.
>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/platforms/powernv/pci.c |    8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index b8b8e0b..e7b7f1a 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -286,11 +286,11 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>>   			       int where, int size, u32 *val)
>>   {
>>   	struct pci_controller *hose = pci_bus_to_host(bus);
>> -	struct pnv_phb *phb = hose->private_data;
>> +	struct pnv_phb *phb = hose ? hose->private_data : NULL;
>>   	u32 bdfn = (((uint64_t)bus->number) << 8) | devfn;
>>   	s64 rc;
>>   
>> -	if (hose == NULL)
>> +	if (!phb)
>>   		return PCIBIOS_DEVICE_NOT_FOUND;
>>   
>>   	switch (size) {
>> @@ -330,10 +330,10 @@ static int pnv_pci_write_config(struct pci_bus *bus,
>>   				int where, int size, u32 val)
>>   {
>>   	struct pci_controller *hose = pci_bus_to_host(bus);
>> -	struct pnv_phb *phb = hose->private_data;
>> +	struct pnv_phb *phb = hose ? hose->private_data : NULL;
>>   	u32 bdfn = (((uint64_t)bus->number) << 8) | devfn;
>>   
>> -	if (hose == NULL)
>> +	if (!phb)
>>   		return PCIBIOS_DEVICE_NOT_FOUND;
>>   
>>   	cfg_dbg("pnv_pci_write_config bus: %x devfn: %x +%x/%x -> %08x\n",
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>