All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Palmer Dabbelt <palmer@rivosinc.com>
Cc: linux-riscv@lists.infradead.org,
	kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH] RISC-V: Avoid dereferening NULL regs in die()
Date: Tue, 20 Sep 2022 22:36:59 +0100	[thread overview]
Message-ID: <YyoyewwUNMMUgCru@spud> (raw)
In-Reply-To: <20220920200037.6727-1-palmer@rivosinc.com>

On Tue, Sep 20, 2022 at 01:00:37PM -0700, Palmer Dabbelt wrote:
> I don't think we can actually die() without a regs pointer, but the
> compiler was warning about a NULL check after a dereference.  It seems
> prudent to just avoid the possibly-NULL dereference, given that when
> die()ing the system is already toast so who knows how we got there.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

I excitedly tried this in the hopes that I was actually hitting this
somehow with the crash-during-boot-with-no-splat that I am currently
bisecting, but sadly it was not the solution.

Either way it looks like a sane change so we at least can get something
out in an edge failure case.

> ---
>  arch/riscv/kernel/traps.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 635e6ec26938..f3e96d60a2ff 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -33,6 +33,7 @@ void die(struct pt_regs *regs, const char *str)
>  {
>  	static int die_counter;
>  	int ret;
> +	long cause;
>  
>  	oops_enter();
>  
> @@ -42,11 +43,13 @@ void die(struct pt_regs *regs, const char *str)
>  
>  	pr_emerg("%s [#%d]\n", str, ++die_counter);
>  	print_modules();
> -	show_regs(regs);
> +	if (regs)
> +		show_regs(regs);
>  
> -	ret = notify_die(DIE_OOPS, str, regs, 0, regs->cause, SIGSEGV);
> +	cause = regs ? regs->cause : -1;
> +	ret = notify_die(DIE_OOPS, str, regs, 0, cause, SIGSEGV);
>  
> -	if (regs && kexec_should_crash(current))
> +	if (kexec_should_crash(current))
>  		crash_kexec(regs);

Had to go down the rabbit hole to see that there is an if(regs) around
the eventual user of this, so:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

>  
>  	bust_spinlocks(0);
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2022-09-20 21:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-20 20:00 [PATCH] RISC-V: Avoid dereferening NULL regs in die() Palmer Dabbelt
2022-09-20 21:36 ` Conor Dooley [this message]
2022-10-13  5:15 ` Palmer Dabbelt

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=YyoyewwUNMMUgCru@spud \
    --to=conor@kernel.org \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lkp@intel.com \
    --cc=palmer@rivosinc.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.