mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
printk: Avoid multiple unregistration in unregister_console_locked()
unregister_console_locked() is called from within
hlist_for_each_entry_safe(console_list) loops. This would be safe if
unregister_console_locked() only unregistered the specified console. After
all, that is what the _safe variant is for.
However, unregister_console_locked() calls printk_kthreads_check_locked(),
which can unregister _multiple_ consoles if kthreads fail to start. This
could lead to hlist_for_each_entry_safe() use-after-free scenarios because
not only was the list entry removed, but possibly the following entry as
well (the @n temporary storage of the next entry).
Move the call to printk_kthreads_check_locked() outside of
unregister_console_locked(). Add comments to both functions to clarify
if they are safe to be used in hlist_for_each_entry_safe(console_list)
loops.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/lkml/87ik31iz4x.fsf@jogness.linutronix.de
Fixes: 76f258bf3f ("printk: nbcon: Introduce printer kthreads")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Derek Barbosa <debarbos@redhat.com>
Link: https://patch.msgid.link/20261030135802.663893-1-john.ogness@linutronix.de
Signed-off-by: Petr Mladek <pmladek@suse.com>
This commit is contained in:
parent
9ead26a759
commit
8a7f26aa07
|
|
@ -3792,6 +3792,13 @@ static struct syscore printk_syscore = {
|
|||
* If appropriate, start nbcon kthreads and set @printk_kthreads_running.
|
||||
* If any kthreads fail to start, those consoles are unregistered.
|
||||
*
|
||||
* Called after consoles are registered or unregistered.
|
||||
*
|
||||
* This MUST NOT be called within an hlist_for_each_entry_safe(console_list)
|
||||
* loop because it can unregister multiple consoles if any kthreads fail to
|
||||
* start. That would lead to use-after-free scenarios within the
|
||||
* hlist_for_each_entry_safe(console_list) loop of the caller.
|
||||
*
|
||||
* Must be called under console_list_lock().
|
||||
*/
|
||||
static void printk_kthreads_check_locked(void)
|
||||
|
|
@ -4230,7 +4237,20 @@ void register_console(struct console *newcon)
|
|||
}
|
||||
EXPORT_SYMBOL(register_console);
|
||||
|
||||
/* Must be called under console_list_lock(). */
|
||||
/*
|
||||
* Unregister the specified console.
|
||||
*
|
||||
* This can be called safely from within an
|
||||
* hlist_for_each_entry_safe(console_list) loop because it unregisters
|
||||
* _only_ the specified console.
|
||||
*
|
||||
* Afterwards the caller should call printk_kthreads_check_locked() since
|
||||
* unregistering a console may require printer threads to start/stop. Note
|
||||
* that printk_kthreads_check_locked() cannot be called safely from within
|
||||
* an hlist_for_each_entry_safe(console_list) loop.
|
||||
*
|
||||
* Must be called under console_list_lock().
|
||||
*/
|
||||
static int unregister_console_locked(struct console *console)
|
||||
{
|
||||
bool use_device_lock = (console->flags & CON_NBCON) && console->write_atomic;
|
||||
|
|
@ -4322,9 +4342,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 +4350,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;
|
||||
}
|
||||
|
|
@ -4472,6 +4494,10 @@ 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();
|
||||
|
||||
ret = cpuhp_setup_state_nocalls(CPUHP_PRINTK_DEAD, "printk:dead", NULL,
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user