linux-kernel - Re: [RFC][PATCH] serial: amba-pl011: Make sure we initialize the port.lock 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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALAqxLWxKfebXTy+p=sbUq3wz8Hx1zMctnuO4H2H2e9LMpcoeA@mail.gmail.com>
Date:   Thu, 23 Apr 2020 21:00:04 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Valentin Schneider <valentin.schneider@....com>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Russell King <linux@...linux.org.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org
Subject: Re: [RFC][PATCH] serial: amba-pl011: Make sure we initialize the
 port.lock spinlock

On Thu, Apr 23, 2020 at 4:14 PM Valentin Schneider
<valentin.schneider@....com> wrote:
> On 23/04/20 23:00, John Stultz wrote:
> > Which seems to be due to the fact that after allocating the uap
> > structure, the pl011 code doesn't initialize the spinlock.
> >
> > This patch fixes it by initializing the spinlock and the warning
> > has gone away.
> >
>
> Thanks for having a look. It does seem like the reasonable thing to do, and
> I no longer get the warning on h960.
>
> That said, I got more curious as this doesn't show up on my Juno (same
> Image). Digging into it I see that uart_add_one_port() has a call to
> uart_port_spin_lock_init() a few lines before uart_configure_port() (in
> which the above warning gets triggered). That thing says:
>
>  * Ensure that the serial console lock is initialised early.
>  * If this port is a console, then the spinlock is already initialised.
>
> Which requires me to ask: are we doing the right thing here?

Hrm. All good points. Obviously from the details above the port->lock
initialization shouldn't be done in the way this patch proposes, but
I'm at a loss as to why we're hitting the issue. My guess is it is
tripping over the console case, but didn't get initialized there
properly?

I'll try to trace through the code to see more about what's going on.

thanks
-john