2019-07-30 17:45:29

by John Garry

[permalink] [raw]
Subject: [PATCH v4 0/5] Fixes for HiSilicon LPC driver and logical PIO code

As reported in [1], the hisi-lpc driver has certain issues in handling
logical PIO regions, specifically unregistering regions.

This series add a method to unregister a logical PIO region, and fixes up
the driver to use them.

RCU usage in logical PIO code looks to always have been broken, so that
is fixed also. This is not a major fix as the list which RCU protects
would be rarely modified.

At this point, there are still separate ongoing discussions about how to
stop the logical PIO and PCI host bridge code leaking ranges, as in [2],
which I haven't had a chance to look at recently.

Hopefully this series can go through the arm soc tree and the maintainers
have no issue with that. I'm talking specifically about the logical PIO
code, which went through PCI tree on only previous upstreaming.


[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/

Changes since v3:
https://lore.kernel.org/lkml/[email protected]/
- drop optimisation patch (lib: logic_pio: Enforce LOGIC_PIO_INDIRECT
region ops are set at registration)
Not a fix
- rebase to v5.3-rc2
- cc stable

Change since v2:
- Add hisi_lpc_acpi_remove() stub to fix build for !CONFIG_ACPI

Changes since v1:
- Add more reasoning in RCU fix patch
- Create separate patch to change LOGIC_PIO_CPU_MMIO registration to
accomodate unregistration

John Garry (5):
lib: logic_pio: Fix RCU usage
lib: logic_pio: Avoid possible overlap for unregistering regions
lib: logic_pio: Add logic_pio_unregister_range()
bus: hisi_lpc: Unregister logical PIO range to avoid potential
use-after-free
bus: hisi_lpc: Add .remove method to avoid driver unbind crash

drivers/bus/hisi_lpc.c | 47 +++++++++++++++++++++----
include/linux/logic_pio.h | 1 +
lib/logic_pio.c | 73 +++++++++++++++++++++++++++++----------
3 files changed, 96 insertions(+), 25 deletions(-)

--
2.17.1


2019-07-30 17:45:38

by John Garry

[permalink] [raw]
Subject: [PATCH v4 4/5] bus: hisi_lpc: Unregister logical PIO range to avoid potential use-after-free

If, after registering a logical PIO range, the driver probe later fails,
the logical PIO range memory will be released automatically.

This causes an issue, in that the logical PIO range is not unregistered
and the released range memory may be later referenced.

Fix by unregistering the logical PIO range.

And since we now unregister the logical PIO range for probe failure, avoid
the special ordering of setting logical PIO range ops, which was the
previous (poor) attempt at a safeguard against this.

Cc: [email protected]
Fixes: adf38bb0b595 ("HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings")
Signed-off-by: John Garry <[email protected]>
---
drivers/bus/hisi_lpc.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 19d7b6ff2f17..6d301aafcad2 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -606,24 +606,25 @@ static int hisi_lpc_probe(struct platform_device *pdev)
range->fwnode = dev->fwnode;
range->flags = LOGIC_PIO_INDIRECT;
range->size = PIO_INDIRECT_SIZE;
+ range->hostdata = lpcdev;
+ range->ops = &hisi_lpc_ops;
+ lpcdev->io_host = range;

ret = logic_pio_register_range(range);
if (ret) {
dev_err(dev, "register IO range failed (%d)!\n", ret);
return ret;
}
- lpcdev->io_host = range;

/* register the LPC host PIO resources */
if (acpi_device)
ret = hisi_lpc_acpi_probe(dev);
else
ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
- if (ret)
+ if (ret) {
+ logic_pio_unregister_range(range);
return ret;
-
- lpcdev->io_host->hostdata = lpcdev;
- lpcdev->io_host->ops = &hisi_lpc_ops;
+ }

io_end = lpcdev->io_host->io_start + lpcdev->io_host->size;
dev_info(dev, "registered range [%pa - %pa]\n",
--
2.17.1

2019-08-16 02:36:18

by Wei Xu

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] Fixes for HiSilicon LPC driver and logical PIO code

Hi John,

On 2019/7/30 21:29, John Garry wrote:
> As reported in [1], the hisi-lpc driver has certain issues in handling
> logical PIO regions, specifically unregistering regions.
>
> This series add a method to unregister a logical PIO region, and fixes up
> the driver to use them.
>
> RCU usage in logical PIO code looks to always have been broken, so that
> is fixed also. This is not a major fix as the list which RCU protects
> would be rarely modified.
>
> At this point, there are still separate ongoing discussions about how to
> stop the logical PIO and PCI host bridge code leaking ranges, as in [2],
> which I haven't had a chance to look at recently.
>
> Hopefully this series can go through the arm soc tree and the maintainers
> have no issue with that. I'm talking specifically about the logical PIO
> code, which went through PCI tree on only previous upstreaming.
>
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/

Thanks!
Series applied to the hisilicon fixes tree.

Best Regards,
Wei

> Changes since v3:
> https://lore.kernel.org/lkml/[email protected]/
> - drop optimisation patch (lib: logic_pio: Enforce LOGIC_PIO_INDIRECT
> region ops are set at registration)
> Not a fix
> - rebase to v5.3-rc2
> - cc stable
>
> Change since v2:
> - Add hisi_lpc_acpi_remove() stub to fix build for !CONFIG_ACPI
>
> Changes since v1:
> - Add more reasoning in RCU fix patch
> - Create separate patch to change LOGIC_PIO_CPU_MMIO registration to
> accomodate unregistration
>
> John Garry (5):
> lib: logic_pio: Fix RCU usage
> lib: logic_pio: Avoid possible overlap for unregistering regions
> lib: logic_pio: Add logic_pio_unregister_range()
> bus: hisi_lpc: Unregister logical PIO range to avoid potential
> use-after-free
> bus: hisi_lpc: Add .remove method to avoid driver unbind crash
>
> drivers/bus/hisi_lpc.c | 47 +++++++++++++++++++++----
> include/linux/logic_pio.h | 1 +
> lib/logic_pio.c | 73 +++++++++++++++++++++++++++++----------
> 3 files changed, 96 insertions(+), 25 deletions(-)
>