linux-kernel - Re: [PATCH] Calgary: Find nearest matching Calgary while walking up the PCI tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Thu, 3 Dec 2009 00:39:45 -0800
From:	"Darrick J. Wong" <djwong@...ibm.com>
To:	Muli Ben-Yehuda <muli@...ibm.com>
Cc:	Jon Mason <jdmason@...zu.us>, discuss@...-64.org,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Corinna Schultz <coschult@...ibm.com>
Subject: Re: [PATCH] Calgary: Find nearest matching Calgary while walking
	up the PCI tree

On Thu, Dec 03, 2009 at 08:49:48AM +0200, Muli Ben-Yehuda wrote:
> On Wed, Dec 02, 2009 at 05:51:19PM -0600, Jon Mason wrote:
> 
> > > diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
> > > index 971a3be..e6ec8a2 100644
> > > --- a/arch/x86/kernel/pci-calgary_64.c
> > > +++ b/arch/x86/kernel/pci-calgary_64.c
> > > @@ -318,13 +318,15 @@ static inline struct iommu_table *find_iommu_table(struct device *dev)
> > >
> > >        pdev = to_pci_dev(dev);
> > >
> > > +       /* search up the device tree for an iommu */
> > >        pbus = pdev->bus;
> > > -
> > > -       /* is the device behind a bridge? Look for the root bus */
> > > -       while (pbus->parent)
> > > +       do {
> > > +               tbl = pci_iommu(pbus);
> > > +               if (tbl && tbl->it_busno == pbus->number)
> > > +                       break;
> > > +               tbl = NULL;
> > 
> > I believe the NULL assignment is unnecessary.  If not, then the if
> > check and BUG_ON are busted.
> 
> I think the NULL assignment is needed for the case where the loop ends
> (pbus is NULL) and we did not find the right tbl. You don't want to
> leave the tbl pointing to the last tbl we saw while walking the bus.

Correct.  I suspect that you'd only hit this situation in a fairly pathological
corner case, but configuring the wrong Calgary for IO could (in theory) be used
to breach the Calgary protections, whereas failing to set up the necessary
permissions will simply crash the system.  In any case it seems cleaner to me to
throw away the tbl pointer once we've decided that we're done with it.

--D
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/