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: <CAL_JsqL4cb8NVOV9QF5__PtjyDUQd-MnuhJhbTHmUF3qr3x47w@mail.gmail.com>
Date: Thu, 29 Feb 2024 15:50:04 -0600
From: Rob Herring <robh@...nel.org>
To: Peng Fan <peng.fan@....com>
Cc: "Peng Fan (OSS)" <peng.fan@....nxp.com>, "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

On Thu, Feb 29, 2024 at 2:01 AM Peng Fan <peng.fan@....com> wrote:
>
> > 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.

I understood the problem.

Your usage of this is broken on applying your overlay. You just got lucky.

> 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.

Again ^^^

> >
> > > 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]

$ git grep jailhouse_pci_virtual_root_devices_remove
(END)

Another out of tree overlay user. I have little interest in worrying
about them. No one wants to step up and solve the problems with
overlays, so I'm not going to worry about them either.

Rob

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ