linux-kernel - Re: [PATCH] pci/aer_inject: switching inject_lock to raw_spinlock

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]

Message-ID: <CAH6oFv+KYGZNzb7gySoyQAB3tn2CrH+H_-vi4E=4NS6pvTBHvw@mail.gmail.com>
Date: Wed, 8 Oct 2025 00:10:45 +0800
From: guangbo cui <jckeep.cuiguangbo@...il.com>
To: Waiman Long <llong@...hat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams <clrkwllms@...nel.org>, 
	Steven Rostedt <rostedt@...dmis.org>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>, Boqun Feng <boqun.feng@...il.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Bjorn Helgaas <bhelgaas@...gle.com>, linux-rt-devel@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH] pci/aer_inject: switching inject_lock to raw_spinlock_t

On Tue, Oct 07, 2025 at 11:13:33AM -0400, Waiman Long wrote:
> On 10/7/25 2:02 AM, Guangbo Cui wrote:
> > [ 1850.947170] pcieport 0000:00:02.0: aer_inject: Injecting errors 00000001/00000000 into device 0000:00:02.0
> > [ 1850.949951]
> > [ 1850.950479] =============================
> > [ 1850.950780] [ BUG: Invalid wait context ]
> > [ 1850.951152] 6.17.0-11316-g7a405dbb0f03-dirty #7 Not tainted
> > [ 1850.951457] -----------------------------
> > [ 1850.951680] irq/16-PCIe PME/56 is trying to lock:
> > [ 1850.952004] ffff800082865238 (inject_lock){+.+.}-{3:3}, at: aer_inj_read_config+0x38/0x1dc
> > [ 1850.952731] other info that might help us debug this:
> > [ 1850.952997] context-{5:5}
> > [ 1850.953192] 5 locks held by irq/16-PCIe PME/56:
> > [ 1850.953415]  #0: ffff800082647390 (local_bh){.+.+}-{1:3}, at: __local_bh_disable_ip+0x30/0x268
> > [ 1850.953931]  #1: ffff8000826c6b38 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x48
> > [ 1850.954453]  #2: ffff000004bb6c58 (&data->lock){+...}-{3:3}, at: pcie_pme_irq+0x34/0xc4
> > [ 1850.954949]  #3: ffff8000826c6b38 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x48
> > [ 1850.955420]  #4: ffff800082863d10 (pci_lock){....}-{2:2}, at: pci_bus_read_config_dword+0x5c/0xd8
> > [ 1850.955932] stack backtrace:
> > [ 1850.956412] CPU: 0 UID: 0 PID: 56 Comm: irq/16-PCIe PME Not tainted 6.17.0-11316-g7a405dbb0f03-dirty #7 PREEMPT_{RT,(full)}
> > [ 1850.957039] Hardware name: linux,dummy-virt (DT)
> > [ 1850.957409] Call trace:
> > [ 1850.957727]  show_stack+0x18/0x24 (C)
> > [ 1850.958089]  dump_stack_lvl+0x40/0xbc
> > [ 1850.958339]  dump_stack+0x18/0x24
> > [ 1850.958586]  __lock_acquire+0xa84/0x3008
> > [ 1850.958907]  lock_acquire+0x128/0x2a8
> > [ 1850.959171]  rt_spin_lock+0x50/0x1b8
> > [ 1850.959476]  aer_inj_read_config+0x38/0x1dc
> > [ 1850.959821]  pci_bus_read_config_dword+0x80/0xd8
> > [ 1850.960079]  pcie_capability_read_dword+0xac/0xd8
> > [ 1850.960454]  pcie_pme_irq+0x44/0xc4
> > [ 1850.960728]  irq_forced_thread_fn+0x30/0x94
> > [ 1850.960984]  irq_thread+0x1ac/0x3a4
> > [ 1850.961308]  kthread+0x1b4/0x208
> > [ 1850.961557]  ret_from_fork+0x10/0x20
> > [ 1850.963088] pcieport 0000:00:02.0: AER: Correctable error message received from 0000:00:02.0
> > [ 1850.963330] pcieport 0000:00:02.0: PCIe Bus Error: severity=Correctable, type=Physical Layer, (Receiver ID)
> > [ 1850.963351] pcieport 0000:00:02.0:   device [1b36:000c] error status/mask=00000001/0000e000
> > [ 1850.963385] pcieport 0000:00:02.0:    [ 0] RxErr                  (First)
>
> Changing inject_lock into a raw_spinlock is the most obvious solution as
> long as it meets the criteria that the lock hold time is deterministic and
> relatively short and no other sleeping locks are being acquired down the
> locking chain.
>
> I am afraid that the these criteria are not met. First of all in
> aer_inject_exit(), inject_lock is acquired while iterating the a linked list
> which can last for while depending on how many items are in the list. This
> may be OK as long as it is guaranteed the list will not be long. Another
> problem is that it call kfree() while holding the lock. kfree() will likely
> acquire another rt_spin_lock which is a sleeping lock. You will have to
> consider pulling kfree() out from the lock critical section.

The list length depends on how many error injections the user performs,
which is typically small since this feature is mainly used for development
and debugging purposes. So the list traversal time should be acceptable
in practice.

Yeah, pulling kfree() out from the lock critical section is right, I
will fix it in the next version.

> Another function __find_aer_error() which does list iteration is called
> while holding inject_lock. Again this may be a problem. If the linked list
> can be long, you may have to consider breaking inject_lock into 2 or more
> separate locks to guard different data.

As mentioned above, the list is usually short in typical use cases,
since error injection is mainly used for debugging or development
purposes. Perhaps we can also get some advice from the PCI folks.

Best regards,
Guangbo