From: Will Deacon <will.deacon@arm.com>
To: David Daney <ddaney@caviumnetworks.com>
Cc: David Daney <ddaney.cavm@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>, Pawel Moll <Pawel.Moll@arm.com>,
Mark Rutland <Mark.Rutland@arm.com>,
Ian Campbell <ijc+devicetree@hellion.org.uk>,
Kumar Gala <galak@codeaurora.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Marc Zyngier <Marc.Zyngier@arm.com>,
David Daney <david.daney@cavium.com>
Subject: Re: [PATCH v2 4/5] PCI: generic: Correct, and avoid overflow, in bus_max calculation.
Date: Wed, 23 Sep 2015 20:33:18 +0100 [thread overview]
Message-ID: <20150923193318.GY7356@arm.com> (raw)
In-Reply-To: <5602EDC4.3040603@caviumnetworks.com>
On Wed, Sep 23, 2015 at 07:21:56PM +0100, David Daney wrote:
> On 09/23/2015 11:01 AM, Will Deacon wrote:
> > On Thu, Sep 17, 2015 at 11:02:11PM +0100, David Daney wrote:
> [...]
> >
> >> Properties of the /chosen node:
> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> >> index 77cf4bd..0a9c453 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> >> @@ -164,7 +164,7 @@ out_release_res:
> >> static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
> >> {
> >> int err;
> >> - u8 bus_max;
> >> + int bus_max;
> >> resource_size_t busn;
> >> struct resource *bus_range;
> >> struct device *dev = pci->host.dev.parent;
> >> @@ -177,8 +177,9 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
> >> }
> >>
> >> /* Limit the bus-range to fit within reg */
> >> - bus_max = pci->cfg.bus_range->start +
> >> - (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1;
> >> + bus_max = (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1;
> >> + if (bus_max > 255)
> >> + bus_max = 255;
> >
> > I still don't understand the need for this part. If the cfg space is bigger
> > than bus_max, isn't that simply an invalid resource? Given that the resource
> > could be broken in other ways too, this check feels more like a specific
> > workaround rather than generally useful code.
>
> Imagine...
>
> bus-range [0x80 .. 0xff], this requires a cfg.res that will cover the
> entire range of 0..0xff.
>
> according to the calculations above, (resource_size(&pci->cfg.res) >>
> pci->cfg.ops.bus_shift) - 1 will have a value of 0xff, so...
>
> bus_max = 0x80 + 0xff -> OVERFLOW of u8!
>
> That is not useful. bus_max should represent the largest bus number
> that can be covered by cfg.res. That is what my patch is attempting to
> accomplish. Calculate the largest bus number that can be accommodated
> by cfg.res, and then clamp it to 0xff.
Sorry, I should've been more specific. The only part I don't like is the
'if (bus_max > 255)' check.
Will
next prev parent reply other threads:[~2015-09-23 19:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-17 22:02 [PATCH v2 0/5] PCI: generic: Misc. bug fixes David Daney
2015-09-17 22:02 ` [PATCH v2 1/5] PCI: Add pci_bus_fixup_irqs() David Daney
2015-09-17 22:02 ` [PATCH v2 2/5] PCI: generic: Only fixup irqs for bus we are creating David Daney
2015-09-23 17:59 ` Will Deacon
2015-09-17 22:02 ` [PATCH v2 3/5] PCI: generic: Quit clobbering our pci_ops David Daney
2015-09-17 22:02 ` [PATCH v2 4/5] PCI: generic: Correct, and avoid overflow, in bus_max calculation David Daney
2015-09-23 18:01 ` Will Deacon
2015-09-23 18:21 ` David Daney
2015-09-23 19:27 ` Arnd Bergmann
2015-09-23 19:35 ` Will Deacon
2015-09-23 19:39 ` Arnd Bergmann
2015-09-23 19:47 ` Will Deacon
2015-09-23 20:45 ` Arnd Bergmann
2015-09-23 19:33 ` Will Deacon [this message]
2015-09-17 22:02 ` [PATCH v2 5/5] PCI: generic: Pass proper starting bus number to pci_scan_root_bus() David Daney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150923193318.GY7356@arm.com \
--to=will.deacon@arm.com \
--cc=Marc.Zyngier@arm.com \
--cc=Mark.Rutland@arm.com \
--cc=Pawel.Moll@arm.com \
--cc=bhelgaas@google.com \
--cc=david.daney@cavium.com \
--cc=ddaney.cavm@gmail.com \
--cc=ddaney@caviumnetworks.com \
--cc=devicetree@vger.kernel.org \
--cc=galak@codeaurora.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).