Re: [PATCH v5 4/4] x86/ldt: Make modify_ldt optional
On Tue, Jul 28, 2015 at 01:42:20PM -0700, Kees Cook wrote: > On Tue, Jul 28, 2015 at 1:03 PM, Willy Tarreau wrote: > > Hi Kees, > > > > On Tue, Jul 28, 2015 at 09:56:12AM -0700, Kees Cook wrote: > >> I look forward to the runtime disabling patch. :) > > > > Did you get my response to your comments regarding the proposed patch ? > > > > I can rebase it and update it if needed, I just want to make sure > > everyone's on the same line regarding this. > > Yeah, I'm fine with what you have. I'd still like a "off until next > reboot", but I'll live. :) That's an improvement we can bring later with special value "-1" which is even less than zero. Thanks, Willy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 4/4] x86/ldt: Make modify_ldt optional
On Tue, Jul 28, 2015 at 1:03 PM, Willy Tarreau wrote: > Hi Kees, > > On Tue, Jul 28, 2015 at 09:56:12AM -0700, Kees Cook wrote: >> I look forward to the runtime disabling patch. :) > > Did you get my response to your comments regarding the proposed patch ? > > I can rebase it and update it if needed, I just want to make sure > everyone's on the same line regarding this. Yeah, I'm fine with what you have. I'd still like a "off until next reboot", but I'll live. :) -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 4/4] x86/ldt: Make modify_ldt optional
Hi Kees, On Tue, Jul 28, 2015 at 09:56:12AM -0700, Kees Cook wrote: > I look forward to the runtime disabling patch. :) Did you get my response to your comments regarding the proposed patch ? I can rebase it and update it if needed, I just want to make sure everyone's on the same line regarding this. Cheers, Willy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 4/4] x86/ldt: Make modify_ldt optional
On Mon, Jul 27, 2015 at 10:29 PM, Andy Lutomirski wrote:
> The modify_ldt syscall exposes a large attack surface and is
> unnecessary for modern userspace. Make it optional.
>
> Signed-off-by: Andy Lutomirski
Reviewed-by: Kees Cook
> ---
> arch/x86/Kconfig | 17 +
> arch/x86/include/asm/mmu.h | 2 ++
> arch/x86/include/asm/mmu_context.h | 31 +++
> arch/x86/kernel/Makefile | 3 ++-
> arch/x86/kernel/cpu/perf_event.c | 4
> arch/x86/kernel/process_64.c | 2 ++
> arch/x86/kernel/step.c | 2 ++
> kernel/sys_ni.c| 1 +
> 8 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b3a1a5d77d92..ede52be845db 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1015,6 +1015,7 @@ config VM86
> config X86_16BIT
> bool "Enable support for 16-bit segments" if EXPERT
> default y
> + depends on MODIFY_LDT_SYSCALL
> ---help---
> This option is required by programs like Wine to run 16-bit
> protected mode legacy code on x86 processors. Disabling
> @@ -2053,6 +2054,22 @@ config CMDLINE_OVERRIDE
> This is used to work around broken boot loaders. This should
> be set to 'N' under normal conditions.
>
> +config MODIFY_LDT_SYSCALL
> + bool "Enable the LDT (local descriptor table)" if EXPERT
> + default y
> + ---help---
> + Linux can allow user programs to install a per-process x86
nit: looks like a spaces vs tabs issue in the line above?
> +Local Descriptor Table (LDT) using the modify_ldt(2) system
> +call. This is required to run 16-bit or segmented code such as
> +DOSEMU or some Wine programs. It is also used by some very old
> +threading libraries.
> +
> +Enabling this feature adds a small amount of overhead to
> +context switches and increases the low-level kernel attack
> +surface. Disabling it removes the modify_ldt(2) system call.
> +
> +Saying 'N' here may make sense for embedded or server kernels.
> +
> source "kernel/livepatch/Kconfig"
>
> endmenu
> diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
> index 364d27481a52..55234d5e7160 100644
> --- a/arch/x86/include/asm/mmu.h
> +++ b/arch/x86/include/asm/mmu.h
> @@ -9,7 +9,9 @@
> * we put the segment information here.
> */
> typedef struct {
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> struct ldt_struct *ldt;
> +#endif
>
> #ifdef CONFIG_X86_64
> /* True if mm supports a task running in 32 bit compatibility mode. */
> diff --git a/arch/x86/include/asm/mmu_context.h
> b/arch/x86/include/asm/mmu_context.h
> index 3fcff70c398e..08094eded318 100644
> --- a/arch/x86/include/asm/mmu_context.h
> +++ b/arch/x86/include/asm/mmu_context.h
> @@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)
> static inline void load_mm_cr4(struct mm_struct *mm) {}
> #endif
>
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> /*
> * ldt_structs can be allocated, used, and freed, but they are never
> * modified while live.
> @@ -48,10 +49,24 @@ struct ldt_struct {
> int size;
> };
>
> +/*
> + * Used for LDT copy/destruction.
> + */
> +int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
> +void destroy_context(struct mm_struct *mm);
> +#else /* CONFIG_MODIFY_LDT_SYSCALL */
> +static inline int init_new_context(struct task_struct *tsk,
> + struct mm_struct *mm)
> +{
> + return 0;
> +}
> +static inline void destroy_context(struct mm_struct *mm) {}
> +#endif
> +
> static inline void load_mm_ldt(struct mm_struct *mm)
> {
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> struct ldt_struct *ldt;
> - DEBUG_LOCKS_WARN_ON(!irqs_disabled());
>
> /* lockless_dereference synchronizes with smp_store_release */
> ldt = lockless_dereference(mm->context.ldt);
> @@ -74,14 +89,12 @@ static inline void load_mm_ldt(struct mm_struct *mm)
> set_ldt(ldt->entries, ldt->size);
> else
> clear_LDT();
> -}
> -
> -/*
> - * Used for LDT copy/destruction.
> - */
> -int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
> -void destroy_context(struct mm_struct *mm);
> +#else
> + clear_LDT();
> +#endif
>
> + DEBUG_LOCKS_WARN_ON(!irqs_disabled());
> +}
>
> static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct
> *tsk)
> {
> @@ -113,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev,
> struct mm_struct *next,
> /* Load per-mm CR4 state */
> load_mm_cr4(next);
>
> +#ifdef CONFIG_MODIFY_LDT_SYSCALL
> /*
> * Load the LDT, if the LDT is different.
> *
> @@ -127,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev,
> struct mm_struct *next,
>
[PATCH v5 4/4] x86/ldt: Make modify_ldt optional
The modify_ldt syscall exposes a large attack surface and is
unnecessary for modern userspace. Make it optional.
Signed-off-by: Andy Lutomirski
---
arch/x86/Kconfig | 17 +
arch/x86/include/asm/mmu.h | 2 ++
arch/x86/include/asm/mmu_context.h | 31 +++
arch/x86/kernel/Makefile | 3 ++-
arch/x86/kernel/cpu/perf_event.c | 4
arch/x86/kernel/process_64.c | 2 ++
arch/x86/kernel/step.c | 2 ++
kernel/sys_ni.c| 1 +
8 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b3a1a5d77d92..ede52be845db 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1015,6 +1015,7 @@ config VM86
config X86_16BIT
bool "Enable support for 16-bit segments" if EXPERT
default y
+ depends on MODIFY_LDT_SYSCALL
---help---
This option is required by programs like Wine to run 16-bit
protected mode legacy code on x86 processors. Disabling
@@ -2053,6 +2054,22 @@ config CMDLINE_OVERRIDE
This is used to work around broken boot loaders. This should
be set to 'N' under normal conditions.
+config MODIFY_LDT_SYSCALL
+ bool "Enable the LDT (local descriptor table)" if EXPERT
+ default y
+ ---help---
+ Linux can allow user programs to install a per-process x86
+Local Descriptor Table (LDT) using the modify_ldt(2) system
+call. This is required to run 16-bit or segmented code such as
+DOSEMU or some Wine programs. It is also used by some very old
+threading libraries.
+
+Enabling this feature adds a small amount of overhead to
+context switches and increases the low-level kernel attack
+surface. Disabling it removes the modify_ldt(2) system call.
+
+Saying 'N' here may make sense for embedded or server kernels.
+
source "kernel/livepatch/Kconfig"
endmenu
diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
index 364d27481a52..55234d5e7160 100644
--- a/arch/x86/include/asm/mmu.h
+++ b/arch/x86/include/asm/mmu.h
@@ -9,7 +9,9 @@
* we put the segment information here.
*/
typedef struct {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
struct ldt_struct *ldt;
+#endif
#ifdef CONFIG_X86_64
/* True if mm supports a task running in 32 bit compatibility mode. */
diff --git a/arch/x86/include/asm/mmu_context.h
b/arch/x86/include/asm/mmu_context.h
index 3fcff70c398e..08094eded318 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)
static inline void load_mm_cr4(struct mm_struct *mm) {}
#endif
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
/*
* ldt_structs can be allocated, used, and freed, but they are never
* modified while live.
@@ -48,10 +49,24 @@ struct ldt_struct {
int size;
};
+/*
+ * Used for LDT copy/destruction.
+ */
+int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
+void destroy_context(struct mm_struct *mm);
+#else /* CONFIG_MODIFY_LDT_SYSCALL */
+static inline int init_new_context(struct task_struct *tsk,
+ struct mm_struct *mm)
+{
+ return 0;
+}
+static inline void destroy_context(struct mm_struct *mm) {}
+#endif
+
static inline void load_mm_ldt(struct mm_struct *mm)
{
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
struct ldt_struct *ldt;
- DEBUG_LOCKS_WARN_ON(!irqs_disabled());
/* lockless_dereference synchronizes with smp_store_release */
ldt = lockless_dereference(mm->context.ldt);
@@ -74,14 +89,12 @@ static inline void load_mm_ldt(struct mm_struct *mm)
set_ldt(ldt->entries, ldt->size);
else
clear_LDT();
-}
-
-/*
- * Used for LDT copy/destruction.
- */
-int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
-void destroy_context(struct mm_struct *mm);
+#else
+ clear_LDT();
+#endif
+ DEBUG_LOCKS_WARN_ON(!irqs_disabled());
+}
static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct
*tsk)
{
@@ -113,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct
mm_struct *next,
/* Load per-mm CR4 state */
load_mm_cr4(next);
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
/*
* Load the LDT, if the LDT is different.
*
@@ -127,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct
mm_struct *next,
*/
if (unlikely(prev->context.ldt != next->context.ldt))
load_mm_ldt(next);
+#endif
}
#ifdef CONFIG_SMP
else {
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0f15af41bd80..2b507befcd3f 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -24,7 +24,8 @@ CFLAGS_irq.o := -I$(src)/../in
