cpqphp: NULL ptr deref in cpqhpc_probe

cpqphp: NULL ptr deref in cpqhpc_probe

[Jiri Slaby]

Hi,

we have a system where there is a pci hotplug class device to be handled
by cpqphp, but it is not a bridge. But in cpqhpc_probe there is:
struct pci_bus *bus;
...
bus = pdev->subordinate;
...
bus->max_bus_speed = PCI_SPEED_66MHz_PCIX;

But as it is not a bridge, subordinate is NULL and the kernel crashes.

Any idea what would be a correct fix here?

The bugzilla entry is at:
https://bugzilla.novell.com/show_bug.cgi?id=609338

thanks,
-- 
js
suse labs

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Jiri Slaby]

On 06/03/2010 11:24 AM, Jiri Slaby wrote:
> we have a system where there is a pci hotplug class device to be handled
> by cpqphp, but it is not a bridge. But in cpqhpc_probe there is:
> struct pci_bus *bus;
> ...
> bus = pdev->subordinate;
> ...
> bus->max_bus_speed = PCI_SPEED_66MHz_PCIX;
> 
> But as it is not a bridge, subordinate is NULL and the kernel crashes.
> 
> Any idea what would be a correct fix here?
> 
> The bugzilla entry is at:
> https://bugzilla.novell.com/show_bug.cgi?id=609338

I forgot to attach the device information:
00:0b.0 PCI Hot-plug controller: Compaq Computer Corporation PCI Hotplug
Controller (rev 11)
	Subsystem: Compaq Computer Corporation Device a2f8
	Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+
Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-
	Interrupt: pin A routed to IRQ 15
	Region 0: Memory at f7ad0000 (32-bit, non-prefetchable) [size=256]

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Jesse Barnes]

On Thu, 03 Jun 2010 11:24:22 +0200
Jiri Slaby <jslaby@suse.cz> wrote:

> Hi,
> 
> we have a system where there is a pci hotplug class device to be handled
> by cpqphp, but it is not a bridge. But in cpqhpc_probe there is:
> struct pci_bus *bus;
> ...
> bus = pdev->subordinate;
> ...
> bus->max_bus_speed = PCI_SPEED_66MHz_PCIX;
> 
> But as it is not a bridge, subordinate is NULL and the kernel crashes.
> 
> Any idea what would be a correct fix here?
> 
> The bugzilla entry is at:
> https://bugzilla.novell.com/show_bug.cgi?id=609338

I don't think we have anyone actively working on CPQHPC these days.
Seems like the simple patch would be to check whether pdev->subordinate
or bus exists before using it...  Have you poked around for specs on
this at all?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Greg KH]

On Tue, Jun 08, 2010 at 02:37:39PM -0700, Jesse Barnes wrote:
> On Thu, 03 Jun 2010 11:24:22 +0200
> Jiri Slaby <jslaby@suse.cz> wrote:
> 
> > Hi,
> > 
> > we have a system where there is a pci hotplug class device to be handled
> > by cpqphp, but it is not a bridge. But in cpqhpc_probe there is:
> > struct pci_bus *bus;
> > ...
> > bus = pdev->subordinate;
> > ...
> > bus->max_bus_speed = PCI_SPEED_66MHz_PCIX;
> > 
> > But as it is not a bridge, subordinate is NULL and the kernel crashes.
> > 
> > Any idea what would be a correct fix here?
> > 
> > The bugzilla entry is at:
> > https://bugzilla.novell.com/show_bug.cgi?id=609338
> 
> I don't think we have anyone actively working on CPQHPC these days.
> Seems like the simple patch would be to check whether pdev->subordinate
> or bus exists before using it...  Have you poked around for specs on
> this at all?

I don't think I even have the specs anymore.  I ported the code from 2.4
back in 2001 or so and got it merged.  I didn't think this device was
still even around in systems.

a simple check would be best.

thanks,

greg k-h

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Alex Chiang]

Hi,

My hp.com address is dead. This is my new address.

Luckily, I just happened to be scanning LKML and thought "who
would be crazy enough to be touching cpqphp?" :)

* Jesse Barnes <jbarnes@virtuousgeek.org>:
> Jiri Slaby <jslaby@suse.cz> wrote:
> > Hi,
> > 
> > we have a system where there is a pci hotplug class device to be handled
> > by cpqphp, but it is not a bridge. But in cpqhpc_probe there is:
> > struct pci_bus *bus;
> > ...
> > bus = pdev->subordinate;
> > ...
> > bus->max_bus_speed = PCI_SPEED_66MHz_PCIX;
> > 
> > But as it is not a bridge, subordinate is NULL and the kernel crashes.
> > 
> > Any idea what would be a correct fix here?
> > 
> > The bugzilla entry is at:
> > https://bugzilla.novell.com/show_bug.cgi?id=609338
> 
> I don't think we have anyone actively working on CPQHPC these days.
> Seems like the simple patch would be to check whether pdev->subordinate
> or bus exists before using it...  Have you poked around for specs on
> this at all?

I think Greg/Jesse's suggestion is correct - just return if it's
not a bridge.

I managed to find an ancient machine that actually had this
hardware but never had time to work on it, and then switched
jobs. As far as I know, the hardware is still there, with my old
group.

But I seriously think it's just time to rip this driver out of
the tree. If anyone is actually using this driver to do hotplug,
I will personally buy that person a full half-barrel of
Colorado's finest microbrew of choice in order to numb the pain
of making poor career decisions. Hell, we can drink it together,
during which time I'll hack on the driver. The code surely can't
get any worse.

The occasional bug report shows that it's just more of a
maintenance burden than anything else.

/ac

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Greg KH]

On Thu, Jun 10, 2010 at 04:34:36PM -0600, Alex Chiang wrote:
> But I seriously think it's just time to rip this driver out of
> the tree. If anyone is actually using this driver to do hotplug,
> I will personally buy that person a full half-barrel of
> Colorado's finest microbrew of choice in order to numb the pain
> of making poor career decisions. Hell, we can drink it together,
> during which time I'll hack on the driver. The code surely can't
> get any worse.

A real user found this problem on their machine, so you might have to go
buy that barrel.  Jiri has more details on that :)

Me, I gave the machine that had this hardware in it to osuosl.org a long
time ago, odds are, it's still being used by some group.

thanks,

greg k-h

Re: cpqphp: NULL ptr deref in cpqhpc_probe

[Alex Chiang]

* Greg KH <greg@kroah.com>:
> On Thu, Jun 10, 2010 at 04:34:36PM -0600, Alex Chiang wrote:
> > But I seriously think it's just time to rip this driver out
> > of the tree. If anyone is actually using this driver to do
> > hotplug, I will personally buy that person a full half-barrel
> > of Colorado's finest microbrew of choice in order to numb the
> > pain of making poor career decisions. Hell, we can drink it
> > together, during which time I'll hack on the driver. The code
> > surely can't get any worse.
> 
> A real user found this problem on their machine, so you might
> have to go buy that barrel.  Jiri has more details on that :)

Well, I went and read the bz before responding, and the user did
say that he wan't interested in the actual hotplug aspect of it.
;)

> Me, I gave the machine that had this hardware in it to
> osuosl.org a long time ago, odds are, it's still being used by
> some group.

Oh, I don't doubt that someone is using the hardware to do
something useful. But I do doubt that anyone's using the hotplug
driver. It's been in various stages of brokenness for several
years now.

/ac