Messages in this thread | | | Date | Fri, 7 Jan 2022 19:15:26 +0000 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH 04/15] PCI: mvebu: Handle invalid size of read config request |
| |
On Fri, Jan 07, 2022 at 12:45:48PM -0600, Bjorn Helgaas wrote: > On Thu, Nov 25, 2021 at 01:45:54PM +0100, Pali Rohár wrote: > > Function mvebu_pcie_hw_rd_conf() does not handle invalid size. So correctly > > set read value to all-ones and return appropriate error return value > > PCIBIOS_BAD_REGISTER_NUMBER like in mvebu_pcie_hw_wr_conf() function. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > Cc: stable@vger.kernel.org > > Is there a bug that this fixes? If not, I would drop the stable tag > (as I see Lorenzo already did, thanks!). > > > --- > > drivers/pci/controller/pci-mvebu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c > > index 08274132cdfb..19c6ee298442 100644 > > --- a/drivers/pci/controller/pci-mvebu.c > > +++ b/drivers/pci/controller/pci-mvebu.c > > @@ -261,6 +261,9 @@ static int mvebu_pcie_hw_rd_conf(struct mvebu_pcie_port *port, > > case 4: > > *val = readl_relaxed(conf_data); > > break; > > + default: > > + *val = 0xffffffff; > > + return PCIBIOS_BAD_REGISTER_NUMBER; > > Might be the right thing to do, but there are many config accessors > that do not set *val to ~0 before returning > PCIBIOS_BAD_REGISTER_NUMBER:
I think a better question would be - how can this function be called with a size that isn't 1, 2 or 4? I suppose if someone were to add another PCI_OP_READ/PCI_OP_WRITE. However... they really need to audit every implementation if they do that.
The generic implementation does this:
if (size == 1) *val = readb(addr); else if (size == 2) *val = readw(addr); else *val = readl(addr);
and therefore completely ignores the size if it isn't 1 or 2. So I don't think this is something that needs fixing.
If we're going to fix this in drivers, shouldn't we fix the generic implementation too?
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |