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

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

On Mon, 2013-04-22 at 11:35 -0600, Bjorn Helgaas wrote:
> On Mon, Apr 22, 2013 at 3:44 AM, Mike Qiu <qiudayu@linux.vnet.ibm.com> wrote:
> > 于 2013/4/22 16:04, Benjamin Herrenschmidt 写道:
> >
> >> On Mon, 2013-04-22 at 15:41 +0800, Mike Qiu wrote:
> >>>
> >>> 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...
> >>
> >> Ok. It *might* still be worth adding a BUG_ON then in pci_bus_to_host()
> >> itself ... no big deal either way.
> >
> > OK, you mean the code will remain the same and my patch is worthless?
> >
> > Anyway, it will be OK for me. But I hope it can be accept for more stable
> > reason.
> 
> This is powerpc code, so I'm kibitzing here, but it was cc'd to linux-pci :)
> 
> There should be no way to get a struct pci_bus * where bus->sysdata
> (== "hose") is NULL.  The sysdata pointer is always supplied to
> pci_create_root_bus() (or a similar interface that calls
> pci_create_root_bus()), so every root bus has a valid sysdata pointer.
>  And every child bus inherits the sysdata pointer of its parent (in
> pci_alloc_child_bus()).  Therefore, every pci_bus should have a valid
> sysdata pointer.
And the kernel has checked whether if sysdata is NULL after call
pci_create_root_bus().
> 
> So I think you should just remove even the existing "if (hose ==
> NULL)" check.  That way, if bus->sysdata actually *does* turn out to
> be NULL, we'll oops on the null pointer dereference, get a nice
> backtrace, and have a chance to fix the problem.  Testing and
> returning an error means whatever bug or memory corruption caused the
> null pointer will most likely be ignored.
> 
Yes, I agree with you, because it is useless, and meaningless. Also
it never achieve its goals even if the "hose==NULL". 

So I suggest either use my patch or remove that check code.
If the second case, I will write v2 patch to remove that check code.
I don't know what's to go for next step, it depends on the
maintainer :)

Thanks
Mike
 
> Bjorn
> --
> 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
>