linux-kernel - RE: [PATCH] of: dynamic: notify before revert

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 29 Feb 2024 08:01:36 +0000
From: Peng Fan <peng.fan@....com>
To: Rob Herring <robh@...nel.org>, "Peng Fan (OSS)" <peng.fan@....nxp.com>
CC: "saravanak@...gle.com" <saravanak@...gle.com>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "pali@...nel.org" <pali@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] of: dynamic: notify before revert

> Subject: Re: [PATCH] of: dynamic: notify before revert
> 
> On Wed, Feb 28, 2024 at 12:13 AM Peng Fan (OSS) <peng.fan@....nxp.com>
> wrote:
> >
> > From: Peng Fan <peng.fan@....com>
> >
> > When PCI node was created using an overlay and the overlay is
> > reverted/destroyed, the "linux,pci-domain" property no longer exists,
> > so of_get_pci_domain_nr will return failure. Then
> > of_pci_bus_release_domain_nr will actually use the dynamic IDA, even
> > if the IDA was allocated in static IDA.
> 
> That usage is broken to begin with unless there is a guarantee that static and
> dynamic domain numbers don't overlap. For example, a dynamic number is
> assigned and then you load an overlay with the same number defined in it.

I may not describe it clear, the code is here, because overlay property
removed, of_get_pci_domain_nr will return failure, so the code path
will goest into free a dynamic ida. But actually there is no such dynamic
ida, so dump.

So the problem is overlay was removed, but the notify callback may
still use the property to do some work.

static void of_pci_bus_release_domain_nr(struct pci_bus *bus, struct device *parent)                    
{                                                                                                       
        if (bus->domain_nr < 0)                                                                         
                return;                                                                                 
                                                                                                        
        /* Release domain from IDA where it was allocated. */                                           
        if (of_get_pci_domain_nr(parent->of_node) == bus->domain_nr)                                    
                ida_free(&pci_domain_nr_static_ida, bus->domain_nr);                                    
        else                                                                                            
                ida_free(&pci_domain_nr_dynamic_ida, bus->domain_nr);                                   
}
> 
> > So move the notify before revert to fix the issue.
> 
> You can't just change the timing. Something might require notify to be after
> the revert.
> 
> > Fixes: c14f7ccc9f5d ("PCI: Assign PCI domain IDs by ida_alloc()")
> 
> I don't see where the notifier is even used.

The stack is as below:

 [  595.150529] CPU: 1 PID: 582 Comm: jailhouse Tainted: G           O       6.5.0-rc4-next-20230804-05021-g3d4cc14b42ef-dirty #355
 [  595.161998] Hardware name: NXP i.MX93 11X11 EVK board (DT)
 [  595.167475] Call trace:
 [  595.169908]  dump_backtrace+0x94/0xec
 [  595.173573]  show_stack+0x18/0x24
 [  595.176884]  dump_stack_lvl+0x48/0x60
 [  595.180541]  dump_stack+0x18/0x24
 [  595.183843]  pci_bus_release_domain_nr+0x34/0x84
 [  595.188453]  pci_remove_root_bus+0xa0/0xa4
 [  595.192544]  pci_host_common_remove+0x28/0x40
 [  595.196895]  platform_remove+0x54/0x6c
 [  595.200641]  device_remove+0x4c/0x80
 [  595.204209]  device_release_driver_internal+0x1d4/0x230
 [  595.209430]  device_release_driver+0x18/0x24
 [  595.213691]  bus_remove_device+0xcc/0x10c
 [  595.217686]  device_del+0x15c/0x41c
 [  595.221170]  platform_device_del.part.0+0x1c/0x88
 [  595.225861]  platform_device_unregister+0x24/0x40
 [  595.230557]  of_platform_device_destroy+0xfc/0x10c
 [  595.235344]  of_platform_notify+0x13c/0x178
 [  595.239518]  blocking_notifier_call_chain+0x6c/0xa0
 [  595.244389]  __of_changeset_entry_notify+0x148/0x16c
 [  595.249346]  of_changeset_revert+0xa8/0xcc
 [  595.253437]  jailhouse_pci_virtual_root_devices_remove+0x50/0x74 [jailhouse]
 [  595.260484]  jailhouse_cmd_disable+0x70/0x1ac [jailhouse]
 [  595.265883]  jailhouse_ioctl+0x60/0xf0 [jailhouse]
 [  595.270674]  __arm64_sys_ioctl+0xac/0xf0
 [  595.274595]  invoke_syscall+0x48/0x114
[  595.278336]  el0_svc_common.constprop.0+0xc4/0xe4

Regards,
Peng.

> 
> Rob