linux-kernel - Re: [PATCH v3] coresight: Fix possible deadlock in coresight_panic

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: <a35e2d54-f1f5-4ae4-9daa-ae1f3a8a302b@linux.dev>
Date: Tue, 16 Sep 2025 12:14:40 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Leo Yan <leo.yan@....com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, Yeoreum Yun <yeoreum.yun@....com>,
 Mike Leach <mike.leach@...aro.org>, Linu Cherian <lcherian@...vell.com>,
 linux-kernel@...r.kernel.org,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 James Clark <james.clark@...aro.org>
Subject: Re: [PATCH v3] coresight: Fix possible deadlock in coresight_panic_cb

On 9/16/25 12:00, Leo Yan wrote:
> On Mon, Sep 15, 2025 at 10:31:24AM -0400, Sean Anderson wrote:
>> On 9/15/25 05:58, Leo Yan wrote:
>> > On Fri, Sep 12, 2025 at 11:13:14AM -0400, Sean Anderson wrote:
>> >> coresight_panic_cb is called with interrupts disabled during panics.
>> >> However, bus_for_each_dev calls bus_to_subsys which takes
>> >> bus_kset->list_lock without disabling IRQs. This may cause a deadlock.
>> > 
>> > I would rephrase it to make it clearer for anyone reading it later:
>> > 
>> >   coresight_panic_cb() is called during panics, which can preempt a flow
>> >   that triggers exceptions (such as data or instruction aborts).
>> 
>> I don't see what exceptions have to do with it. You can also panic
>> during a regular interrupt.
> 
> The commit mentioned "without disabling IRQs" gives the impression that
> the deadlock is caused by IRQ-unsafe locking, which might mislead into
> thinking why the issue cannot be fixed with IRQ-safe locking.
> 
> Regardless of whether IRQs are disabled, and regardless of the context
> (interrupt, bottom-half, or normal thread), the conditions for the
> deadlock are only about:
> 
>   (a) The bus lock has been acquired;
>   (b) A panic is triggered to try to acquire the same lock.
> 
> [...]
> 
>> > When I review this patch, I recognize we can consolidate panic notifier
>> > in coresight-tmc-core.c, so we don't need to distribute the changes
>> > into ETF and ETR drivers (sorry if I misled you in my previous reply).
>> 
>> And this kind of thing is why I went with the straightforward fix
>> initially. I do not want to bikeshed the extent that this gets removed.
>> IMO the whole "panic ops" stuff should be done directly with the panic
>> notifier, hence this patch. If you do not agree with that, then ack v2
>> and send a follow up of your own to fix it how you see fit.
> 
> I would fix it in one go.
> 
> I agree with you that "the whole panic ops stuff should be done directly
> with the panic". The only difference between us is that I would keep the
> `panic_ops` callback. To me, this encapsulates panic callbacks into
> different modules, to make the code more general.
> 
> Could you check if the drafted patch below looks good to you? If so, I

As stated above I disagree with a half-hearted removal. If you want to do that,
then I will resend v2 done with an rcu list and you can make your own follow-up.

--Sean