[PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses

[PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses

[Russell Currey]

fail_iommu_setup() registers the fail_iommu_bus_notifier struct to both
PCI and VIO buses.  struct notifier_block is a linked list node, so this
causes any notifiers later registered to either bus type to also be
registered to the other since they share the same node.

This causes issues in (at least) the vgaarb code, which registers a
notifier for PCI buses.  pci_notify() ends up being called on a vio
device, converted with to_pci_dev() even though it's not a PCI device,
and finally makes a bad access in vga_arbiter_add_pci_device() as
discovered with KASAN:

 BUG: KASAN: slab-out-of-bounds in vga_arbiter_add_pci_device+0x60/0xe00
 Read of size 4 at addr c000000264c26fdc by task swapper/0/1

 Call Trace:
 [c000000263607520] [c000000010f7023c] dump_stack_lvl+0x1bc/0x2b8 (unreliable)
 [c000000263607560] [c00000000f142a64] print_report+0x3f4/0xc60
 [c000000263607640] [c00000000f142144] kasan_report+0x244/0x698
 [c000000263607740] [c00000000f1460e8] __asan_load4+0xe8/0x250
 [c000000263607760] [c00000000ff4b850] vga_arbiter_add_pci_device+0x60/0xe00
 [c000000263607850] [c00000000ff4c678] pci_notify+0x88/0x444
 [c0000002636078b0] [c00000000e94dfc4] notifier_call_chain+0x104/0x320
 [c000000263607950] [c00000000e94f050] blocking_notifier_call_chain+0xa0/0x140
 [c000000263607990] [c0000000100cb3b8] device_add+0xac8/0x1d30
 [c000000263607aa0] [c0000000100ccd98] device_register+0x58/0x80
 [c000000263607ad0] [c00000000e84247c] vio_register_device_node+0x9ac/0xce0
 [c000000263607ba0] [c0000000126c95d8] vio_bus_scan_register_devices+0xc4/0x13c
 [c000000263607bd0] [c0000000126c96e4] __machine_initcall_pseries_vio_device_init+0x94/0xf0
 [c000000263607c00] [c00000000e69467c] do_one_initcall+0x12c/0xaa8
 [c000000263607cf0] [c00000001268b8a8] kernel_init_freeable+0xa48/0xba8
 [c000000263607dd0] [c00000000e695f24] kernel_init+0x64/0x400
 [c000000263607e50] [c00000000e68e0e4] ret_from_kernel_thread+0x5c/0x64

Fix this by creating separate notifier_block structs for each bus type.

Fixes: d6b9a81b2a45 ("powerpc: IOMMU fault injection")
Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/kernel/iommu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index ee95937bdaf1..6f1117fe3870 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -171,17 +171,26 @@ static int fail_iommu_bus_notify(struct notifier_block *nb,
 	return 0;
 }
 
-static struct notifier_block fail_iommu_bus_notifier = {
+/*
+ * PCI and VIO buses need separate notifier_block structs, since they're linked
+ * list nodes.  Sharing a notifier_block would mean that any notifiers later
+ * registered for PCI buses would also get called by VIO buses and vice versa.
+ */
+static struct notifier_block fail_iommu_pci_bus_notifier = {
+	.notifier_call = fail_iommu_bus_notify
+};
+
+static struct notifier_block fail_iommu_vio_bus_notifier = {
 	.notifier_call = fail_iommu_bus_notify
 };
 
 static int __init fail_iommu_setup(void)
 {
 #ifdef CONFIG_PCI
-	bus_register_notifier(&pci_bus_type, &fail_iommu_bus_notifier);
+	bus_register_notifier(&pci_bus_type, &fail_iommu_pci_bus_notifier);
 #endif
 #ifdef CONFIG_IBMVIO
-	bus_register_notifier(&vio_bus_type, &fail_iommu_bus_notifier);
+	bus_register_notifier(&vio_bus_type, &fail_iommu_vio_bus_notifier);
 #endif
 
 	return 0;
-- 
2.39.2

Re: [PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses

[Andrew Donnellan]

On Wed, 2023-03-22 at 14:53 +1100, Russell Currey wrote:
> fail_iommu_setup() registers the fail_iommu_bus_notifier struct to
> both
> PCI and VIO buses.  struct notifier_block is a linked list node, so
> this
> causes any notifiers later registered to either bus type to also be
> registered to the other since they share the same node.
> 
> This causes issues in (at least) the vgaarb code, which registers a
> notifier for PCI buses.  pci_notify() ends up being called on a vio
> device, converted with to_pci_dev() even though it's not a PCI
> device,
> and finally makes a bad access in vga_arbiter_add_pci_device() as
> discovered with KASAN:
> 
>  BUG: KASAN: slab-out-of-bounds in
> vga_arbiter_add_pci_device+0x60/0xe00
>  Read of size 4 at addr c000000264c26fdc by task swapper/0/1
> 
>  Call Trace:
>  [c000000263607520] [c000000010f7023c] dump_stack_lvl+0x1bc/0x2b8
> (unreliable)
>  [c000000263607560] [c00000000f142a64] print_report+0x3f4/0xc60
>  [c000000263607640] [c00000000f142144] kasan_report+0x244/0x698
>  [c000000263607740] [c00000000f1460e8] __asan_load4+0xe8/0x250
>  [c000000263607760] [c00000000ff4b850]
> vga_arbiter_add_pci_device+0x60/0xe00
>  [c000000263607850] [c00000000ff4c678] pci_notify+0x88/0x444
>  [c0000002636078b0] [c00000000e94dfc4]
> notifier_call_chain+0x104/0x320
>  [c000000263607950] [c00000000e94f050]
> blocking_notifier_call_chain+0xa0/0x140
>  [c000000263607990] [c0000000100cb3b8] device_add+0xac8/0x1d30
>  [c000000263607aa0] [c0000000100ccd98] device_register+0x58/0x80
>  [c000000263607ad0] [c00000000e84247c]
> vio_register_device_node+0x9ac/0xce0
>  [c000000263607ba0] [c0000000126c95d8]
> vio_bus_scan_register_devices+0xc4/0x13c
>  [c000000263607bd0] [c0000000126c96e4]
> __machine_initcall_pseries_vio_device_init+0x94/0xf0
>  [c000000263607c00] [c00000000e69467c] do_one_initcall+0x12c/0xaa8
>  [c000000263607cf0] [c00000001268b8a8]
> kernel_init_freeable+0xa48/0xba8
>  [c000000263607dd0] [c00000000e695f24] kernel_init+0x64/0x400
>  [c000000263607e50] [c00000000e68e0e4]
> ret_from_kernel_thread+0x5c/0x64
> 
> Fix this by creating separate notifier_block structs for each bus
> type.
> 
> Fixes: d6b9a81b2a45 ("powerpc: IOMMU fault injection")
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Russell Currey <ruscur@russell.cc>

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>


-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited

Re: [PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses

[R Nageswara Sastry]

On 22/03/23 9:23 am, Russell Currey wrote:
> fail_iommu_setup() registers the fail_iommu_bus_notifier struct to both
> PCI and VIO buses.  struct notifier_block is a linked list node, so this
> causes any notifiers later registered to either bus type to also be
> registered to the other since they share the same node.
> 
> This causes issues in (at least) the vgaarb code, which registers a
> notifier for PCI buses.  pci_notify() ends up being called on a vio
> device, converted with to_pci_dev() even though it's not a PCI device,
> and finally makes a bad access in vga_arbiter_add_pci_device() as
> discovered with KASAN:
> 
>   BUG: KASAN: slab-out-of-bounds in vga_arbiter_add_pci_device+0x60/0xe00
>   Read of size 4 at addr c000000264c26fdc by task swapper/0/1
> 
>   Call Trace:
>   [c000000263607520] [c000000010f7023c] dump_stack_lvl+0x1bc/0x2b8 (unreliable)
>   [c000000263607560] [c00000000f142a64] print_report+0x3f4/0xc60
>   [c000000263607640] [c00000000f142144] kasan_report+0x244/0x698
>   [c000000263607740] [c00000000f1460e8] __asan_load4+0xe8/0x250
>   [c000000263607760] [c00000000ff4b850] vga_arbiter_add_pci_device+0x60/0xe00
>   [c000000263607850] [c00000000ff4c678] pci_notify+0x88/0x444
>   [c0000002636078b0] [c00000000e94dfc4] notifier_call_chain+0x104/0x320
>   [c000000263607950] [c00000000e94f050] blocking_notifier_call_chain+0xa0/0x140
>   [c000000263607990] [c0000000100cb3b8] device_add+0xac8/0x1d30
>   [c000000263607aa0] [c0000000100ccd98] device_register+0x58/0x80
>   [c000000263607ad0] [c00000000e84247c] vio_register_device_node+0x9ac/0xce0
>   [c000000263607ba0] [c0000000126c95d8] vio_bus_scan_register_devices+0xc4/0x13c
>   [c000000263607bd0] [c0000000126c96e4] __machine_initcall_pseries_vio_device_init+0x94/0xf0
>   [c000000263607c00] [c00000000e69467c] do_one_initcall+0x12c/0xaa8
>   [c000000263607cf0] [c00000001268b8a8] kernel_init_freeable+0xa48/0xba8
>   [c000000263607dd0] [c00000000e695f24] kernel_init+0x64/0x400
>   [c000000263607e50] [c00000000e68e0e4] ret_from_kernel_thread+0x5c/0x64
> 
> Fix this by creating separate notifier_block structs for each bus type.
> 
> Fixes: d6b9a81b2a45 ("powerpc: IOMMU fault injection")
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Russell Currey <ruscur@russell.cc>


Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

> ---
>   arch/powerpc/kernel/iommu.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index ee95937bdaf1..6f1117fe3870 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -171,17 +171,26 @@ static int fail_iommu_bus_notify(struct notifier_block *nb,
>   	return 0;
>   }
>   
> -static struct notifier_block fail_iommu_bus_notifier = {
> +/*
> + * PCI and VIO buses need separate notifier_block structs, since they're linked
> + * list nodes.  Sharing a notifier_block would mean that any notifiers later
> + * registered for PCI buses would also get called by VIO buses and vice versa.
> + */
> +static struct notifier_block fail_iommu_pci_bus_notifier = {
> +	.notifier_call = fail_iommu_bus_notify
> +};
> +
> +static struct notifier_block fail_iommu_vio_bus_notifier = {
>   	.notifier_call = fail_iommu_bus_notify
>   };
>   
>   static int __init fail_iommu_setup(void)
>   {
>   #ifdef CONFIG_PCI
> -	bus_register_notifier(&pci_bus_type, &fail_iommu_bus_notifier);
> +	bus_register_notifier(&pci_bus_type, &fail_iommu_pci_bus_notifier);
>   #endif
>   #ifdef CONFIG_IBMVIO
> -	bus_register_notifier(&vio_bus_type, &fail_iommu_bus_notifier);
> +	bus_register_notifier(&vio_bus_type, &fail_iommu_vio_bus_notifier);
>   #endif
>   
>   	return 0;

-- 
Thanks and Regards
R.Nageswara Sastry

Re: [PATCH] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses

[Michael Ellerman]

On Wed, 22 Mar 2023 14:53:22 +1100, Russell Currey wrote:
> fail_iommu_setup() registers the fail_iommu_bus_notifier struct to both
> PCI and VIO buses.  struct notifier_block is a linked list node, so this
> causes any notifiers later registered to either bus type to also be
> registered to the other since they share the same node.
> 
> This causes issues in (at least) the vgaarb code, which registers a
> notifier for PCI buses.  pci_notify() ends up being called on a vio
> device, converted with to_pci_dev() even though it's not a PCI device,
> and finally makes a bad access in vga_arbiter_add_pci_device() as
> discovered with KASAN:
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/iommu: Fix notifiers being shared by PCI and VIO buses
      https://git.kernel.org/powerpc/c/c37b6908f7b2bd24dcaaf14a180e28c9132b9c58

cheers