x86: modify_ldt improvement, test, and config option

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

Cc [email protected]

Signed-off-by Andy Lutomirski <[email protected]>

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

Cc [email protected]

Signed-off-by Andy Lutomirski <[email protected]>

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Quoted 2 lines · Andy Lutomirski

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Andy, you didn't respond whether you think it wouldn't be better to make it runtime-configurable instead. The goal here is to ensure distros ship with modify_ldt disabled by default. But if it means breaking compatibility with (rare) existing applications, I'm seeing a risk that they'll ship with it enabled instead, which would make the config option useless. The CONFIG_DEFAULT_MMAP_ADDR was a good example of successful deployment of a hardening measure that has been widely adopted despite its (low) risk of breakage in field because it was adjustable in field.

That's why here I think we should do the same, and possibly even emit a warning once to report the first user of modify_ldt if that can help.

What do you think ?

Quoted 2 lines · Andy Lutomirski

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Andy, you didn't respond whether you think it wouldn't be better to make it runtime-configurable instead. The goal here is to ensure distros ship with modify_ldt disabled by default. But if it means breaking compatibility with (rare) existing applications, I'm seeing a risk that they'll ship with it enabled instead, which would make the config option useless. The CONFIG_DEFAULT_MMAP_ADDR was a good example of successful deployment of a hardening measure that has been widely adopted despite its (low) risk of breakage in field because it was adjustable in field.

That's why here I think we should do the same, and possibly even emit a warning once to report the first user of modify_ldt if that can help.

What do you think ?

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Quoted 5 lines

That's why here I think we should do the same, and possibly even emit a warning once to report the first user of modify_ldt if that can help.

What do you think ?

I'm generally in favor.

On the other hand, the current series is already written, might even be compatible with Xen, and patch 1 at least fixes a real bug. Maybe several real bugs.

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Quoted 5 lines

That's why here I think we should do the same, and possibly even emit a warning once to report the first user of modify_ldt if that can help.

What do you think ?

I'm generally in favor.

On the other hand, the current series is already written, might even be compatible with Xen, and patch 1 at least fixes a real bug. Maybe several real bugs.

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

From 93cadf50b56a1f2f1e43137503edc1242f8476a7 Mon Sep 17 00:00:00 2001

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

From 93cadf50b56a1f2f1e43137503edc1242f8476a7 Mon Sep 17 00:00:00 2001

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

Quoted 18 lines · Andy Lutomirski

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Quoted 5 lines

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

I understand it's not complete but I'm a bit bothered with conflating this sysctl with other setting methods, because if the purpose of the sysctl is to disable the syscall, it should do that only. I'd rather document that it's less complete than the Kconfig method and continue to recommend using your option whenever possible (eg: all my kernels will use it just as I've already disabled X86_16BIT everywhere).

Also one benefit of having both options is that it will mechanically make LDT a much less interesting target for future attacks, since it will significantly reduce the likeliness of success, hence the motivation for writing exploits that only work in conferences.

Quoted 18 lines · Andy Lutomirski

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Quoted 5 lines

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

I understand it's not complete but I'm a bit bothered with conflating this sysctl with other setting methods, because if the purpose of the sysctl is to disable the syscall, it should do that only. I'd rather document that it's less complete than the Kconfig method and continue to recommend using your option whenever possible (eg: all my kernels will use it just as I've already disabled X86_16BIT everywhere).

Also one benefit of having both options is that it will mechanically make LDT a much less interesting target for future attacks, since it will significantly reduce the likeliness of success, hence the motivation for writing exploits that only work in conferences.

Quoted 25 lines · Willy Tarreau

Quoted 18 lines · Andy Lutomirski

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Quoted 5 lines

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

Quoted 7 lines

I understand it's not complete but I'm a bit bothered with conflating this sysctl with other setting methods, because if the purpose of the sysctl is to disable the syscall, it should do that only. I'd rather document that it's less complete than the Kconfig method and continue to recommend using your option whenever possible (eg: all my kernels will use it just as I've already disabled X86_16BIT everywhere).

Agreed. We can certainly tighten up the espfix code later.

Quoted 5 lines

Also one benefit of having both options is that it will mechanically make LDT a much less interesting target for future attacks, since it will significantly reduce the likeliness of success, hence the motivation for writing exploits that only work in conferences.

Patch looks fine to me.

Quoted 25 lines · Willy Tarreau

Quoted 18 lines · Andy Lutomirski

There's one thing that I think is incomplete here. Currently, espfix triggers if SS points to the LDT. It's possible for SS to point to the LDT even with modify_ldt disabled, and there's a decent amount of attack surface there.

Can we improve this? Two ideas:

1. In the asm, patch out or otherwise disable espfix if that sysctl has never been set. (Ick.)

2. When modify_ldt is runtime-disabled (or compile-time disabled, perhaps), disallow setting the LDT bit in SS in the handful of places that would allow it (ptrace and sigreturn off the top of my head). We don't need to worry about (regs->ss & 4) being set on kernel entry because we'll never be in user mode with that bit set if the LDT is disabled, but that bit could still be set using kernel APIs. (In fact, my sigreturn test does exactly that.)

Quoted 5 lines

Hmm. With synchronous LDT, we could plausibly check at runtime in the espfix code, too. We used to use LAR to do this, but hpa removed it when he realized that it was racy. It shouldn't be racy any more, because, with my patches applied, the LDT never changes while interrupts are off.

Quoted 7 lines

I understand it's not complete but I'm a bit bothered with conflating this sysctl with other setting methods, because if the purpose of the sysctl is to disable the syscall, it should do that only. I'd rather document that it's less complete than the Kconfig method and continue to recommend using your option whenever possible (eg: all my kernels will use it just as I've already disabled X86_16BIT everywhere).

Agreed. We can certainly tighten up the espfix code later.

Quoted 5 lines

Also one benefit of having both options is that it will mechanically make LDT a much less interesting target for future attacks, since it will significantly reduce the likeliness of success, hence the motivation for writing exploits that only work in conferences.

Patch looks fine to me.

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

CCing Kees as well.

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

CCing Kees as well.

Quoted 24 lines · Willy Tarreau

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

Quoted 5 lines

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

CCing Kees as well.

This patch looks reasonable, but I'd prefer a tri-state (enable, disable, hard-disable).

I do something like this for Yama's ptrace
zero to max_scope range (which "pins" to max_scope if set):

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/yama/yama_lsm.c#n361

Quoted 24 lines · Willy Tarreau

Quoted 6 lines · Willy Tarreau

Quoted 2 lines · Andy Lutomirski

I'm all for it, but I think it should be hard-disablable in config, too, for the -tiny people.

I totally agree.

Quoted 7 lines

Quoted 3 lines

If we add a runtime disable, let's do a
separate patch, and you and Kees can fight over how general it should be.

Initially I was thinking about changing it for a 3-state option but that would prevent X86_16BIT from being hard-disablable, so I'll do something completely separate.

So here comes the proposed patch. It adds a default setting for the sysctl when the option is not hard-disabled (eg: distros not wanting to take risks with legacy apps). It suggests to leave the option off. In case a syscall is blocked, a printk_ratelimited() is called with relevant info (program name, pid, uid) so that the admin can decide whether it's a legitimate call or not. Eg:

Denied a call to modify_ldt() from a.out[1736] (uid: 100). Adjust sysctl if this was not an exploit attempt.

Quoted 5 lines

I personally think it completes well your series, hence the 4/3 numbering. Feel free to adopt it if you cycle another round and if you're OK with it of course.

CCing Kees as well.

This patch looks reasonable, but I'd prefer a tri-state (enable, disable, hard-disable).

I do something like this for Yama's ptrace
zero to max_scope range (which "pins" to max_scope if set):

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/yama/yama_lsm.c#n361

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 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(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -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

arch/x86/Kconfig +12 -0

+       ---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.

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 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(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -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

arch/x86/Kconfig +12 -0

+       ---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.

Quoted 31 lines · Andy Lutomirski

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 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(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -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.
 

arch/x86/Kconfig config CMDLINE_OVERRIDE · line 2058

Quoted 2 lines

arch/x86/Kconfig +2 -0

+config MODIFY_LDT_SYSCALL
+       bool "Enable the LDT (local descriptor table)" if EXPERT

bool "Enable modify_ldt() for per-process Local Descriptor Table"

is how I'd call it.

arch/x86/Kconfig config CMDLINE_OVERRIDE · line 2059

Quoted 2 lines

arch/x86/Kconfig +1 -0

+       default y

Is that "default y" going to turn into a "default n" after a grace period?

Quoted 31 lines · Andy Lutomirski

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 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(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -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.
 

arch/x86/Kconfig config CMDLINE_OVERRIDE · line 2058

Quoted 2 lines

arch/x86/Kconfig +2 -0

+config MODIFY_LDT_SYSCALL
+       bool "Enable the LDT (local descriptor table)" if EXPERT

bool "Enable modify_ldt() for per-process Local Descriptor Table"

is how I'd call it.

arch/x86/Kconfig config CMDLINE_OVERRIDE · line 2059

Quoted 2 lines

arch/x86/Kconfig +1 -0

+       default y

Is that "default y" going to turn into a "default n" after a grace period?

Quoted 2 lines · Borislav Petkov

Is that "default y" going to turn into a "default n" after a grace period?

Let's see how Willy's default-off sysctl plays out. In the long run, maybe we'll have it compiled in but runtime-disabled by default.

There's a big community of users who *really* like using Wine :)

Quoted 2 lines · Borislav Petkov

Is that "default y" going to turn into a "default n" after a grace period?

Let's see how Willy's default-off sysctl plays out. In the long run, maybe we'll have it compiled in but runtime-disabled by default.

There's a big community of users who *really* like using Wine :)

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

- Fixed 32-bit test case on 32-bit native kernel.

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

-boris

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

-boris

Quoted 16 lines · Boris Ostrovsky

Quoted 14 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

Quoted 5 lines

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 16 lines · Boris Ostrovsky

Quoted 14 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

Quoted 5 lines

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

Quoted 37 lines · Boris Ostrovsky

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 7 lines

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

Quoted 37 lines · Boris Ostrovsky

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 7 lines

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

Quoted 107 lines · Andy Lutomirski

Quoted 37 lines · Boris Ostrovsky

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 7 lines

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 107 lines · Andy Lutomirski

Quoted 37 lines · Boris Ostrovsky

Quoted 27 lines · Andy Lutomirski

Quoted 24 lines · Boris Ostrovsky

Quoted 8 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Quoted 7 lines

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

Quoted 6 lines

Quoted 1 line

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

I just sent v4.1 of patch 3. Can you try that?

Quoted 7 lines

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

If writeable aliases did exist then
xen_alloc_ldt() could indeed be insufficient to make the frames usable as an LDT, but xen_free_ldt() wouldn't fail when trying to recreate the writeable mappings.

~Andrew

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

If writeable aliases did exist then
xen_alloc_ldt() could indeed be insufficient to make the frames usable as an LDT, but xen_free_ldt() wouldn't fail when trying to recreate the writeable mappings.

~Andrew

Quoted 103 lines · Andrew Cooper

Quoted 78 lines

Quoted 62 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

Quoted 6 lines

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

Quoted 103 lines · Andrew Cooper

Quoted 78 lines

Quoted 62 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

Quoted 6 lines

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

Quoted 108 lines · Andrew Cooper

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

-boris

Quoted 108 lines · Andrew Cooper

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

-boris

Quoted 109 lines · Boris Ostrovsky

Quoted 108 lines · Andrew Cooper

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

The Linux kernel does a bunch of lazy maps and unmaps and we may be getting an interrupt while the lazy unmap hasn't been called (arch_leave_lazy_mmu_mode).

Having the vm_unmap_aliases and then xc_mc_flush (which is what arch_leave_lazy_mmu_mode ends up doing too and more) would solve it.

Thought I would have thought that vm_unmap_aliases would call arch_leave_lazy_mmu_mode.

Quoted 5 lines

Quoted 5 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

Quoted 1 line

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Which would imply that it has not been written in. Which corresponds to the set_aliased_prot hitting the first BUG_ON.

The xc_mc_flush() also triggers the batched hypercalls - which means we may have some hypercalls that have not yet gone to the hypervisor and then we try do an LDT hypercall (not batched).

You could try building with this debug:

arch/x86/xen/multicalls.c +2 -2

index ea54a08..5d214ce 100644
@@ -28,9 +28,9 @@
  #include "multicalls.h"
  #include "debugfs.h"
 
-#define MC_BATCH	32
+#define MC_BATCH	1
 
-#define MC_DEBUG	0
+#define MC_DEBUG	1
 
  #define MC_ARGS		(MC_BATCH * 16)
 

Quoted 2 lines

-boris

Quoted 109 lines · Boris Ostrovsky

Quoted 108 lines · Andrew Cooper

Quoted 107 lines · Boris Ostrovsky

Quoted 106 lines · Andrew Cooper

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

The Linux kernel does a bunch of lazy maps and unmaps and we may be getting an interrupt while the lazy unmap hasn't been called (arch_leave_lazy_mmu_mode).

Having the vm_unmap_aliases and then xc_mc_flush (which is what arch_leave_lazy_mmu_mode ends up doing too and more) would solve it.

Thought I would have thought that vm_unmap_aliases would call arch_leave_lazy_mmu_mode.

Quoted 5 lines

Quoted 5 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

Quoted 1 line

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Which would imply that it has not been written in. Which corresponds to the set_aliased_prot hitting the first BUG_ON.

The xc_mc_flush() also triggers the batched hypercalls - which means we may have some hypercalls that have not yet gone to the hypervisor and then we try do an LDT hypercall (not batched).

You could try building with this debug:

arch/x86/xen/multicalls.c +2 -2

index ea54a08..5d214ce 100644
@@ -28,9 +28,9 @@
  #include "multicalls.h"
  #include "debugfs.h"
 
-#define MC_BATCH	32
+#define MC_BATCH	1
 
-#define MC_DEBUG	0
+#define MC_DEBUG	1
 
  #define MC_ARGS		(MC_BATCH * 16)
 

Quoted 2 lines

-boris

Quoted 121 lines · Andrew Cooper

Quoted 120 lines · Boris Ostrovsky

Quoted 119 lines · Andrew Cooper

Quoted 86 lines

Quoted 69 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX:
80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP:
c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4:
00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001
cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000
c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e
c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b

4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5

[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On
the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever
referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables
are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine.
Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory),
or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Quoted 121 lines · Andrew Cooper

Quoted 120 lines · Boris Ostrovsky

Quoted 119 lines · Andrew Cooper

Quoted 86 lines

Quoted 69 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX:
80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP:
c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4:
00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001
cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000
c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e
c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b

4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5

[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
      const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
      int i;

      vm_unmap_aliases();
      xen_mc_flush();

      for(i = 0; i < entries; i += entries_per_page)
          set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On
the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever
referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables
are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine.
Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory),
or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Quoted 124 lines · Boris Ostrovsky

Quoted 123 lines · Andrew Cooper

Quoted 122 lines · Boris Ostrovsky

Quoted 121 lines · Andrew Cooper

Quoted 86 lines

Quoted 69 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX:
80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP:
c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4:
00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001
cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000
c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e
c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b

4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5

[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
       const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
       int i;

       vm_unmap_aliases();
       xen_mc_flush();

       for(i = 0; i < entries; i += entries_per_page)
           set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On
the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever
referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables
are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine.
Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory),
or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Are you certain that the set_ldt(NULL, 0) has been flushed to Xen to actually remove the LDT reference? All of this is hidden behind some lazy logic.

Quoted 8 lines

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Did you get the companion "Bad L1 flags" error message with that?

Quoted 124 lines · Boris Ostrovsky

Quoted 123 lines · Andrew Cooper

Quoted 122 lines · Boris Ostrovsky

Quoted 121 lines · Andrew Cooper

Quoted 86 lines

Quoted 69 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX:
80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP:
c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4:
00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001
cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000
c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e
c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b

4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5

[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP
0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
       const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
       int i;

       vm_unmap_aliases();
       xen_mc_flush();

       for(i = 0; i < entries; i += entries_per_page)
           set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On
the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever
referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables
are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine.
Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory),
or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

It would be interesting to know which of the two BUG()s in set_aliased_prot() tripped.

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

The first one (i.e. not the alias)

In which case the page in question is still referenced in an LDT (perhaps on a different vcpu)

The problem is reproducible on a UP guest so it's not that.

Are you certain that the set_ldt(NULL, 0) has been flushed to Xen to actually remove the LDT reference? All of this is hidden behind some lazy logic.

Quoted 8 lines

Quoted 6 lines

or has been reused as a pagetable (I
really hope this is not the case).

A sufficiently-debug Xen might be persuaded into telling you exactly what it didn't like about the attempted transition.

It just can't find l1 entry for the LDT address in __do_update_va_mapping().

Did you get the companion "Bad L1 flags" error message with that?

Quoted 116 lines · Andy Lutomirski

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 116 lines · Andy Lutomirski

Quoted 99 lines · Andy Lutomirski

Quoted 27 lines · Boris Ostrovsky

Quoted 26 lines · Andy Lutomirski

Quoted 13 lines · Boris Ostrovsky

Quoted 12 lines · Andy Lutomirski

Here's v3. It fixes the "dazed and confused" issue, I hope. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Quoted 8 lines

Quoted 2 lines

Changes from v3:
- Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

Quoted 1 line

- Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.

I misplaced the fix in the wrong git commit, so I failed to sent it. Oops.

Quoted 10 lines

Quoted 1 line

I just sent v4.1 of patch 3. Can you try that?

I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT in destroy_context(). Interestingly though when I run the test in the debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

If the typechange fails, the pagefault gets propagated back to the guest.

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

Quoted 84 lines

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 8 lines

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

Makes sense.

Quoted 6 lines

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

Quoted 1 line

If the typechange fails, the pagefault gets propagated back to the guest.

The part I don't understand is that I didn't observe any page faults.

Quoted 3 lines

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

And the mystery here is that I don't see how the typechange to LDT would have succeeded in the first place if we had a writable alias.

In fact, I just fudged xen_set_ldt to probe the entire LDT using lsl, and it didn't page fault, so I'm still quite confused as to what's going on. I've confirmed that my lsl hack really does work -- if I disable xen_alloc_ldt, then xen_set_ldt blows up immediately with my patch.

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/tmp&id=ce664ed73aac804ef2a16ddef45589dbeba55570

Oddly, the OOPS seems much more likely when I kill ldt_gdt_32 with Ctrl-C than when I let it run to completion. The bug is here:

if (HYPERVISOR_update_va_mapping((unsigned long)v, pte, 0))
BUG();

so the only sensible explanation I have is either the guest really didn't drop all refs to the LDT or that Xen hasn't noticed yet. I still don't see how aliases could be involved, because Xen really did accept the LDT.

Are multicalls ordered?

Hrm. Does Xen actually do something sensible with set_ldt(NULL, 0)?

Also, xen_mc_issue seems buggy. Is lazy_mode an enum or a bit field? What happens if two separate lazy modes are entered? I suspect that one of them clobbers the other one.

Quoted 84 lines

Quoted 77 lines

Quoted 61 lines

After forward-porting my virtio patches, I got this thing to run on Xen. After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)

{
    const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
    int i;

    vm_unmap_aliases();
    xen_mc_flush();

    for(i = 0; i < entries; i += entries_per_page)
        set_aliased_prot(ldt + i, PAGE_KERNEL);
}

Quoted 4 lines

then it works. I don't know why this makes a difference. (xen_mc_flush makes a little bit of sense to me. vm_unmap_aliases doesn't.)

That fix makes sense if there's some way that the vmalloc area we're freeing has an extra alias somewhere, which is very much possible. On the other hand, I don't see how this happens without first doing an MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate the actual (hidden) LDT mapping unless the pages backing it are unaliased and well-formed, which make me wonder why this stuff ever worked. Wouldn't LDT access with pre-existing vmalloc aliases result in segfaults?

Quoted 7 lines

The semantics seem to be very odd. xen_free_ldt with an aliased address might fail (and OOPS), but actual access to the LDT with an aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests that there really is something to my theory that the problem involves unexpected aliases.

Quoted 8 lines

Xen does lazily populate the LDT frames. The first time a page is ever referenced via the LDT, Xen will perform a typechange.

Under Xen, guest mappings are reference counted with both a plain reference, and a type count. Types of writeable, segdec and pagetables are mutually exclusive. This prevents the guest from having writeable mappings of interesting datastructures, but readable mappings are fine. Typechanges may only occur when the type reference count is 0.

Makes sense.

Quoted 6 lines

At the point of the typechange, no writeable mappings of the frame may exist (and it must not be referenced by a L2 or greater page directory), or the typechange will fail. Additionally the descriptors are audited at this point, so if Xen objects to any of the descriptors in the same page, the typechange will also fail.

Quoted 1 line

If the typechange fails, the pagefault gets propagated back to the guest.

The part I don't understand is that I didn't observe any page faults.

Quoted 3 lines

The corollary to this is that, for xen_free_ldt() to create writeable mappings again, a typechange back to writeable is needed. This will fail if the LDT frames are still referenced in any vcpus LDT.

And the mystery here is that I don't see how the typechange to LDT would have succeeded in the first place if we had a writable alias.

In fact, I just fudged xen_set_ldt to probe the entire LDT using lsl, and it didn't page fault, so I'm still quite confused as to what's going on. I've confirmed that my lsl hack really does work -- if I disable xen_alloc_ldt, then xen_set_ldt blows up immediately with my patch.

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/tmp&id=ce664ed73aac804ef2a16ddef45589dbeba55570

Oddly, the OOPS seems much more likely when I kill ldt_gdt_32 with Ctrl-C than when I let it run to completion. The bug is here:

if (HYPERVISOR_update_va_mapping((unsigned long)v, pte, 0))
BUG();

so the only sensible explanation I have is either the guest really didn't drop all refs to the LDT or that Xen hasn't noticed yet. I still don't see how aliases could be involved, because Xen really did accept the LDT.

Are multicalls ordered?

Hrm. Does Xen actually do something sensible with set_ldt(NULL, 0)?

Also, xen_mc_issue seems buggy. Is lazy_mode an enum or a bit field? What happens if two separate lazy modes are entered? I suspect that one of them clobbers the other one.

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

~Andrew

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

~Andrew

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 4 lines · Andy Lutomirski

Quoted 1 line · Andrew Cooper

Quoted 8 lines

Quoted 4 lines

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Quoted 4 lines · Andy Lutomirski

Quoted 1 line · Andrew Cooper

Quoted 8 lines

Quoted 4 lines

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Quoted 11 lines · Boris Ostrovsky

Quoted 10 lines · Andy Lutomirski

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 4 lines

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Looking at map_ldt_shadow_page: what keeps shadow_ldt_mapcnt from getting incremented once on each CPU at the same time if both CPUs fault in the same shadow LDT page at the same time?

Similarly, what
keeps both CPUs from calling get_page_type at the same time and therefore losing track of the page type reference count?

I don't see why vmalloc or vm_unmap_aliases would have anything to do with this, though.

Quoted 11 lines · Boris Ostrovsky

Quoted 10 lines · Andy Lutomirski

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 4 lines

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Looking at map_ldt_shadow_page: what keeps shadow_ldt_mapcnt from getting incremented once on each CPU at the same time if both CPUs fault in the same shadow LDT page at the same time?

Similarly, what
keeps both CPUs from calling get_page_type at the same time and therefore losing track of the page type reference count?

I don't see why vmalloc or vm_unmap_aliases would have anything to do with this, though.

Quoted 13 lines · Andy Lutomirski

Quoted 11 lines · Boris Ostrovsky

Quoted 10 lines · Andy Lutomirski

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 7 lines

Quoted 4 lines

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Looking at map_ldt_shadow_page: what keeps shadow_ldt_mapcnt from getting incremented once on each CPU at the same time if both CPUs fault in the same shadow LDT page at the same time?

Nothing, but that is fine. If a page is in use in two vcpus LDTs, it is expected to have a type refcount of 2.

Quoted 3 lines

Similarly, what
keeps both CPUs from calling get_page_type at the same time and therefore losing track of the page type reference count?

a cmpxchg() loop in the depths of __get_page_type().

Quoted 2 lines

I don't see why vmalloc or vm_unmap_aliases would have anything to do with this, though.

Quoted 13 lines · Andy Lutomirski

Quoted 11 lines · Boris Ostrovsky

Quoted 10 lines · Andy Lutomirski

Quoted 4 lines · Andrew Cooper

I suspect that the set_ldt(NULL, 0) call hasn't reached Xen before xen_free_ldt() is attempting to nab back the pages which Xen still has mapped as an LDT.

I just instrumented it with yet more LSL instructions. I'm pretty sure that set_ldt really is clearing at least LDT entry zero. Nonetheless the free_ldt call still oopses.

Quoted 7 lines

Quoted 4 lines

Yes, I added some instrumentation to the hypervisor and we definitely set LDT to NULL before failing.

-boris

Looking at map_ldt_shadow_page: what keeps shadow_ldt_mapcnt from getting incremented once on each CPU at the same time if both CPUs fault in the same shadow LDT page at the same time?

Nothing, but that is fine. If a page is in use in two vcpus LDTs, it is expected to have a type refcount of 2.

Quoted 3 lines

Similarly, what
keeps both CPUs from calling get_page_type at the same time and therefore losing track of the page type reference count?

a cmpxchg() loop in the depths of __get_page_type().

Quoted 2 lines

I don't see why vmalloc or vm_unmap_aliases would have anything to do with this, though.