All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: akpm@linux-foundation.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, ebiederm@xmission.com
Subject: Re: [PATCH] sysrq: Simplify sysrq-c handler
Date: Tue, 5 May 2009 10:23:59 -0400	[thread overview]
Message-ID: <20090505142359.GB9909@redhat.com> (raw)
In-Reply-To: <20090505134547.GA11780@hmsreliant.think-freely.org>

On Tue, May 05, 2009 at 09:45:47AM -0400, Neil Horman wrote:
> Currently the sysrq-c handler is bit over-engineered.  Its behavior is dependent
> on a few compile time and run time factors that alter its behavior which is
> really unnecessecary.  If CONFIG_KEXEC is not configured, sysrq-c, crashes the
> system with a NULL pointer dereference.  If CONFIG_KEXEC is configured, it calls
> crash_kexec directly, which implies that the kexec kernel will either be booted
> (if its been previously loaded), or it will simply do nothing (the no kexec
> kernel has been loaded).  It would be much easier to just simplify the whole
> thing to dereference a NULL pointer all the time regardless of configuration.
> That way, it will always try to crash the system, and if a kexec kernel has been
> loaded into reserved space, it will still boot from the page fault trap handler
> (assuming panic_on_oops is set appropriately).
> 

Neil,

Would it make sense to call panic() directly so that we are not dependent
on panic_on_oops being set?

Thanks
Vivek

> Neil
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 
>  sysrq.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> index b0a6a3e..9319e75 100644
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/sysrq.c
> @@ -120,20 +120,17 @@ static struct sysrq_key_op sysrq_unraw_op = {
>  #define sysrq_unraw_op (*(struct sysrq_key_op *)0)
>  #endif /* CONFIG_VT */
>  
> -#ifdef CONFIG_KEXEC
> -static void sysrq_handle_crashdump(int key, struct tty_struct *tty)
> +static void sysrq_handle_crash(int key, struct tty_struct *tty)
>  {
> -	crash_kexec(get_irq_regs());
> +	void *killer = NULL;
> +	*killer = 1;
>  }
>  static struct sysrq_key_op sysrq_crashdump_op = {
> -	.handler	= sysrq_handle_crashdump,
> -	.help_msg	= "Crashdump",
> -	.action_msg	= "Trigger a crashdump",
> +	.handler	= sysrq_handle_crash,
> +	.help_msg	= "Crash",
> +	.action_msg	= "Trigger a crash",
>  	.enable_mask	= SYSRQ_ENABLE_DUMP,
>  };
> -#else
> -#define sysrq_crashdump_op (*(struct sysrq_key_op *)0)
> -#endif
>  
>  static void sysrq_handle_reboot(int key, struct tty_struct *tty)
>  {

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	ebiederm@xmission.com, akpm@linux-foundation.org
Subject: Re: [PATCH] sysrq: Simplify sysrq-c handler
Date: Tue, 5 May 2009 10:23:59 -0400	[thread overview]
Message-ID: <20090505142359.GB9909@redhat.com> (raw)
In-Reply-To: <20090505134547.GA11780@hmsreliant.think-freely.org>

On Tue, May 05, 2009 at 09:45:47AM -0400, Neil Horman wrote:
> Currently the sysrq-c handler is bit over-engineered.  Its behavior is dependent
> on a few compile time and run time factors that alter its behavior which is
> really unnecessecary.  If CONFIG_KEXEC is not configured, sysrq-c, crashes the
> system with a NULL pointer dereference.  If CONFIG_KEXEC is configured, it calls
> crash_kexec directly, which implies that the kexec kernel will either be booted
> (if its been previously loaded), or it will simply do nothing (the no kexec
> kernel has been loaded).  It would be much easier to just simplify the whole
> thing to dereference a NULL pointer all the time regardless of configuration.
> That way, it will always try to crash the system, and if a kexec kernel has been
> loaded into reserved space, it will still boot from the page fault trap handler
> (assuming panic_on_oops is set appropriately).
> 

Neil,

Would it make sense to call panic() directly so that we are not dependent
on panic_on_oops being set?

Thanks
Vivek

> Neil
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 
>  sysrq.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> index b0a6a3e..9319e75 100644
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/sysrq.c
> @@ -120,20 +120,17 @@ static struct sysrq_key_op sysrq_unraw_op = {
>  #define sysrq_unraw_op (*(struct sysrq_key_op *)0)
>  #endif /* CONFIG_VT */
>  
> -#ifdef CONFIG_KEXEC
> -static void sysrq_handle_crashdump(int key, struct tty_struct *tty)
> +static void sysrq_handle_crash(int key, struct tty_struct *tty)
>  {
> -	crash_kexec(get_irq_regs());
> +	void *killer = NULL;
> +	*killer = 1;
>  }
>  static struct sysrq_key_op sysrq_crashdump_op = {
> -	.handler	= sysrq_handle_crashdump,
> -	.help_msg	= "Crashdump",
> -	.action_msg	= "Trigger a crashdump",
> +	.handler	= sysrq_handle_crash,
> +	.help_msg	= "Crash",
> +	.action_msg	= "Trigger a crash",
>  	.enable_mask	= SYSRQ_ENABLE_DUMP,
>  };
> -#else
> -#define sysrq_crashdump_op (*(struct sysrq_key_op *)0)
> -#endif
>  
>  static void sysrq_handle_reboot(int key, struct tty_struct *tty)
>  {

  reply	other threads:[~2009-05-05 14:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-05 13:45 [PATCH] sysrq: Simplify sysrq-c handler Neil Horman
2009-05-05 13:45 ` Neil Horman
2009-05-05 14:23 ` Vivek Goyal [this message]
2009-05-05 14:23   ` Vivek Goyal
2009-05-05 14:49   ` Neil Horman
2009-05-05 14:49     ` Neil Horman
2009-05-05 14:53     ` Vivek Goyal
2009-05-05 14:53       ` Vivek Goyal
2009-05-05 15:04       ` Brayan Arraes

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=20090505142359.GB9909@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.