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
| ||
|
Date: Mon, 26 Jul 2010 16:34:20 -0400 From: Neil Horman <nhorman@...driver.com> To: Dmitry Torokhov <dmitry.torokhov@...il.com> Cc: Xiaotian Feng <dfeng@...hat.com>, linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>, Andrew Morton <akpm@...ux-foundation.org>, "David S. Miller" <davem@...emloft.net> Subject: Re: [RFC PATCH] sysrq: don't hold the sysrq_key_table_lock during the handler On Mon, Jul 26, 2010 at 10:41:54AM -0700, Dmitry Torokhov wrote: > On Mon, Jul 26, 2010 at 06:51:48AM -0400, Neil Horman wrote: > > On Mon, Jul 26, 2010 at 05:54:02PM +0800, Xiaotian Feng wrote: > > > sysrq_key_table_lock is used to protect the sysrq_key_table, make sure > > > we get/replace the right operation for the sysrq. But in __handle_sysrq, > > > kernel will hold this lock and disable irqs until we finished op_p->handler(). > > > This may cause false positive watchdog alert when we're doing "show-task-states" > > > on a system with many tasks. > > > > > > Signed-off-by: Xiaotian Feng <dfeng@...hat.com> > > > Cc: Ingo Molnar <mingo@...e.hu> > > > Cc: Dmitry Torokhov <dmitry.torokhov@...il.com> > > > Cc: Andrew Morton <akpm@...ux-foundation.org> > > > Cc: Neil Horman <nhorman@...driver.com> > > > Cc: "David S. Miller" <davem@...emloft.net> > > > --- > > > drivers/char/sysrq.c | 4 +++- > > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c > > > index 878ac0c..0856e2e 100644 > > > --- a/drivers/char/sysrq.c > > > +++ b/drivers/char/sysrq.c > > > @@ -520,9 +520,11 @@ void __handle_sysrq(int key, struct tty_struct *tty, int check_mask) > > > if (!check_mask || sysrq_on_mask(op_p->enable_mask)) { > > > printk("%s\n", op_p->action_msg); > > > console_loglevel = orig_log_level; > > > + spin_unlock_irqrestore(&sysrq_key_table_lock, flags); > > > op_p->handler(key, tty); > > > } else { > > > printk("This sysrq operation is disabled.\n"); > > > + spin_unlock_irqrestore(&sysrq_key_table_lock, flags); > > > } > > > } else { > > > printk("HELP : "); > > > @@ -541,8 +543,8 @@ void __handle_sysrq(int key, struct tty_struct *tty, int check_mask) > > > } > > > printk("\n"); > > > console_loglevel = orig_log_level; > > > + spin_unlock_irqrestore(&sysrq_key_table_lock, flags); > > > } > > > - spin_unlock_irqrestore(&sysrq_key_table_lock, flags); > > > } > > > > > > void handle_sysrq(int key, struct tty_struct *tty) > > > -- > > > 1.7.2 > > > > > > > > > > This creates the possibility of a race in the handler. Not that it happens > > often, but sysrq keys can be registered and unregistered dynamically. If that > > lock isn't held while we call the keys handler, the code implementing that > > handler can live in a module that gets removed while its executing, leading to > > an oops, etc. I think the better solution would be to use an rcu lock here. > > I'd simply changed spinlock to a mutex. > I don't think you can do that safely in this path, as sysrqs will be looked up in both process (echo t > /proc/sysrq-trigger) context and in interrupt (alt-sysrq-t) context. If a mutex is locked and you try to take it in interrupt context, you get a sleeping-in-interrupt panic IIRC Neil > -- > Dmitry > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists