Re: [PATCH printk v3 33/40] printk, xen: fbfront: create/use safe function for forcing preferred
From: Petr Mladek <pmladek@suse.com>
Date: 2022-11-10 15:35:06
Also in:
dri-devel, lkml
On Mon 2022-11-07 15:22:31, John Ogness wrote:
quoted hunk ↗ jump to hunk
With commit 9e124fe16ff2("xen: Enable console tty by default in domU if it's not a dummy") a hack was implemented to make sure that the tty console remains the console behind the /dev/console device. The main problem with the hack is that, after getting the console pointer to the tty console, it is assumed the pointer is still valid after releasing the console_sem. This assumption is incorrect and unsafe. Make the hack safe by introducing a new function console_force_preferred_locked() and perform the full operation under the console_list_lock.--- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c@@ -3457,6 +3458,43 @@ int unregister_console(struct console *console) } EXPORT_SYMBOL(unregister_console); +/** + * console_force_preferred_locked - force a registered console preferred + * @con: The registered console to force preferred. + * + * Must be called under console_list_lock(). + */ +void console_force_preferred_locked(struct console *con) +{ + struct console *cur_pref_con; + + if (!console_is_registered_locked(con)) + return; + + cur_pref_con = console_first(); + + /* Already preferred? */ + if (cur_pref_con == con) + return; + + hlist_del_init_rcu(&con->node);
We actually should re-initialize the node only after all existing console list walks are finished. Se we should use here: hlist_del_rcu(&con->node);
+ + /* + * Ensure that all SRCU list walks have completed so that the console + * can be added to the beginning of the console list and its forward + * list pointer can be re-initialized.
The comment is right ;-)
+ */ + synchronize_srcu(&console_srcu); + + con->flags |= CON_CONSDEV; + WARN_ON(!con->device); + + /* Only the new head can have CON_CONSDEV set. */ + WRITE_ONCE(cur_pref_con->flags, cur_pref_con->flags & ~CON_CONSDEV);
As mentioned in the reply for 7th patch, I would prefer to hide this WRITE_ONCE into a wrapper, e.g. console_set_flag(). It might also check that the console_list_lock is taken...
+ hlist_add_behind_rcu(&con->node, console_list.first); +} +EXPORT_SYMBOL(console_force_preferred_locked); + /* * Initialize the console device. This is called *early*, so * we can't necessarily depend on lots of kernel help here.
Best Regards, Petr