[PATCH v7 0/3] Fix dosemu vm86() fault

Pawan Gupta posted 3 patches 2 months ago
arch/x86/entry/entry_32.S            | 6 ++++--
arch/x86/include/asm/nospec-branch.h | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
[PATCH v7 0/3] Fix dosemu vm86() fault
Posted by Pawan Gupta 2 months ago
Changes in v7:
- Using %ss for verw fails kselftest ldt_gdt.c in 32-bit mode, use safer %cs instead (Dave).

v6: https://lore.kernel.org/r/20240905-fix-dosemu-vm86-v6-0-7aff8e53cbbf@linux.intel.com
- Use %ss in 64-bit mode as well for all VERW calls. This avoids any having
  a separate macro for 32-bit (Dave).
- Split 32-bit mode fixes into separate patches.

v5: https://lore.kernel.org/r/20240711-fix-dosemu-vm86-v5-1-e87dcd7368aa@linux.intel.com
- Simplify the use of ALTERNATIVE construct (Uros/Jiri/Peter).

v4: https://lore.kernel.org/r/20240710-fix-dosemu-vm86-v4-1-aa6464e1de6f@linux.intel.com
- Further simplify the patch by using %ss for all VERW calls in 32-bit mode (Brian).
- In NMI exit path move VERW after RESTORE_ALL_NMI that touches GPRs (Dave).

v3: https://lore.kernel.org/r/20240701-fix-dosemu-vm86-v3-1-b1969532c75a@linux.intel.com
- Simplify CLEAR_CPU_BUFFERS_SAFE by using %ss instead of %ds (Brian).
- Do verw before popf in SYSEXIT path (Jari).

v2: https://lore.kernel.org/r/20240627-fix-dosemu-vm86-v2-1-d5579f698e77@linux.intel.com
- Safe guard against any other system calls like vm86() that might change %ds (Dave).

v1: https://lore.kernel.org/r/20240426-fix-dosemu-vm86-v1-1-88c826a3f378@linux.intel.com

Hi,

This series fixes a #GP in 32-bit kernels when executing vm86() system call
in dosemu software. In 32-bit mode, their are cases when user can set an
arbitrary %ds that can cause a #GP when executing VERW instruction. The
fix is to use %ss for referencing the VERW operand.

Patch 1-2: Fixes the VERW callsites in 32-bit entry path.
Patch   3: Uses %ss for VERW in 32-bit and 64-bit mode.

The fix is tested with below kselftest on 32-bit kernel:

	./tools/testing/selftests/x86/entry_from_vm86.c

64-bit kernel was boot tested. On a Rocket Lake, measuring the CPU cycles
for VERW with and without the %ss shows no significant difference. This
indicates that the scrubbing behavior of VERW is intact.

Thanks,
Pawan

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
Pawan Gupta (3):
      x86/entry_32: Do not clobber user EFLAGS.ZF
      x86/entry_32: Clear CPU buffers after register restore in NMI return
      x86/bugs: Use code segment selector for VERW operand

 arch/x86/entry/entry_32.S            | 6 ++++--
 arch/x86/include/asm/nospec-branch.h | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)
---
base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
change-id: 20240426-fix-dosemu-vm86-dd111a01737e

Best regards,
-- 
Thanks,
Pawan
Re: [PATCH v7 0/3] Fix dosemu vm86() fault
Posted by Thorsten Leemhuis 1 month, 3 weeks ago
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

Is there hope that patches like these makes it to mainline any time
soon? I fully understand that this it a hard problem, but in the end
what triggered this were at least two regression reports afaics:
https://bugzilla.kernel.org/show_bug.cgi?id=218707
https://lore.kernel.org/lkml/IdYcxU6x6xuUqUg8cliJUnucfwfTO29TrKIlLGCCYbbIr1EQnP0ZAtTxdAM2hp5e5Gny_acIN3OFDS6v0sazocnZZ1UBaINEJ0HoDnbasSI=@protonmail.com/

Sure, the older one was in April, so one week more or less now won't
make much of a difference. But I think it still would be great to get
this fixed rather sooner than later. Or where those issues meanwhile
fixed through other patches without me noticing and I'm making a fool of
myself here?

This yet again makes me wonder if some "[regression fix]" in the subject
or "CC: regressions@lists.linux.dev" in the patches would help to make
the regression aspect obvious to everyone involved. But it would create
yet another small bit of overhead... :-/

Pawan Gupta, btw: many thx for working on this and sticking to it!

Ciao, Thorsten

On 26.09.24 00:25, Pawan Gupta wrote:
> Changes in v7:
> - Using %ss for verw fails kselftest ldt_gdt.c in 32-bit mode, use safer %cs instead (Dave).
> 
> v6: https://lore.kernel.org/r/20240905-fix-dosemu-vm86-v6-0-7aff8e53cbbf@linux.intel.com
> - Use %ss in 64-bit mode as well for all VERW calls. This avoids any having
>   a separate macro for 32-bit (Dave).
> - Split 32-bit mode fixes into separate patches.
> 
> v5: https://lore.kernel.org/r/20240711-fix-dosemu-vm86-v5-1-e87dcd7368aa@linux.intel.com
> - Simplify the use of ALTERNATIVE construct (Uros/Jiri/Peter).
> 
> v4: https://lore.kernel.org/r/20240710-fix-dosemu-vm86-v4-1-aa6464e1de6f@linux.intel.com
> - Further simplify the patch by using %ss for all VERW calls in 32-bit mode (Brian).
> - In NMI exit path move VERW after RESTORE_ALL_NMI that touches GPRs (Dave).
> 
> v3: https://lore.kernel.org/r/20240701-fix-dosemu-vm86-v3-1-b1969532c75a@linux.intel.com
> - Simplify CLEAR_CPU_BUFFERS_SAFE by using %ss instead of %ds (Brian).
> - Do verw before popf in SYSEXIT path (Jari).
> 
> v2: https://lore.kernel.org/r/20240627-fix-dosemu-vm86-v2-1-d5579f698e77@linux.intel.com
> - Safe guard against any other system calls like vm86() that might change %ds (Dave).
> 
> v1: https://lore.kernel.org/r/20240426-fix-dosemu-vm86-v1-1-88c826a3f378@linux.intel.com
> 
> Hi,
> 
> This series fixes a #GP in 32-bit kernels when executing vm86() system call
> in dosemu software. In 32-bit mode, their are cases when user can set an
> arbitrary %ds that can cause a #GP when executing VERW instruction. The
> fix is to use %ss for referencing the VERW operand.
> 
> Patch 1-2: Fixes the VERW callsites in 32-bit entry path.
> Patch   3: Uses %ss for VERW in 32-bit and 64-bit mode.
> 
> The fix is tested with below kselftest on 32-bit kernel:
> 
> 	./tools/testing/selftests/x86/entry_from_vm86.c
> 
> 64-bit kernel was boot tested. On a Rocket Lake, measuring the CPU cycles
> for VERW with and without the %ss shows no significant difference. This
> indicates that the scrubbing behavior of VERW is intact.
> 
> Thanks,
> Pawan
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
> Pawan Gupta (3):
>       x86/entry_32: Do not clobber user EFLAGS.ZF
>       x86/entry_32: Clear CPU buffers after register restore in NMI return
>       x86/bugs: Use code segment selector for VERW operand
> 
>  arch/x86/entry/entry_32.S            | 6 ++++--
>  arch/x86/include/asm/nospec-branch.h | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> ---
> base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
> change-id: 20240426-fix-dosemu-vm86-dd111a01737e
> 
> Best regards,

#regzbot poke
Re: [PATCH v7 0/3] Fix dosemu vm86() fault
Posted by Dave Hansen 1 month, 2 weeks ago
On 10/8/24 06:52, Thorsten Leemhuis wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
> 
> Is there hope that patches like these makes it to mainline any time
> soon?

Unless it breaks something again:

> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=785bf1ab58aa1f89a5dfcb17b682b7089d69c34f

;)

> This yet again makes me wonder if some "[regression fix]" in the subject
> or "CC: regressions@lists.linux.dev" in the patches would help to make
> the regression aspect obvious to everyone involved. But it would create
> yet another small bit of overhead

In this case, not really.  This was a typical email screwup where I
didn't pick up that there was an updated patch that got appended to a
reply among the normal email noise.

We've been poking at this pretty regularly since getting back from Plumbers.
Re: [PATCH v7 0/3] Fix dosemu vm86() fault
Posted by Linux regression tracking (Thorsten Leemhuis) 1 month, 2 weeks ago
On 09.10.24 00:48, Dave Hansen wrote:
> On 10/8/24 06:52, Thorsten Leemhuis wrote:
>> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
>> for once, to make this easily accessible to everyone.
>>
>> Is there hope that patches like these makes it to mainline any time
>> soon?
> 
> Unless it breaks something again:
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=785bf1ab58aa1f89a5dfcb17b682b7089d69c34f
> 
> ;)

:-D

Great, thx!

>> This yet again makes me wonder if some "[regression fix]" in the subject
>> or "CC: regressions@lists.linux.dev" in the patches would help to make
>> the regression aspect obvious to everyone involved. But it would create
>> yet another small bit of overhead
> 
> In this case, not really.  This was a typical email screwup where I
> didn't pick up that there was an updated patch that got appended to a
> reply among the normal email noise.
> 
> We've been poking at this pretty regularly since getting back from Plumbers.

Many thx!

Ciao, Thorsten