linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch V3 00/20] x86/iopl: Prevent user space from using CLI/STI with iopl(3)
@ 2019-11-13 20:42 Thomas Gleixner
  2019-11-13 20:42 ` [patch V3 01/20] x86/ptrace: Prevent truncation of bitmap size Thomas Gleixner
                   ` (20 more replies)
  0 siblings, 21 replies; 53+ messages in thread
From: Thomas Gleixner @ 2019-11-13 20:42 UTC (permalink / raw)
  To: LKML
  Cc: x86, Andy Lutomirski, Linus Torvalds, Stephen Hemminger,
	Willy Tarreau, Juergen Gross, Sean Christopherson,
	H. Peter Anvin

This is the third version of the attempt to confine the unwanted side
effects of iopl(). The first version of this series can be found here:

   https://lore.kernel.org/r/20191106193459.581614484@linutronix.de

Second version is here:

   https://lore.kernel.org/r/20191111220314.519933535@linutronix.de

The V1 cover letter also contains a longer variant of the
background. Summary:

iopl(level = 3) enables aside of access to all 65536 I/O ports also the
usage of CLI/STI in user space.

Disabling interrupts in user space can lead to system lockups and breaks
assumptions in the kernel that userspace always runs with interrupts
enabled.

iopl() is often preferred over ioperm() as it avoids the overhead of
copying the tasks I/O bitmap to the TSS bitmap on context switch. This
overhead can be avoided by providing a all zeroes bitmap in the TSS and
switching the TSS bitmap offset to this permit all IO bitmap. It's
marginally slower than iopl() which is a one time setup, but prevents the
usage of CLI/STI in user space.

The changes vs. V3:

    - Split out the restructuring of the first/subsequent ioperm()
      invocation into a seperate patch to address the inconsisteny which
      Andy detected in the patch which introduces the concept of
      invalidating the I/O bitmap base to speed up context switching.
      This change is moved in front so the subsequent changes are
      functionally correct.

    - Moved the non HW TSS data related to I/O bitmap(s) into a seperate
      data structure. Modified version of Ingos proposed patch.

    - Made struct memeber names more consistent (Ingo)

    - Dropped the bitmap union. It is not longer necessary because V2
      already dropped the finer grained copying algorithm. The sequence
      count approach should avoid most of the copying overhead when the
      number of ioperm() using processes is very low which is the normal
      case.

    - Dropped the pointer storage of the bitmap in the TSS data as it is
      not required (Peter, Andy)

    - Fixed the missing refcount setting in the bitmap duplication code
      path. (Peter, Andy)

    - Updated changelog and comment to explain the bitmap invalidation
      logic. (Andy)

    - Removed TIF_IO_BITMAP from the TIF flags which are evaluated on the
      next task for entering the slow path.

    - Folded the NULL pointer check fix

    - Simplified the config option in the legacy removal patch (Andy)

    - Extended the scope of the config option to disable ioperm() along
      with iopl() which also mokes all related storage and functions
      compile time conditional. (Andy)

The series is also available from git:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/iopl

Thanks,

	tglx
---
 arch/x86/Kconfig                        |   18 ++
 arch/x86/entry/common.c                 |    4 
 arch/x86/include/asm/io_bitmap.h        |   29 ++++
 arch/x86/include/asm/paravirt.h         |    4 
 arch/x86/include/asm/paravirt_types.h   |    2 
 arch/x86/include/asm/pgtable_32_types.h |    2 
 arch/x86/include/asm/processor.h        |  113 ++++++++++-------
 arch/x86/include/asm/ptrace.h           |    6 
 arch/x86/include/asm/switch_to.h        |   10 +
 arch/x86/include/asm/thread_info.h      |   14 +-
 arch/x86/include/asm/xen/hypervisor.h   |    2 
 arch/x86/kernel/cpu/common.c            |  188 ++++++++++++----------------
 arch/x86/kernel/doublefault.c           |    2 
 arch/x86/kernel/ioport.c                |  209 +++++++++++++++++++++-----------
 arch/x86/kernel/paravirt.c              |    2 
 arch/x86/kernel/process.c               |  200 ++++++++++++++++++++++++------
 arch/x86/kernel/process_32.c            |   77 -----------
 arch/x86/kernel/process_64.c            |   86 -------------
 arch/x86/kernel/ptrace.c                |   12 +
 arch/x86/kvm/vmx/vmx.c                  |    8 -
 arch/x86/mm/cpu_entry_area.c            |    8 +
 arch/x86/xen/enlighten_pv.c             |   10 -
 tools/testing/selftests/x86/ioperm.c    |   16 ++
 tools/testing/selftests/x86/iopl.c      |  129 ++++++++++++++++++-
 24 files changed, 674 insertions(+), 477 deletions(-)


^ permalink raw reply	[flat|nested] 53+ messages in thread
* [tip: x86/iopl] x86/ioperm: Extend IOPL config to control ioperm() as well
@ 2019-11-16 11:51 tip-bot2 for Thomas Gleixner
  0 siblings, 0 replies; 53+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2019-11-16 11:51 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	linux-kernel

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

Commit-ID:     111e7b15cf10f6e973ccf537c70c66a5de539060
Gitweb:        https://git.kernel.org/tip/111e7b15cf10f6e973ccf537c70c66a5de539060
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 12 Nov 2019 21:40:33 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Sat, 16 Nov 2019 11:24:06 +01:00

x86/ioperm: Extend IOPL config to control ioperm() as well

If iopl() is disabled, then providing ioperm() does not make much sense.

Rename the config option and disable/enable both syscalls with it. Guard
the code with #ifdefs where appropriate.

Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/Kconfig                   |  7 +++++--
 arch/x86/include/asm/io_bitmap.h   |  6 ++++++
 arch/x86/include/asm/processor.h   |  9 ++++++++-
 arch/x86/include/asm/thread_info.h |  7 ++++++-
 arch/x86/kernel/cpu/common.c       | 26 +++++++++++++++++---------
 arch/x86/kernel/ioport.c           | 26 +++++++++++++++++++-------
 arch/x86/kernel/process.c          |  4 ++++
 7 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1f926e3..b162ce1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1254,10 +1254,13 @@ config X86_VSYSCALL_EMULATION
 	 Disabling this option saves about 7K of kernel size and
 	 possibly 4K of additional runtime pagetable memory.
 
-config X86_IOPL_EMULATION
-	bool "IOPL Emulation"
+config X86_IOPL_IOPERM
+	bool "IOPERM and IOPL Emulation"
 	default y
 	---help---
+	  This enables the ioperm() and iopl() syscalls which are necessary
+	  for legacy applications.
+
 	  Legacy IOPL support is an overbroad mechanism which allows user
 	  space aside of accessing all 65536 I/O ports also to disable
 	  interrupts. To gain this access the caller needs CAP_SYS_RAWIO
diff --git a/arch/x86/include/asm/io_bitmap.h b/arch/x86/include/asm/io_bitmap.h
index b664baa..02c6ef8 100644
--- a/arch/x86/include/asm/io_bitmap.h
+++ b/arch/x86/include/asm/io_bitmap.h
@@ -15,9 +15,15 @@ struct io_bitmap {
 
 struct task_struct;
 
+#ifdef CONFIG_X86_IOPL_IOPERM
 void io_bitmap_share(struct task_struct *tsk);
 void io_bitmap_exit(void);
 
 void tss_update_io_bitmap(void);
+#else
+static inline void io_bitmap_share(struct task_struct *tsk) { }
+static inline void io_bitmap_exit(void) { }
+static inline void tss_update_io_bitmap(void) { }
+#endif
 
 #endif
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 1387d31..45f416a 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -340,13 +340,18 @@ struct x86_hw_tss {
 	(offsetof(struct tss_struct, io_bitmap.mapall) -	\
 	 offsetof(struct tss_struct, x86_tss))
 
+#ifdef CONFIG_X86_IOPL_IOPERM
 /*
  * sizeof(unsigned long) coming from an extra "long" at the end of the
  * iobitmap. The limit is inclusive, i.e. the last valid byte.
  */
-#define __KERNEL_TSS_LIMIT	\
+# define __KERNEL_TSS_LIMIT	\
 	(IO_BITMAP_OFFSET_VALID_ALL + IO_BITMAP_BYTES + \
 	 sizeof(unsigned long) - 1)
+#else
+# define __KERNEL_TSS_LIMIT	\
+	(offsetof(struct tss_struct, x86_tss) + sizeof(struct x86_hw_tss) - 1)
+#endif
 
 /* Base offset outside of TSS_LIMIT so unpriviledged IO causes #GP */
 #define IO_BITMAP_OFFSET_INVALID	(__KERNEL_TSS_LIMIT + 1)
@@ -398,7 +403,9 @@ struct tss_struct {
 	 */
 	struct x86_hw_tss	x86_tss;
 
+#ifdef CONFIG_X86_IOPL_IOPERM
 	struct x86_io_bitmap	io_bitmap;
+#endif
 } __aligned(PAGE_SIZE);
 
 DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 0accf44..d779366 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -156,8 +156,13 @@ struct thread_info {
 # define _TIF_WORK_CTXSW	(_TIF_WORK_CTXSW_BASE)
 #endif
 
-#define _TIF_WORK_CTXSW_PREV	(_TIF_WORK_CTXSW| _TIF_USER_RETURN_NOTIFY | \
+#ifdef CONFIG_X86_IOPL_IOPERM
+# define _TIF_WORK_CTXSW_PREV	(_TIF_WORK_CTXSW| _TIF_USER_RETURN_NOTIFY | \
 				 _TIF_IO_BITMAP)
+#else
+# define _TIF_WORK_CTXSW_PREV	(_TIF_WORK_CTXSW| _TIF_USER_RETURN_NOTIFY)
+#endif
+
 #define _TIF_WORK_CTXSW_NEXT	(_TIF_WORK_CTXSW)
 
 #define STACK_WARN		(THREAD_SIZE/8)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7bf402b..6f6ca6b 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1804,6 +1804,22 @@ static inline void gdt_setup_doublefault_tss(int cpu)
 }
 #endif /* !CONFIG_X86_64 */
 
+static inline void tss_setup_io_bitmap(struct tss_struct *tss)
+{
+	tss->x86_tss.io_bitmap_base = IO_BITMAP_OFFSET_INVALID;
+
+#ifdef CONFIG_X86_IOPL_IOPERM
+	tss->io_bitmap.prev_max = 0;
+	tss->io_bitmap.prev_sequence = 0;
+	memset(tss->io_bitmap.bitmap, 0xff, sizeof(tss->io_bitmap.bitmap));
+	/*
+	 * Invalidate the extra array entry past the end of the all
+	 * permission bitmap as required by the hardware.
+	 */
+	tss->io_bitmap.mapall[IO_BITMAP_LONGS] = ~0UL;
+#endif
+}
+
 /*
  * cpu_init() initializes state that is per-CPU. Some data is already
  * initialized (naturally) in the bootstrap process, such as the GDT
@@ -1860,15 +1876,7 @@ void cpu_init(void)
 
 	/* Initialize the TSS. */
 	tss_setup_ist(tss);
-	tss->x86_tss.io_bitmap_base = IO_BITMAP_OFFSET_INVALID;
-	tss->io_bitmap.prev_max = 0;
-	tss->io_bitmap.prev_sequence = 0;
-	memset(tss->io_bitmap.bitmap, 0xff, sizeof(tss->io_bitmap.bitmap));
-	/*
-	 * Invalidate the extra array entry past the end of the all
-	 * permission bitmap as required by the hardware.
-	 */
-	tss->io_bitmap.mapall[IO_BITMAP_LONGS] = ~0UL;
+	tss_setup_io_bitmap(tss);
 	set_tss_desc(cpu, &get_cpu_entry_area(cpu)->tss.x86_tss);
 
 	load_TR_desc();
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index d5dcde9..8abeee0 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -14,6 +14,8 @@
 #include <asm/io_bitmap.h>
 #include <asm/desc.h>
 
+#ifdef CONFIG_X86_IOPL_IOPERM
+
 static atomic64_t io_bitmap_sequence;
 
 void io_bitmap_share(struct task_struct *tsk)
@@ -172,13 +174,6 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
 	struct thread_struct *t = &current->thread;
 	unsigned int old;
 
-	/*
-	 * Careful: the IOPL bits in regs->flags are undefined under Xen PV
-	 * and changing them has no effect.
-	 */
-	if (IS_ENABLED(CONFIG_X86_IOPL_NONE))
-		return -ENOSYS;
-
 	if (level > 3)
 		return -EINVAL;
 
@@ -200,3 +195,20 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
 
 	return 0;
 }
+
+#else /* CONFIG_X86_IOPL_IOPERM */
+
+long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
+{
+	return -ENOSYS;
+}
+SYSCALL_DEFINE3(ioperm, unsigned long, from, unsigned long, num, int, turn_on)
+{
+	return -ENOSYS;
+}
+
+SYSCALL_DEFINE1(iopl, unsigned int, level)
+{
+	return -ENOSYS;
+}
+#endif
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 8a844a5..7964d7d 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -322,6 +322,7 @@ void arch_setup_new_exec(void)
 	}
 }
 
+#ifdef CONFIG_X86_IOPL_IOPERM
 static inline void tss_invalidate_io_bitmap(struct tss_struct *tss)
 {
 	/*
@@ -409,6 +410,9 @@ void tss_update_io_bitmap(void)
 		tss_invalidate_io_bitmap(tss);
 	}
 }
+#else /* CONFIG_X86_IOPL_IOPERM */
+static inline void switch_to_bitmap(unsigned long tifp) { }
+#endif
 
 #ifdef CONFIG_SMP
 

^ permalink raw reply related	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2019-11-16 11:52 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 20:42 [patch V3 00/20] x86/iopl: Prevent user space from using CLI/STI with iopl(3) Thomas Gleixner
2019-11-13 20:42 ` [patch V3 01/20] x86/ptrace: Prevent truncation of bitmap size Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 02/20] x86/process: Unify copy_thread_tls() Thomas Gleixner
2019-11-13 21:10   ` Linus Torvalds
2019-11-13 21:41     ` Thomas Gleixner
2019-11-13 22:10       ` Linus Torvalds
2019-11-13 22:33         ` Thomas Gleixner
2019-11-13 21:44     ` Brian Gerst
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 03/20] x86/cpu: Unify cpu_init() Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 04/20] x86/tss: Fix and move VMX BUILD_BUG_ON() Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 05/20] x86/iopl: Cleanup include maze Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 06/20] x86/ioperm: Simplify first ioperm() invocation logic Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 07/20] x86/ioperm: Avoid bitmap allocation if no permissions are set Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 08/20] x86/io: Speedup schedule out of I/O bitmap user Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 09/20] x86/tss: Move I/O bitmap data into a seperate struct Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 10/20] x86/ioperm: Move iobitmap data into a struct Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 11/20] x86/ioperm: Add bitmap sequence number Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 12/20] x86/ioperm: Move TSS bitmap update to exit to user work Thomas Gleixner
2019-11-13 21:19   ` Linus Torvalds
2019-11-13 21:21     ` Linus Torvalds
2019-11-13 21:44       ` Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 13/20] x86/ioperm: Remove bitmap if all permissions dropped Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 14/20] x86/ioperm: Share I/O bitmap if identical Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 15/20] selftests/x86/ioperm: Extend testing so the shared bitmap is exercised Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 16/20] x86/iopl: Fixup misleading comment Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 17/20] x86/iopl: Restrict iopl() permission scope Thomas Gleixner
2019-11-14 18:13   ` Borislav Petkov
2019-11-14 18:39     ` Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 18/20] x86/iopl: Remove legacy IOPL option Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:42 ` [patch V3 19/20] x86/ioperm: Extend IOPL config to control ioperm() as well Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-13 20:43 ` [patch V3 20/20] selftests/x86/iopl: Extend test to cover IOPL emulation Thomas Gleixner
2019-11-15 21:12   ` [tip: x86/iopl] " tip-bot2 for Thomas Gleixner
2019-11-14  8:43 ` [patch V3 00/20] x86/iopl: Prevent user space from using CLI/STI with iopl(3) Peter Zijlstra
2019-11-16 11:51 [tip: x86/iopl] x86/ioperm: Extend IOPL config to control ioperm() as well tip-bot2 for Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).