linux-kernel - Re: [PATCH] PCI: Avoid handing out address 0 to devices

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220419033752.GA1101844@bhelgaas>
Date:   Mon, 18 Apr 2022 22:37:52 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     "Maciej W. Rozycki" <macro@...am.me.uk>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: Avoid handing out address 0 to devices

On Sat, Apr 16, 2022 at 03:02:35PM +0100, Maciej W. Rozycki wrote:
> On Fri, 15 Apr 2022, Bjorn Helgaas wrote:
> ...

> > Another possibility is PCIBIOS_MIN_IO.  It's also kind of an ugly
> > special case, but at least it already exists.  Most arches define it
> > to be non-zero, which should avoid this issue.
> > 
> > Defining PCIBIOS_MIN_IO would be simple; what would we lose compared
> > to adding code in pci_bus_alloc_from_region()?
> 
>  As I explained in the change description:
> 
>   Especially I/O space ranges are particularly valuable, because
>   bridges only decode bits from 12 up and consequently where 16-bit
>   addressing is in effect, as few as 16 separate ranges can be
>   assigned to individual buses only.
> 
>   Therefore avoid handing out address 0, however rather than bumping
>   the lowest address available to PCI via PCIBIOS_MIN_IO and
>   PCIBIOS_MIN_MEM, or doing an equivalent arrangement in
>   `__pci_assign_resource', let the whole range assigned to a bus
>   start from that address and instead only avoid it for actual
>   devices.  [...]
> 
> Yes, just bumping up PCIBIOS_MIN_IO was my first thought and the
> path of least resistance.  However with the strictly hierarchical
> topology of PCIe systems the limit of 16 ranges feels so
> frighteningly low to me already as to make me rather unwilling to
> reduce it even further for a system that is free from PC legacy junk
> (no southbridge let alone ISA) and therefore does not require it.
> So I've reconsidered my initial approach and came up with this
> proposal instead.  I think it is a good compromise.

Sorry for being dense here, I think it's finally sinking in.

The problem is that making PCIBIOS_MIN_IO greater than zero would keep
us from assigning a [io 0x0000- ] window, so instead of 16 I/O bridge
windows, we could only have 15 (unless bridges support 32-bit I/O
windows).  Right?

Are you running into that limit?  Most devices don't need I/O port
space, and almost all other arches define PCIBIOS_MIN_IO, so they live
without that window today.

Sparc uses the MMIO I/O port address directly in the struct resource,
so it will never try to allocate [io 0x0000], and there's no problem
with using PCI I/O port 0:

  pci_bus 0000:00: root bus resource [io  0x804000000000-0x80400fffffff] (bus address [0x0000-0xfffffff])
  mpt2sas0: ioport(0x0000804000001000), size(256)

The sparc ioport interfaces are basically:

  ioport_map(port)  { return port; }
  ioread8(addr)     { return readb(addr); }
  inb(addr)         { return readb(addr); }

RISC-V uses the generic ones, i.e.,

  ioport_map(port)  { return PIO_OFFSET + port; }
  ioread8(addr)     { if (addr) >= PIO_RESERVED)
                        return readb(addr);
                      else
                        return inb(addr & PIO_MASK); }
  inb(addr)         { return __raw_readb(PCI_IOBASE + addr); }

Obviously RISC-V gives you prettier I/O port addresses, but at the
cost of having PCI I/O port 0 be 0 in the struct resource as well,
which makes it basically unusable.  Is it worth it?

Bjorn