linux-kernel - Re: [PATCH printk-rework 08/14] printk: add syslog

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]

Date:   Fri, 19 Feb 2021 17:33:13 +0100
From:   John Ogness <john.ogness@...utronix.de>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org, linux-parisc@...r.kernel.org
Subject: Re: [PATCH printk-rework 08/14] printk: add syslog_lock

Added CC: linux-parisc@...r.kernel.org

On 2021-02-19, John Ogness <john.ogness@...utronix.de> wrote:
>>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>>> index 20c21a25143d..401df370832b 100644
>>> --- a/kernel/printk/printk.c
>>> +++ b/kernel/printk/printk.c
>>> +/* Return a consistent copy of @syslog_seq. */
>>> +static u64 read_syslog_seq_irq(void)
>>> +{
>>> +	u64 seq;
>>> +
>>> +	raw_spin_lock_irq(&syslog_lock);
>>> +	seq = syslog_seq;
>>> +	raw_spin_unlock_irq(&syslog_lock);
>>
>> Is there any particular reason to disable interrupts here?
>>
>> It would make sense only when the lock could be taken in IRQ
>> context. Then we would need to always disable interrupts when
>> the lock is taken. And if it is taken in IRQ context, we would
>> need to safe flags.
>
> All other instances of locking @syslog_lock are done with interrupts
> disabled. And we have:
>
> register_console()
>   logbuf_lock_irqsave()
>     raw_spin_lock(&syslog_lock)
>
> I suppose I need to go through all the console drivers to see if any
> register in interrupt context. If not, that logbuf_lock_irqsave()
> should be replaced with logbuf_lock_irq(). And then locking
> @syslog_lock will not need to disable interrupts.

I found a possible call chain in interrupt context. From arch/parisc
there is the interrupt handler:

handle_interruption(code=1) /* High-priority machine check (HPMC) */
  pdc_console_restart()
    pdc_console_init_force()
      register_console()

All other register_console() calls in the kernel are either during init
(within __init sections and probe functions) or are clearly not in
interrupt context (using mutex, kzalloc, spin_lock_irq, etc).

I am not familiar with parisc, but I am assuming handle_interruption()
is always called with interrupts disabled (unless the HPMC interrupt is
somehow an exception).

John Ogness