Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   John Ogness <john.ogness@linutronix.de>
To:     Neil Armstrong <narmstrong@baylibre.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Petr Mladek <pmladek@suse.com>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-amlogic@lists.infradead.org
Subject: Re: [PATCH printk v5 1/1] printk: extend console_lock for
 per-console locking
In-Reply-To: <ca6b70a2-9bcc-fb86-200c-e1714c9d53be@baylibre.com>
References: <Ymfgis0EAw0Oxoa5@alley> <Ymfwk+X0CHq6ex3s@alley>
 <CGME20220427070833eucas1p27a32ce7c41c0da26f05bd52155f0031c@eucas1p2.samsung.com>
 <2a82eae7-a256-f70c-fd82-4e510750906e@samsung.com>
 <Ymjy3rHRenba7r7R@alley>
 <b6c1a8ac-c691-a84d-d3a1-f99984d32f06@samsung.com>
 <87fslyv6y3.fsf@jogness.linutronix.de>
 <51dfc4a0-f6cf-092f-109f-a04eeb240655@samsung.com>
 <87k0b6blz2.fsf@jogness.linutronix.de>
 <32bba8f8-dec7-78aa-f2e5-f62928412eda@samsung.com>
 <Ym/Z7PYPqvWPEjuL@alley>
 <45849b63-d7a8-5cc3-26ad-256a28d09991@samsung.com>
 <87pmktm2a9.fsf@jogness.linutronix.de>
 <87a6bwapij.fsf@jogness.linutronix.de>
 <87zgjvd2zb.fsf@jogness.linutronix.de>
 <b7c81f02-039e-e877-d7c3-6834728d2117@samsung.com>
 <ca6b70a2-9bcc-fb86-200c-e1714c9d53be@baylibre.com>
Date:   Sun, 08 May 2022 13:08:33 +0206
Message-ID: <871qx4qoc6.fsf@jogness.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Hi Neil,

On 2022-05-06, Neil Armstrong <narmstrong@baylibre.com> wrote:
> Thanks all for figuring out the issue, perhaps other uart drivers
> could fall in the same issue if startup code isn't protected with
> lock?

When preparing for the official patch submission [0], I needed quite a
bit of time to understand why another function (meson_uart_reset) should
not and cannot acquire the port->lock.

I then started investigating some other drivers and indeed I see lots of
potential problems. Any console initializing port->lock from the
driver's probe() is probably wrong (and there are lots of them). But as
I've learned with the meson driver, the details are subtle. Each driver
will need to be carefully evaluated to see if it is actually safe.

uart_ops->startup() is called without holding port->lock. If the device
is a console, it is already registered and printing.

driver->probe() is called without holding port->lock. If the device is a
console, it is already registered and printing.

For both functions, port->lock might not be initialized yet, so blindly
acquiring it is wrong.

Note that this is not related to the introduction of kthread printing.

I've put it on my TODO list to go through the ~76 console drivers to
investigate their startup() and probe() implementations. But I will not
be able to do this quickly. My time might be better spent writing to all
the maintainers asking them to please verify the usage.

John Ogness

[0] https://lore.kernel.org/lkml/20220508103547.626355-1-john.ogness@linutronix.de