2021-04-08 17:29:16

by Thomas Tai

[permalink] [raw]
Subject: [PATCH 1/2] x86/traps: call cond_local_irq_disable before returning from exc_general_protection and math_error

This fixes commit 334872a09198 ("x86/traps: Attempt to fixup exceptions
in vDSO before signaling") which added return statements without calling
cond_local_irq_disable(). According to commit ca4c6a9858c2
("x86/traps: Make interrupt enable/disable symmetric in C code"),
cond_local_irq_disable() is needed because the ASM return code no
longer disables interrupts. Follow the existing code as an example to
use "goto exit" instead of "return" statement.

Signed-off-by: Thomas Tai <[email protected]>
---
arch/x86/kernel/traps.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index ac1874a..651e3e5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -556,7 +556,7 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs,
tsk->thread.trap_nr = X86_TRAP_GP;

if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
- return;
+ goto exit;

show_signal(tsk, SIGSEGV, "", desc, regs, error_code);
force_sig(SIGSEGV);
@@ -1057,7 +1057,7 @@ static void math_error(struct pt_regs *regs, int trapnr)
goto exit;

if (fixup_vdso_exception(regs, trapnr, 0, 0))
- return;
+ goto exit;

force_sig_fault(SIGFPE, si_code,
(void __user *)uprobe_get_trap_addr(regs));
--
1.8.3.1


2021-04-08 17:29:34

by Thomas Tai

[permalink] [raw]
Subject: [PATCH 2/2] arch/x86: arch/sparc: tools/perf: fix typos in comments

s/insted/instead/
s/maintaing/maintaining/

Signed-off-by: Thomas Tai <[email protected]>
---
arch/sparc/vdso/vdso2c.c | 2 +-
arch/x86/entry/vdso/vdso2c.c | 2 +-
arch/x86/kernel/cpu/intel.c | 2 +-
tools/perf/arch/x86/util/perf_regs.c | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/vdso/vdso2c.c b/arch/sparc/vdso/vdso2c.c
index ab75041..70df9a8 100644
--- a/arch/sparc/vdso/vdso2c.c
+++ b/arch/sparc/vdso/vdso2c.c
@@ -192,7 +192,7 @@ int main(int argc, char **argv)

/*
* Figure out the struct name. If we're writing to a .so file,
- * generate raw output insted.
+ * generate raw output instead.
*/
name = strdup(argv[3]);
namelen = strlen(name);
diff --git a/arch/x86/entry/vdso/vdso2c.c b/arch/x86/entry/vdso/vdso2c.c
index 2d0f3d8..edfe978 100644
--- a/arch/x86/entry/vdso/vdso2c.c
+++ b/arch/x86/entry/vdso/vdso2c.c
@@ -218,7 +218,7 @@ int main(int argc, char **argv)

/*
* Figure out the struct name. If we're writing to a .so file,
- * generate raw output insted.
+ * generate raw output instead.
*/
name = strdup(argv[3]);
namelen = strlen(name);
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 0e422a5..63e381a 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -301,7 +301,7 @@ static void early_init_intel(struct cpuinfo_x86 *c)
* The operating system must reload CR3 to cause the TLB to be flushed"
*
* As a result, boot_cpu_has(X86_FEATURE_PGE) in arch/x86/include/asm/tlbflush.h
- * should be false so that __flush_tlb_all() causes CR3 insted of CR4.PGE
+ * should be false so that __flush_tlb_all() causes CR3 instead of CR4.PGE
* to be modified.
*/
if (c->x86 == 5 && c->x86_model == 9) {
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index fca81b3..207c568 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -165,7 +165,7 @@ static int sdt_init_op_regex(void)
/*
* Max x86 register name length is 5(ex: %r15d). So, 6th char
* should always contain NULL. This helps to find register name
- * length using strlen, insted of maintaing one more variable.
+ * length using strlen, instead of maintaining one more variable.
*/
#define SDT_REG_NAME_SIZE 6

@@ -207,7 +207,7 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op)
* and displacement 0 (Both sign and displacement 0 are
* optional so it may be empty). Use one more character
* to hold last NULL so that strlen can be used to find
- * prefix length, instead of maintaing one more variable.
+ * prefix length, instead of maintaining one more variable.
*/
char prefix[3] = {0};

--
1.8.3.1

2021-04-09 09:08:56

by Alexandre Chartre

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86/traps: call cond_local_irq_disable before returning from exc_general_protection and math_error


On 4/8/21 7:28 PM, Thomas Tai wrote:
> This fixes commit 334872a09198 ("x86/traps: Attempt to fixup exceptions
> in vDSO before signaling") which added return statements without calling
> cond_local_irq_disable(). According to commit ca4c6a9858c2
> ("x86/traps: Make interrupt enable/disable symmetric in C code"),
> cond_local_irq_disable() is needed because the ASM return code no
> longer disables interrupts. Follow the existing code as an example to
> use "goto exit" instead of "return" statement.
>
> Signed-off-by: Thomas Tai <[email protected]>
> ---
> arch/x86/kernel/traps.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Alexandre Chartre <[email protected]>

And it is probably worth adding a 'Fixes:' tag:

Fixes: 334872a09198 ("x86/traps: Attempt to fixup exceptions in vDSO before signaling")

alex.

2021-04-09 09:10:18

by Alexandre Chartre

[permalink] [raw]
Subject: Re: [PATCH 2/2] arch/x86: arch/sparc: tools/perf: fix typos in comments


On 4/8/21 7:28 PM, Thomas Tai wrote:
> s/insted/instead/
> s/maintaing/maintaining/
>
> Signed-off-by: Thomas Tai <[email protected]>
> ---
> arch/sparc/vdso/vdso2c.c | 2 +-
> arch/x86/entry/vdso/vdso2c.c | 2 +-
> arch/x86/kernel/cpu/intel.c | 2 +-
> tools/perf/arch/x86/util/perf_regs.c | 4 ++--
> 4 files changed, 5 insertions(+), 5 deletions(-)
>

Reviewed-by: Alexandre Chartre <[email protected]>

alex.

Subject: [tip: x86/urgent] x86/traps: Correct exc_general_protection() and math_error() return paths

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 632a1c209b8773cb0119fe3aada9f1db14fa357c
Gitweb: https://git.kernel.org/tip/632a1c209b8773cb0119fe3aada9f1db14fa357c
Author: Thomas Tai <[email protected]>
AuthorDate: Thu, 08 Apr 2021 13:28:33 -04:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Fri, 09 Apr 2021 13:45:09 +02:00

x86/traps: Correct exc_general_protection() and math_error() return paths

Commit

334872a09198 ("x86/traps: Attempt to fixup exceptions in vDSO before signaling")

added return statements which bypass calling cond_local_irq_disable().

According to

ca4c6a9858c2 ("x86/traps: Make interrupt enable/disable symmetric in C code"),

cond_local_irq_disable() is needed because the asm return code no longer
disables interrupts. Follow the existing code as an example to use "goto
exit" instead of "return" statement.

[ bp: Massage commit message. ]

Fixes: 334872a09198 ("x86/traps: Attempt to fixup exceptions in vDSO before signaling")
Signed-off-by: Thomas Tai <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Alexandre Chartre <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/traps.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index ac1874a..651e3e5 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -556,7 +556,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
tsk->thread.trap_nr = X86_TRAP_GP;

if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
- return;
+ goto exit;

show_signal(tsk, SIGSEGV, "", desc, regs, error_code);
force_sig(SIGSEGV);
@@ -1057,7 +1057,7 @@ static void math_error(struct pt_regs *regs, int trapnr)
goto exit;

if (fixup_vdso_exception(regs, trapnr, 0, 0))
- return;
+ goto exit;

force_sig_fault(SIGFPE, si_code,
(void __user *)uprobe_get_trap_addr(regs));

2021-04-09 13:11:19

by Thomas Tai

[permalink] [raw]
Subject: RE: [PATCH 1/2] x86/traps: call cond_local_irq_disable before returning from exc_general_protection and math_error

> -----Original Message-----
> From: Alexandre CHARTRE <[email protected]>
> Sent: April 9, 2021 5:06 AM
> To: Thomas Tai <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH 1/2] x86/traps: call cond_local_irq_disable before
> returning from exc_general_protection and math_error
>
>
> On 4/8/21 7:28 PM, Thomas Tai wrote:
> > This fixes commit 334872a09198 ("x86/traps: Attempt to fixup
> exceptions
> > in vDSO before signaling") which added return statements without
> calling
> > cond_local_irq_disable(). According to commit ca4c6a9858c2
> > ("x86/traps: Make interrupt enable/disable symmetric in C code"),
> > cond_local_irq_disable() is needed because the ASM return code no
> > longer disables interrupts. Follow the existing code as an example to
> > use "goto exit" instead of "return" statement.
> >
> > Signed-off-by: Thomas Tai <[email protected]>
> > ---
> > arch/x86/kernel/traps.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
>
> Reviewed-by: Alexandre Chartre <[email protected]>

Thank you Alex.

Thomas

>
> And it is probably worth adding a 'Fixes:' tag:
>
> Fixes: 334872a09198 ("x86/traps: Attempt to fixup exceptions in vDSO
> before signaling")
>
> alex.