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

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

[Thomas Tai]

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 <thomas.tai@oracle.com>
---
 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

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

[Thomas Tai]

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

Signed-off-by: Thomas Tai <thomas.tai@oracle.com>
---
 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

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

[Alexandre Chartre]

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 <thomas.tai@oracle.com>
> ---
>   arch/x86/kernel/traps.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

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

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

alex.

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

[Alexandre Chartre]

On 4/8/21 7:28 PM, Thomas Tai wrote:
> s/insted/instead/
> s/maintaing/maintaining/
> 
> Signed-off-by: Thomas Tai <thomas.tai@oracle.com>
> ---
>   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 <alexandre.chartre@oracle.com>

alex.

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

[tip-bot2 for Thomas Tai]

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 <thomas.tai@oracle.com>
AuthorDate:    Thu, 08 Apr 2021 13:28:33 -04:00
Committer:     Borislav Petkov <bp@suse.de>
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 <thomas.tai@oracle.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
Link: https://lkml.kernel.org/r/1617902914-83245-1-git-send-email-thomas.tai@oracle.com
---
 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));

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

[Thomas Tai]

> -----Original Message-----
> From: Alexandre CHARTRE <alexandre.chartre@oracle.com>
> Sent: April 9, 2021 5:06 AM
> To: Thomas Tai <thomas.tai@oracle.com>; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; x86@kernel.org
> Cc: sean.j.christopherson@intel.com; luto@amacapital.net;
> jarkko@kernel.org; bp@suse.de; jethro@fortanix.com; linux-
> kernel@vger.kernel.org
> 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 <thomas.tai@oracle.com>
> > ---
> >   arch/x86/kernel/traps.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

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.