x86/ldt: Prevent ldt inheritance on exec

x86/ldt: Prevent ldt inheritance on exec

[Thomas Gleixner]

From: Thomas Gleixner <tglx@linutronix.de>

The LDT is inheritet independent of fork or exec, but that makes no sense
at all because exec is supposed to start the process clean.

The reason why this happens is that init_new_context_ldt() is called from
init_new_context() which obviously needs to be called for both fork() and
exec().

It would be surprising if anything relies on that behaviour, so it seems to
be safe to remove that misfeature.

Check whether current is in exec() and avoid the LDT clone if so.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/ldt.c                 |    6 +++++-
 tools/testing/selftests/x86/ldt_gdt.c |    9 +++------
 2 files changed, 8 insertions(+), 7 deletions(-)

--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -135,7 +135,11 @@ int init_new_context_ldt(struct task_str
 
 	mutex_init(&mm->context.lock);
 	old_mm = current->mm;
-	if (!old_mm) {
+	/*
+	 * No action if current is a kernel thread or exec() is in
+	 * progress.
+	 */
+	if (!old_mm || current->in_execve) {
 		mm->context.ldt = NULL;
 		return 0;
 	}
--- a/tools/testing/selftests/x86/ldt_gdt.c
+++ b/tools/testing/selftests/x86/ldt_gdt.c
@@ -627,13 +627,10 @@ static void do_multicpu_tests(void)
 static int finish_exec_test(void)
 {
 	/*
-	 * In a sensible world, this would be check_invalid_segment(0, 1);
-	 * For better or for worse, though, the LDT is inherited across exec.
-	 * We can probably change this safely, but for now we test it.
+	 * Older kernel versions did inherit the LDT on exec() which is
+	 * wrong because exec() starts from a clean state.
 	 */
-	check_valid_segment(0, 1,
-			    AR_DPL3 | AR_TYPE_XRCODE | AR_S | AR_P | AR_DB,
-			    42, true);
+	check_invalid_segment(0, 1);
 
 	return nerrs ? 1 : 0;
 }

Re: x86/ldt: Prevent ldt inheritance on exec

[Linus Torvalds]

On Fri, Dec 8, 2017 at 12:02 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The reason why this happens is that init_new_context_ldt() is called from
> init_new_context() which obviously needs to be called for both fork() and
> exec().
>
> It would be surprising if anything relies on that behaviour, so it seems to
> be safe to remove that misfeature.

Looks sane. That said, can't we separate this out into the copy_mm()
phase only?

We have "arch_dup_mmap()" that is called on fork() only, so that could
do the LDT copy from the old mm, and the actual init_new_context would
just zero it out.

Then there wouldn't be any odd "check if this is an execve" because
the copying would be done in the right place.

Hmm?

             Linus

Re: x86/ldt: Prevent ldt inheritance on exec

[Thomas Gleixner]

On Fri, 8 Dec 2017, Linus Torvalds wrote:

> On Fri, Dec 8, 2017 at 12:02 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > The reason why this happens is that init_new_context_ldt() is called from
> > init_new_context() which obviously needs to be called for both fork() and
> > exec().
> >
> > It would be surprising if anything relies on that behaviour, so it seems to
> > be safe to remove that misfeature.
> 
> Looks sane. That said, can't we separate this out into the copy_mm()
> phase only?
> 
> We have "arch_dup_mmap()" that is called on fork() only, so that could
> do the LDT copy from the old mm, and the actual init_new_context would
> just zero it out.
> 
> Then there wouldn't be any odd "check if this is an execve" because
> the copying would be done in the right place.

Yes, that should work. It just needs to change arch_dup_mmap() so it can
return an error code.

Thanks,

	tglx