linux-kernel - Re: [PATCH v2] bus: hisi_lpc: Don't use devm_kzalloc() to allocate logical PIO range

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

Message-ID: <CAErSpo4DemDWtnP2Gtram9tfQ0CaN9Na9_Gxk6Qk+nG5+JLuzA@mail.gmail.com>
Date:   Mon, 17 Jun 2019 15:33:32 -0500
From:   Bjorn Helgaas <bhelgaas@...gle.com>
To:     John Garry <john.garry@...wei.com>
Cc:     xuwei5@...wei.com, linuxarm@...wei.com, arm@...nel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Joe Perches <joe@...ches.com>,
        "zhichang.yuan" <zhichang.yuan02@...il.com>
Subject: Re: [PATCH v2] bus: hisi_lpc: Don't use devm_kzalloc() to allocate
 logical PIO range

On Mon, Jun 17, 2019 at 3:47 AM John Garry <john.garry@...wei.com> wrote:
>
> >>
> >> For the logical PIO framework, it was written to match what was done
> >> originally for PCI IO port management in pci_register_io_range(), cf
> >> https://elixir.bootlin.com/linux/v4.4.180/source/drivers/of/address.c#L691
> >>
> >> That is, no method to unregister ranges. As such, it leaks IO port
> >> ranges. I can come up with a few guesses why the original PCI IO port
> >> management author did not add an unregistration method.
> >
>
> Hi Bjorn,
>
> > I think that was written before the era of support for hot-pluggable
> > host bridges and loadable drivers for them.
>
> I see that the original support was added in 41f8bba7f555. I don't know
> how this coincides with hot-pluggable host bridges and their loadable
> drivers support.
>
> >
> >> Anyway, we can work on adding support to unregister regions, at least at
> >> probe time. It may become more tricky to do this once the host children
> >> have probed and are accessing the IO port regions.
> >
> > I think we *do* need support for unregistering regions because we do
> > claim to support hot-pluggable host bridges, and the I/O port regions
> > below them should go away when the host bridge does.
>
> It's now on my todo list.
>
> I'll need advice on how to test this for hot-pluggable host bridges.
>
> >
> > Could you just move the logic_pio_register_range() call farther down
> > in hisi_lpc_probe()?  IIUC, once logic_pio_register_range() returns,
> > an inb() with the right port number will try to access that port, so
> > we should be prepared for that, i.e., maybe this in the wrong order to
> > begin with?
>
> No, unfortunately we can't. The reason is that we need the logical PIO
> base for that range before we enumerate the children of that host. We
> need that base address for "translating" the child bus addresses to
> logical PIO addresses.

Ah, yeah, that makes sense.  I think.  We do assume that we know all
the MMIO and I/O port translations before enumerating devices.  It's
*conceivable* that could be changed someday since we don't actually
need the translations until a driver claims the device, and it would
gain some flexibility if we didn't have to program the host bridge
windows until we know how much space is required.  But I don't see
that happening anytime soon.

Bjorn