linux-kernel - Re: 6.1-rc1 regression: bisected to 57365a04c921 iommu: Move bus setup to IOMMU device registration

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

Message-ID: <Y1F1bbaVX4/bmbS0@google.com>
Date:   Thu, 20 Oct 2022 09:21:01 -0700
From:   Brian Norris <briannorris@...omium.org>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Krishna Reddy <vdumpa@...dia.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Niklas Schnelle <schnelle@...ux.ibm.com>,
        Joerg Roedel <jroedel@...e.de>, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org
Subject: Re: 6.1-rc1 regression: bisected to 57365a04c921 iommu: Move bus
 setup to IOMMU device registration

On Thu, Oct 20, 2022 at 11:02:23AM +0100, Robin Murphy wrote:
> We shouldn't really need locking for iommu_buses itself, but I guess to
> support async probe we would need per-device locking in
> iommu_probe_device() to prevent multiple threads trying to probe the
> same device at once, which must be what's happening in your case to
> cause a double-dev_iommu_free().

Perhaps. I still haven't spent enough time trying to learn the expected
behavior here.

> I'll see what I can do ASAP, since I
> think that's worthwhile.

Awesome!

> In the meantime, as to why you're hitting the
> failure path at all, I think that's another subtle oversight on my part,

Ah, I didn't even pay attention to the fact we were hitting an error
path here.

The comments you move ("Use the first registered IOMMU device") might
suggest that some of the problems could be more fundamental to rk_iommu,
since with async probe, there is no such "first registerered device".
(Well, they get serialized into some lists eventually, but for the most
part we have to consider them unordered.)

> does something like the diff below help?

I get a different crash at least!

[    0.183048] Unable to handle kernel NULL pointer dereference at virtual address 00000000000002a0
...
[    0.183133] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc1+ #66
[    0.183143] Hardware name: Google Scarlet (DT)
...
[    0.183314] Call trace:
[    0.183319]  _raw_spin_lock_irqsave+0x44/0x9c
[    0.183328]  devres_add+0x34/0x64
[    0.183336]  devm_kmalloc+0xac/0xcc
[    0.183345]  rk_iommu_of_xlate+0x3c/0x80
[    0.183358]  of_iommu_xlate+0x8c/0xd0
[    0.183367]  of_iommu_configure+0x120/0x1d0
[    0.183377]  of_dma_configure_id+0x190/0x244
[    0.183387]  platform_dma_configure+0x40/0x88
[    0.183397]  really_probe+0xac/0x284
[    0.183409]  __driver_probe_device+0xc0/0xec
[    0.183419]  driver_probe_device+0x4c/0xd4
[    0.183429]  __driver_attach+0xb8/0x10c
[    0.183439]  bus_for_each_dev+0x8c/0xd8
[    0.183448]  driver_attach+0x30/0x3c
[    0.183458]  bus_add_driver+0x118/0x1f8
[    0.183467]  driver_register+0x70/0x10c
[    0.183475]  __platform_register_drivers+0x5c/0xcc
[    0.183484]  rockchip_drm_init+0x74/0xc4
[    0.183495]  do_one_initcall+0x154/0x2e0
[    0.183505]  do_initcall_level+0x134/0x160
[    0.183515]  do_initcalls+0x60/0xa0
[    0.183524]  do_basic_setup+0x28/0x34
[    0.183532]  kernel_init_freeable+0xf8/0x150
[    0.183541]  kernel_init+0x2c/0x12c
[    0.183551]  ret_from_fork+0x10/0x20
[    0.183567] Code: 5280002b 1100054a b900092a f9800011 (885ffc01)
[    0.183574] ---[ end trace 0000000000000000 ]---

Brian