linux-kernel - Re: [PATCH] serial: 8250: drop lockdep annotation from serial8250_clear

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: <7d8ae4f8-8900-5a06-5b7b-d4a3aea0673e@kernel.org>
Date:   Mon, 14 Aug 2023 08:28:36 +0200
From:   Jiri Slaby <jirislaby@...nel.org>
To:     John Ogness <john.ogness@...utronix.de>, gregkh@...uxfoundation.org
Cc:     linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vlastimil Babka <vbabka@...e.cz>,
        Thomas Gleixner <tglx@...utronix.de>,
        Petr Mladek <pmladek@...e.com>
Subject: Re: [PATCH] serial: 8250: drop lockdep annotation from
 serial8250_clear_IER()

On 14. 08. 23, 8:15, John Ogness wrote:
> On 2023-08-11, "Jiri Slaby (SUSE)" <jirislaby@...nel.org> wrote:
>> The port lock is not always held when calling serial8250_clear_IER().
>> When an oops is in progress, the lock is tried to be taken and when it
>> is not, a warning is issued:
> 
> Yes, and that is a potential deadlock. The warning is correct.

Could you elaborate on how can not-taking a lock be a potential deadlock?

>> Therefore, remove the annotation as it doesn't hold for all invocations.
> 
> ... because those invocations are broken by design.

Perhaps. But the system is crashing. Better to emit something without 
the lock rather than nothing (and wait for the lock infinitely).

>> The other option would be to make the lockdep test conditional on
>> 'oops_in_progress' or pass 'locked' from serial8250_console_write(). I
>> don't think, that is worth it.
> 
> The proper thing to do is to fix the invocation. The upcoming atomic
> console implementation for the 8250 does exactly that.

So what does it do?

> If this patch gets accepted (which it appears it will be), I will revert
> it in my series implementing the 8250 atomic console.

That's fine as soon as the warning is not a problem.

thanks,
-- 
js
suse labs