Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1249197 > unrolled thread

[PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on

Started byAndy Lutomirski <luto@kernel.org>
First post2015-10-17 00:50 +0200
Last post2015-10-19 06:50 +0200
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on Andy Lutomirski <luto@kernel.org> - 2015-10-17 00:50 +0200
    Re: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect  interrupts to be on Brian Gerst <brgerst@gmail.com> - 2015-10-17 05:00 +0200
      Re: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect  interrupts to be on Andy Lutomirski <luto@amacapital.net> - 2015-10-17 06:10 +0200
    [tip:x86/asm] x86/entry/32: Fix entry_INT80_32()   to expect interrupts to be on tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2015-10-18 12:20 +0200
      Re: [tip:x86/asm] x86/entry/32: Fix entry_INT80_32() to expect  interrupts to be on Andy Lutomirski <luto@amacapital.net> - 2015-10-19 06:50 +0200

#1249197 — [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on

FromAndy Lutomirski <luto@kernel.org>
Date2015-10-17 00:50 +0200
Subject[PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on
Message-ID<qklZ0-3t5-3@gated-at.bofh.it>
When I rewrote entry_INT80_32, I thought that int80 was an interrupt
gate.  It's a trap gate.  *facepalm*

Thanks to Brian Gerst for pointing out that it's better to change
the entry code than to change the gate type.

Suggested-by: Brian Gerst <brgerst@gmail.com>
Reported-by: Borislav Petkov <bp@suse.de>
Fixes: 150ac78d63af ("x86/entry/32: Switch INT80 to the new C syscall path")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/common.c          | 15 ++++++++++++---
 arch/x86/entry/entry_32.S        |  8 ++++----
 arch/x86/entry/entry_64_compat.S |  2 +-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index b53e04d301a3..09afb3b6acbb 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -351,7 +351,14 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
  * in workloads that use it, and it's usually called from
  * do_fast_syscall_32, so forcibly inline it to improve performance.
  */
-static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
+#ifdef CONFIG_X86_32
+/* 32-bit kernels use a trap gate for int80, and the asm code calls here. */
+__visible
+#else
+/* 64-bit kernels use do_syscall_32_irqs_off instead. */
+static
+#endif
+__always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 {
 	struct thread_info *ti = pt_regs_to_thread_info(regs);
 	unsigned int nr = (unsigned int)regs->orig_ax;
@@ -386,12 +393,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 	syscall_return_slowpath(regs);
 }
 
-/* Handles int $0x80 */
-__visible void do_int80_syscall_32(struct pt_regs *regs)
+#ifdef CONFIG_X86_64
+/* Handles int $0x80 on 64-bit kernels */
+__visible void do_syscall_32_irqs_off(struct pt_regs *regs)
 {
 	local_irq_enable();
 	do_syscall_32_irqs_on(regs);
 }
+#endif
 
 /* Returns 0 to return using IRET or 1 to return using SYSEXIT/SYSRETL. */
 __visible long do_fast_syscall_32(struct pt_regs *regs)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 07a041b91af1..ba0dad937be9 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -346,13 +346,13 @@ ENTRY(entry_INT80_32)
 	SAVE_ALL pt_regs_ax=$-ENOSYS	/* save rest */
 
 	/*
-	 * User mode is traced as though IRQs are on, and the interrupt gate
-	 * turned them off.
+	 * User mode is traced as though IRQs are on.  Unlike the 64-bit
+	 * case, int80 is a trap gate on 32-bit kernels, so interrupts
+	 * are already on (unless user code is messing around with iopl).
 	 */
-	TRACE_IRQS_OFF
 
 	movl	%esp, %eax
-	call	do_int80_syscall_32
+	call	do_syscall_32_irqs_on
 .Lsyscall_32_done:
 
 restore_all:
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 78becafe60d1..dd160e4e2ef5 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -303,7 +303,7 @@ ENTRY(entry_INT80_compat)
 	TRACE_IRQS_OFF
 
 	movq	%rsp, %rdi
-	call	do_int80_syscall_32
+	call	do_syscall_32_irqs_off
 .Lsyscall_32_done:
 
 	/* Go back to user mode. */
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1249253 — Re: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on

FromBrian Gerst <brgerst@gmail.com>
Date2015-10-17 05:00 +0200
SubjectRe: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on
Message-ID<qkpSV-K2-1@gated-at.bofh.it>
In reply to#1249197
On Fri, Oct 16, 2015 at 6:42 PM, Andy Lutomirski <luto@kernel.org> wrote:
> When I rewrote entry_INT80_32, I thought that int80 was an interrupt
> gate.  It's a trap gate.  *facepalm*
>
> Thanks to Brian Gerst for pointing out that it's better to change
> the entry code than to change the gate type.
>
> Suggested-by: Brian Gerst <brgerst@gmail.com>
> Reported-by: Borislav Petkov <bp@suse.de>
> Fixes: 150ac78d63af ("x86/entry/32: Switch INT80 to the new C syscall path")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/entry/common.c          | 15 ++++++++++++---
>  arch/x86/entry/entry_32.S        |  8 ++++----
>  arch/x86/entry/entry_64_compat.S |  2 +-
>  3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index b53e04d301a3..09afb3b6acbb 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -351,7 +351,14 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
>   * in workloads that use it, and it's usually called from
>   * do_fast_syscall_32, so forcibly inline it to improve performance.
>   */
> -static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
> +#ifdef CONFIG_X86_32
> +/* 32-bit kernels use a trap gate for int80, and the asm code calls here. */
> +__visible
> +#else
> +/* 64-bit kernels use do_syscall_32_irqs_off instead. */
> +static
> +#endif
> +__always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
>  {
>         struct thread_info *ti = pt_regs_to_thread_info(regs);
>         unsigned int nr = (unsigned int)regs->orig_ax;
> @@ -386,12 +393,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
>         syscall_return_slowpath(regs);
>  }
>
> -/* Handles int $0x80 */
> -__visible void do_int80_syscall_32(struct pt_regs *regs)
> +#ifdef CONFIG_X86_64
> +/* Handles int $0x80 on 64-bit kernels */
> +__visible void do_syscall_32_irqs_off(struct pt_regs *regs)
>  {
>         local_irq_enable();
>         do_syscall_32_irqs_on(regs);
>  }
> +#endif

This would be more readable if the STI were moved down into the asm
for 64-bit.  In fact, we should be re-enabling interrupts as early as
possible once the full kernel environment is set up (on the process
stack, NT clear, and after SWAPGS).  What was your reasoning for
moving it later?

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249258 — Re: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on

FromAndy Lutomirski <luto@amacapital.net>
Date2015-10-17 06:10 +0200
SubjectRe: [PATCH v2 1/2] x86/entry/32: Fix entry_INT80_32 to expect interrupts to be on
Message-ID<qkqYG-2y6-7@gated-at.bofh.it>
In reply to#1249253
On Oct 16, 2015 7:54 PM, "Brian Gerst" <brgerst@gmail.com> wrote:
>
> On Fri, Oct 16, 2015 at 6:42 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > When I rewrote entry_INT80_32, I thought that int80 was an interrupt
> > gate.  It's a trap gate.  *facepalm*
> >
> > Thanks to Brian Gerst for pointing out that it's better to change
> > the entry code than to change the gate type.
> >
> > Suggested-by: Brian Gerst <brgerst@gmail.com>
> > Reported-by: Borislav Petkov <bp@suse.de>
> > Fixes: 150ac78d63af ("x86/entry/32: Switch INT80 to the new C syscall path")
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  arch/x86/entry/common.c          | 15 ++++++++++++---
> >  arch/x86/entry/entry_32.S        |  8 ++++----
> >  arch/x86/entry/entry_64_compat.S |  2 +-
> >  3 files changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index b53e04d301a3..09afb3b6acbb 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -351,7 +351,14 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
> >   * in workloads that use it, and it's usually called from
> >   * do_fast_syscall_32, so forcibly inline it to improve performance.
> >   */
> > -static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
> > +#ifdef CONFIG_X86_32
> > +/* 32-bit kernels use a trap gate for int80, and the asm code calls here. */
> > +__visible
> > +#else
> > +/* 64-bit kernels use do_syscall_32_irqs_off instead. */
> > +static
> > +#endif
> > +__always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
> >  {
> >         struct thread_info *ti = pt_regs_to_thread_info(regs);
> >         unsigned int nr = (unsigned int)regs->orig_ax;
> > @@ -386,12 +393,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
> >         syscall_return_slowpath(regs);
> >  }
> >
> > -/* Handles int $0x80 */
> > -__visible void do_int80_syscall_32(struct pt_regs *regs)
> > +#ifdef CONFIG_X86_64
> > +/* Handles int $0x80 on 64-bit kernels */
> > +__visible void do_syscall_32_irqs_off(struct pt_regs *regs)
> >  {
> >         local_irq_enable();
> >         do_syscall_32_irqs_on(regs);
> >  }
> > +#endif
>
> This would be more readable if the STI were moved down into the asm
> for 64-bit.  In fact, we should be re-enabling interrupts as early as
> possible once the full kernel environment is set up (on the process
> stack, NT clear, and after SWAPGS).  What was your reasoning for
> moving it later?

On x86_64, we have to use an interrupt gate because of swapgs, and for
context tracking, once I clean up the SYSCALL64 entry, I want to make
it all the way to user_exit with IRQs off.   There are nice
optimizations that become possible once user_exit is always called
with IRQs off, and there's another cleanup we can do when IRQs are no
longer possible in kernel mode with IRQs on.

For x86_32, we don't actually support context tracking, but we could.
So maybe we should suck up the ~3 cycles for int80 users and just use
an interrupt gate everywhere.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250104 — [tip:x86/asm] x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on

Fromtip-bot for Andy Lutomirski <tipbot@zytor.com>
Date2015-10-18 12:20 +0200
Subject[tip:x86/asm] x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on
Message-ID<qkTeh-2al-7@gated-at.bofh.it>
In reply to#1249197
Commit-ID:  657c1eea0019e80685a84cbb1919794243a187c9
Gitweb:     http://git.kernel.org/tip/657c1eea0019e80685a84cbb1919794243a187c9
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 16 Oct 2015 15:42:54 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 18 Oct 2015 12:11:16 +0200

x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on

When I rewrote entry_INT80_32, I thought that int80 was an
interrupt gate.  It's a trap gate.  *facepalm*

Thanks to Brian Gerst for pointing out that it's better to
change the entry code than to change the gate type.

Suggested-by: Brian Gerst <brgerst@gmail.com>
Reported-and-tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 150ac78d63af ("x86/entry/32: Switch INT80 to the new C syscall path")
Link: http://lkml.kernel.org/r/dc09d9b574a5c1dcca996847875c73f8341ce0ad.1445035014.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/common.c          | 15 ++++++++++++---
 arch/x86/entry/entry_32.S        |  8 ++++----
 arch/x86/entry/entry_64_compat.S |  2 +-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index b53e04d..a89fdbc 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -351,7 +351,14 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
  * in workloads that use it, and it's usually called from
  * do_fast_syscall_32, so forcibly inline it to improve performance.
  */
-static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
+#ifdef CONFIG_X86_32
+/* 32-bit kernels use a trap gate for INT80, and the asm code calls here. */
+__visible
+#else
+/* 64-bit kernels use do_syscall_32_irqs_off() instead. */
+static
+#endif
+__always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 {
 	struct thread_info *ti = pt_regs_to_thread_info(regs);
 	unsigned int nr = (unsigned int)regs->orig_ax;
@@ -386,12 +393,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 	syscall_return_slowpath(regs);
 }
 
-/* Handles int $0x80 */
-__visible void do_int80_syscall_32(struct pt_regs *regs)
+#ifdef CONFIG_X86_64
+/* Handles INT80 on 64-bit kernels */
+__visible void do_syscall_32_irqs_off(struct pt_regs *regs)
 {
 	local_irq_enable();
 	do_syscall_32_irqs_on(regs);
 }
+#endif
 
 /* Returns 0 to return using IRET or 1 to return using SYSEXIT/SYSRETL. */
 __visible long do_fast_syscall_32(struct pt_regs *regs)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index c1c7c63..4f97f49 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -345,13 +345,13 @@ ENTRY(entry_INT80_32)
 	SAVE_ALL pt_regs_ax=$-ENOSYS	/* save rest */
 
 	/*
-	 * User mode is traced as though IRQs are on, and the interrupt gate
-	 * turned them off.
+	 * User mode is traced as though IRQs are on.  Unlike the 64-bit
+	 * case, INT80 is a trap gate on 32-bit kernels, so interrupts
+	 * are already on (unless user code is messing around with iopl).
 	 */
-	TRACE_IRQS_OFF
 
 	movl	%esp, %eax
-	call	do_int80_syscall_32
+	call	do_syscall_32_irqs_on
 .Lsyscall_32_done:
 
 restore_all:
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 92b0b27..c320183 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -303,7 +303,7 @@ ENTRY(entry_INT80_compat)
 	TRACE_IRQS_OFF
 
 	movq	%rsp, %rdi
-	call	do_int80_syscall_32
+	call	do_syscall_32_irqs_off
 .Lsyscall_32_done:
 
 	/* Go back to user mode. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1250328 — Re: [tip:x86/asm] x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on

FromAndy Lutomirski <luto@amacapital.net>
Date2015-10-19 06:50 +0200
SubjectRe: [tip:x86/asm] x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on
Message-ID<qlayu-2bl-3@gated-at.bofh.it>
In reply to#1250104
On Oct 18, 2015 3:16 AM, "tip-bot for Andy Lutomirski" <tipbot@zytor.com> wrote:
>
> Commit-ID:  657c1eea0019e80685a84cbb1919794243a187c9
> Gitweb:     http://git.kernel.org/tip/657c1eea0019e80685a84cbb1919794243a187c9
> Author:     Andy Lutomirski <luto@kernel.org>
> AuthorDate: Fri, 16 Oct 2015 15:42:54 -0700
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Sun, 18 Oct 2015 12:11:16 +0200
>
> x86/entry/32: Fix entry_INT80_32() to expect interrupts to be on

FWIW, I may want to revert this down the road, but it's fine for now.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web