From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
Steven Rostedt <rostedt@goodmis.org>,
Derek Barbosa <debarbos@redhat.com>,
linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@kernel.org>
Subject: Re: [PATCH printk v2] printk: Postpone kthread startup
Date: Fri, 16 Oct 2026 15:00:54 +0206 [thread overview]
Message-ID: <87ik31iz4x.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20261016085441.857695-1-john.ogness@linutronix.de>
On 2026-10-16, sashiko.dev wrote:
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 0ed86b5e8b160..9a205f94bfac7 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
>>
>> return 0;
>> }
>> -early_initcall(printk_set_kthreads_ready);
>> +subsys_initcall(printk_set_kthreads_ready);
>
> Can this delay expose a regression where a use-after-free occurs
> during failure paths?
>
> If nbcon_kthread_create(con) fails, it calls
> unregister_console_locked(con). This unregisters the console and
> recursively calls printk_kthreads_check_locked().
>
> The recursive call restarts iterating over console_list and may
> unregister subsequent consoles, including the tmp node held by the
> outer loop:
>
> kernel/printk/printk.c:printk_kthreads_check_locked() {
> hlist_for_each_entry_safe(con, tmp, &console_list, node) {
> if (!(con->flags & CON_NBCON))
> continue;
>
> if (!nbcon_kthread_create(con))
> unregister_console_locked(con);
> }
> }
>
> kernel/printk/printk.c:unregister_console_locked() {
> ...
> /* Changed console list, may require printer threads to start/stop. */
> printk_kthreads_check_locked();
> ...
> }
>
> Because hlist_for_each_entry_safe relies on the tmp pointer, the outer
> loop will later process the already unregistered tmp console and call
> nbcon_kthread_create(tmp) on it.
>
> If this succeeds on the retry, it spawns a zombie kthread for a
> detached console. If the detached console is dynamically allocated and
> freed by its driver's exit routine, the kthread and the outer loop
> will dereference freed memory.
>
> Can the loop in printk_kthreads_check_locked() handle the recursive
> unregistration of the tmp pointer safely without resulting in a
> use-after-free?
The case Sashiko presents is possible (even if unlikely). I would not
associate it with this patch though. It is a pre-existing
condition. Probably the cleanest fix is to pull
printk_kthreads_check_locked() out of
unregister_console_locked(). Something like this:
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0ed86b5e8b160..0a87f49b3908a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4322,9 +4322,6 @@ static int unregister_console_locked(struct console *console)
if (console->exit)
res = console->exit(console);
- /* Changed console list, may require printer threads to start/stop. */
- printk_kthreads_check_locked();
-
return res;
}
@@ -4333,7 +4330,12 @@ int unregister_console(struct console *console)
int res;
console_list_lock();
+
res = unregister_console_locked(console);
+
+ /* Changed console list, may require printer threads to start/stop. */
+ printk_kthreads_check_locked();
+
console_list_unlock();
return res;
}
@@ -4471,6 +4473,9 @@ static int __init printk_late_init(void)
}
unregister_console_locked(con);
+
+ /* Changed console list, may require printer threads to start/stop. */
+ printk_kthreads_check_locked();
}
console_list_unlock();
I can submit an official patch after the 7.4 merge window.
John
next prev parent reply other threads:[~2026-10-16 12:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-16 8:54 John Ogness
2026-10-16 12:54 ` John Ogness [this message]
2026-10-16 14:43 ` Petr Mladek
2026-10-16 15:56 ` Derek Barbosa
2026-10-16 13:36 ` Derek Barbosa
2026-10-16 15:06 ` Petr Mladek
2026-10-16 16:51 ` Derek Barbosa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ik31iz4x.fsf@jogness.linutronix.de \
--to=john.ogness@linutronix.de \
--cc=debarbos@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=tglx@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®