2020-06-21 10:31:23

by Bo YU

Subject: [PATCH -next] arch/x86: Return value from notify_die should to be checked.

This is detected by Coverity scan: #CID: 1464472(CHECKED_RETURN)

FIXES: c94082656dac7(x86: Use enum instead of literals for trap values)
Signed-off-by: Bo YU <[email protected]>
---
arch/x86/kernel/traps.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index af75109485c2..bf014fb59017 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -401,7 +401,8 @@ DEFINE_IDTENTRY_DF(exc_double_fault)

nmi_enter();
instrumentation_begin();
- notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV);
+ if (notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV))
+ return;

tsk->thread.error_code = error_code;
tsk->thread.trap_nr = X86_TRAP_DF;
--
2.11.0

2020-06-22 08:55:36

by Alexandre Chartre

Subject: Re: [PATCH -next] arch/x86: Return value from notify_die should to be checked.

On 6/21/20 12:26 PM, Bo YU wrote:
> This is detected by Coverity scan: #CID: 1464472(CHECKED_RETURN)
>
> FIXES: c94082656dac7(x86: Use enum instead of literals for trap values)
> Signed-off-by: Bo YU <[email protected]>
> ---
> arch/x86/kernel/traps.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index af75109485c2..bf014fb59017 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -401,7 +401,8 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
>
>     nmi_enter();
>     instrumentation_begin();
> -    notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV);
> +    if (notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV))
> +        return;
>

This change is not correct, if there's a double fault then we should die even if
notify_die() fails. So the appropriate change to make Coverity happy is probably:

(void) notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_DF, SIGSEGV);

alex.

2020-06-22 09:21:59

by Borislav Petkov

Subject: Re: [PATCH -next] arch/x86: Return value from notify_die should to be checked.

On June 22, 2020 10:52:23 AM GMT+02:00, Alexandre Chartre <[email protected]> wrote:
> So the appropriate change to make Coverity happy

Or we can stop "fixing" the kernel in order to shut up tools and not do anything.


--
Sent from a small device: formatting sux and brevity is inevitable.

2020-06-22 09:50:57

by Peter Zijlstra

Subject: Re: [PATCH -next] arch/x86: Return value from notify_die should to be checked.

On Mon, Jun 22, 2020 at 11:17:51AM +0200, Boris Petkov wrote:
> On June 22, 2020 10:52:23 AM GMT+02:00, Alexandre Chartre <[email protected]> wrote:
> > So the appropriate change to make Coverity happy
>
> Or we can stop "fixing" the kernel in order to shut up tools and not do anything.

Agreed, no change required here.

2020-06-22 11:58:59

by Bo YU

Subject: Re: [PATCH -next] arch/x86: Return value from notify_die should to be checked.

On Mon, Jun 22, 2020 at 5:46 PM Peter Zijlstra <[email protected]> wrote:
>
> On Mon, Jun 22, 2020 at 11:17:51AM +0200, Boris Petkov wrote:
> > On June 22, 2020 10:52:23 AM GMT+02:00, Alexandre Chartre <[email protected]> wrote:
> > > So the appropriate change to make Coverity happy
> >
> > Or we can stop "fixing" the kernel in order to shut up tools and not do anything.
>
> Agreed, no change required here.
Ok, thanks for everyone.