[PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically

Binbin Wu posted 2 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically
Posted by Binbin Wu 1 year, 7 months ago
Check whether a KVM hypercall needs to exit to userspace or not based on
hypercall_exit_enabled field of struct kvm_arch.

Userspace can request a hypercall to exit to userspace for handling by
enable KVM_CAP_EXIT_HYPERCALL and the enabled hypercall will be set in
hypercall_exit_enabled.  Make the check code generic based on it.

Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
---
 arch/x86/kvm/x86.c | 4 ++--
 arch/x86/kvm/x86.h | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 994743266480..f84c1f263e9b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10223,8 +10223,8 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 	cpl = kvm_x86_call(get_cpl)(vcpu);
 
 	ret = __kvm_emulate_hypercall(vcpu, nr, a0, a1, a2, a3, op_64_bit, cpl);
-	if (nr == KVM_HC_MAP_GPA_RANGE && !ret)
-		/* MAP_GPA tosses the request to the user space. */
+	if (is_kvm_hc_exit_enabled(vcpu->kvm, nr) && !ret)
+		/* The hypercall is requested to exit to userspace. */
 		return 0;
 
 	if (!op_64_bit)
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 50596f6f8320..02809a915d72 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -547,4 +547,8 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size,
 			 unsigned int port, void *data,  unsigned int count,
 			 int in);
 
+static inline bool is_kvm_hc_exit_enabled(struct kvm *kvm, unsigned long hc_nr)
+{
+	return kvm->arch.hypercall_exit_enabled & (1 << hc_nr);
+}
 #endif
-- 
2.43.2
Re: [PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically
Posted by kernel test robot 1 year, 6 months ago

Hello,

kernel test robot noticed "UBSAN:shift-out-of-bounds_in_arch/x86/kvm#h" on:

commit: 1635eb4564804d324e91d78e8e5ed206e006e3a6 ("[PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically")
url: https://github.com/intel-lab-lkp/linux/commits/Binbin-Wu/KVM-x86-Check-hypercall-s-exit-to-userspace-generically/20240708-172555
patch link: https://lore.kernel.org/all/20240708092150.1799371-2-binbin.wu@linux.intel.com/
patch subject: [PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically

in testcase: kvm-unit-tests-qemu
version: 
with following parameters:




compiler: gcc-13
test machine: 128 threads 2 sockets Intel(R) Xeon(R) Platinum 8358 CPU @ 2.60GHz (Ice Lake) with 128G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202407242159.893be500-oliver.sang@intel.com


[  414.980354][T21255] ------------[ cut here ]------------
[  414.989024][T21255] UBSAN: shift-out-of-bounds in arch/x86/kvm/x86.h:552:47
[  415.001167][T21255] shift exponent 4294967295 is too large for 32-bit type 'int'
[  415.011803][T21255] CPU: 107 PID: 21255 Comm: qemu-system-x86 Not tainted 6.10.0-rc2-00186-g1635eb456480 #1
[  415.024716][T21255] Call Trace:
[  415.030982][T21255]  <TASK>
[415.036836][T21255] dump_stack_lvl (lib/dump_stack.c:117) 
[415.044268][T21255] __ubsan_handle_shift_out_of_bounds (lib/ubsan.c:232 lib/ubsan.c:468) 
[415.053610][T21255] kvm_emulate_hypercall.cold (include/trace/events/kvm.h:213 (discriminator 6)) kvm
[415.063097][T21255] ? __pfx_kvm_emulate_hypercall (arch/x86/kvm/x86.c:10206) kvm
[415.073104][T21255] ? __vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:6469) kvm_intel
[415.082284][T21255] vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:6632 (discriminator 1)) kvm_intel
[415.090893][T21255] vcpu_enter_guest+0x130f/0x3350 kvm
[415.100855][T21255] ? vmx_segment_cache_test_set (arch/x86/include/asm/bitops.h:206 (discriminator 1) arch/x86/include/asm/bitops.h:238 (discriminator 1) include/asm-generic/bitops/instrumented-non-atomic.h:142 (discriminator 1) arch/x86/kvm/vmx/../kvm_cache_regs.h:56 (discriminator 1) arch/x86/kvm/vmx/vmx.c:825 (discriminator 1)) kvm_intel
[415.110593][T21255] ? __pfx_vcpu_enter_guest+0x10/0x10 kvm
[415.120837][T21255] ? vmx_read_guest_seg_ar (arch/x86/kvm/vmx/vmx.c:865 (discriminator 1)) kvm_intel
[415.130124][T21255] ? skip_emulated_instruction (arch/x86/kvm/vmx/vmx.c:1775) kvm_intel
[415.139821][T21255] ? __pfx_skip_emulated_instruction (arch/x86/kvm/vmx/vmx.c:1715) kvm_intel
[415.149853][T21255] ? __pfx_kvm_get_linear_rip (arch/x86/kvm/x86.c:13256) kvm
[415.159211][T21255] vcpu_run (arch/x86/kvm/x86.c:11311) kvm
[415.167028][T21255] kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:11537) kvm
[415.176327][T21255] ? __pfx_do_vfs_ioctl (fs/ioctl.c:805) 
[415.184065][T21255] kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4440) kvm
[415.192450][T21255] ? __pfx_kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4394) kvm
[415.201351][T21255] ? down_read_trylock (arch/x86/include/asm/atomic64_64.h:20 include/linux/atomic/atomic-arch-fallback.h:2629 include/linux/atomic/atomic-long.h:79 include/linux/atomic/atomic-instrumented.h:3224 kernel/locking/rwsem.c:176 kernel/locking/rwsem.c:181 kernel/locking/rwsem.c:1288 kernel/locking/rwsem.c:1565) 
[415.209117][T21255] ? __fget_light (fs/file.c:1154) 
[415.216411][T21255] ? fput (arch/x86/include/asm/atomic64_64.h:61 (discriminator 1) include/linux/atomic/atomic-arch-fallback.h:4404 (discriminator 1) include/linux/atomic/atomic-long.h:1571 (discriminator 1) include/linux/atomic/atomic-instrumented.h:4540 (discriminator 1) fs/file_table.c:473 (discriminator 1)) 
[415.222864][T21255] ? __fget_light (fs/file.c:1154) 
[415.230119][T21255] __x64_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:907 fs/ioctl.c:893 fs/ioctl.c:893) 
[415.237407][T21255] do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1) arch/x86/entry/common.c:83 (discriminator 1)) 
[415.244400][T21255] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) 
[  415.252801][T21255] RIP: 0033:0x7f12912f8c5b
[ 415.259801][T21255] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
All code
========
   0:	00 48 89             	add    %cl,-0x77(%rax)
   3:	44 24 18             	rex.R and $0x18,%al
   6:	31 c0                	xor    %eax,%eax
   8:	48 8d 44 24 60       	lea    0x60(%rsp),%rax
   d:	c7 04 24 10 00 00 00 	movl   $0x10,(%rsp)
  14:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
  19:	48 8d 44 24 20       	lea    0x20(%rsp),%rax
  1e:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
  23:	b8 10 00 00 00       	mov    $0x10,%eax
  28:	0f 05                	syscall 
  2a:*	89 c2                	mov    %eax,%edx		<-- trapping instruction
  2c:	3d 00 f0 ff ff       	cmp    $0xfffff000,%eax
  31:	77 1c                	ja     0x4f
  33:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
  38:	64                   	fs
  39:	48                   	rex.W
  3a:	2b                   	.byte 0x2b
  3b:	04 25                	add    $0x25,%al
  3d:	28 00                	sub    %al,(%rax)
	...

Code starting with the faulting instruction
===========================================
   0:	89 c2                	mov    %eax,%edx
   2:	3d 00 f0 ff ff       	cmp    $0xfffff000,%eax
   7:	77 1c                	ja     0x25
   9:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
   e:	64                   	fs
   f:	48                   	rex.W
  10:	2b                   	.byte 0x2b
  11:	04 25                	add    $0x25,%al
  13:	28 00                	sub    %al,(%rax)
	...
[  415.282007][T21255] RSP: 002b:00007f128e7ff5e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  415.293025][T21255] RAX: ffffffffffffffda RBX: 000055cecae83b00 RCX: 00007f12912f8c5b
[  415.303708][T21255] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 000000000000000c
[  415.314228][T21255] RBP: 000000000000ae80 R08: 0000000000000000 R09: 0000000000000000
[  415.324787][T21255] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[  415.335326][T21255] R13: 0000000000000001 R14: 00000000000003f8 R15: 0000000000000000
[  415.345809][T21255]  </TASK>
[  415.351386][T21255] ---[ end trace ]---



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240724/202407242159.893be500-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically
Posted by Binbin Wu 1 year, 6 months ago
On 7/24/2024 9:48 PM, kernel test robot wrote:
>
> Hello,
>
> kernel test robot noticed "UBSAN:shift-out-of-bounds_in_arch/x86/kvm#h" on:

Oops, the return value of __kvm_emulate_hypercall() should be checked first.
Also add a warning if the hc_nr out of the range of u32 can accommodate.

Will send a new version with the following fixup.


diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 15d55a5f5755..b0d2407872ac 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10236,7 +10236,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
         cpl = static_call(kvm_x86_get_cpl)(vcpu);

         ret = __kvm_emulate_hypercall(vcpu, nr, a0, a1, a2, a3, 
op_64_bit, cpl);
-       if (is_kvm_hc_exit_enabled(vcpu->kvm, nr) && !ret)
+       if (!ret && is_kvm_hc_exit_enabled(vcpu->kvm, nr))
                 /* The hypercall is requested to exit to userspace. */
                 return 0;

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 3bb3c6aaad0e..bd7fe5428741 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -544,6 +544,9 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, 
unsigned int size,

  static inline bool is_kvm_hc_exit_enabled(struct kvm *kvm, unsigned 
long hc_nr)
  {
+       if(WARN_ON_ONCE(hc_nr >= 
sizeof(kvm->arch.hypercall_exit_enabled) * 8))
+               return false;
+
         return kvm->arch.hypercall_exit_enabled & (1 << hc_nr);
  }
  #endif


>
> commit: 1635eb4564804d324e91d78e8e5ed206e006e3a6 ("[PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically")
> url: https://github.com/intel-lab-lkp/linux/commits/Binbin-Wu/KVM-x86-Check-hypercall-s-exit-to-userspace-generically/20240708-172555
> patch link: https://lore.kernel.org/all/20240708092150.1799371-2-binbin.wu@linux.intel.com/
> patch subject: [PATCH 1/2] KVM: x86: Check hypercall's exit to userspace generically
>
> in testcase: kvm-unit-tests-qemu
> version:
> with following parameters:
>
>
>
>
> compiler: gcc-13
> test machine: 128 threads 2 sockets Intel(R) Xeon(R) Platinum 8358 CPU @ 2.60GHz (Ice Lake) with 128G memory
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202407242159.893be500-oliver.sang@intel.com
>
>
> [  414.980354][T21255] ------------[ cut here ]------------
> [  414.989024][T21255] UBSAN: shift-out-of-bounds in arch/x86/kvm/x86.h:552:47
> [  415.001167][T21255] shift exponent 4294967295 is too large for 32-bit type 'int'
> [  415.011803][T21255] CPU: 107 PID: 21255 Comm: qemu-system-x86 Not tainted 6.10.0-rc2-00186-g1635eb456480 #1
> [  415.024716][T21255] Call Trace:
> [  415.030982][T21255]  <TASK>
> [415.036836][T21255] dump_stack_lvl (lib/dump_stack.c:117)
> [415.044268][T21255] __ubsan_handle_shift_out_of_bounds (lib/ubsan.c:232 lib/ubsan.c:468)
> [415.053610][T21255] kvm_emulate_hypercall.cold (include/trace/events/kvm.h:213 (discriminator 6)) kvm
> [415.063097][T21255] ? __pfx_kvm_emulate_hypercall (arch/x86/kvm/x86.c:10206) kvm
> [415.073104][T21255] ? __vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:6469) kvm_intel
> [415.082284][T21255] vmx_handle_exit (arch/x86/kvm/vmx/vmx.c:6632 (discriminator 1)) kvm_intel
> [415.090893][T21255] vcpu_enter_guest+0x130f/0x3350 kvm
> [415.100855][T21255] ? vmx_segment_cache_test_set (arch/x86/include/asm/bitops.h:206 (discriminator 1) arch/x86/include/asm/bitops.h:238 (discriminator 1) include/asm-generic/bitops/instrumented-non-atomic.h:142 (discriminator 1) arch/x86/kvm/vmx/../kvm_cache_regs.h:56 (discriminator 1) arch/x86/kvm/vmx/vmx.c:825 (discriminator 1)) kvm_intel
> [415.110593][T21255] ? __pfx_vcpu_enter_guest+0x10/0x10 kvm
> [415.120837][T21255] ? vmx_read_guest_seg_ar (arch/x86/kvm/vmx/vmx.c:865 (discriminator 1)) kvm_intel
> [415.130124][T21255] ? skip_emulated_instruction (arch/x86/kvm/vmx/vmx.c:1775) kvm_intel
> [415.139821][T21255] ? __pfx_skip_emulated_instruction (arch/x86/kvm/vmx/vmx.c:1715) kvm_intel
> [415.149853][T21255] ? __pfx_kvm_get_linear_rip (arch/x86/kvm/x86.c:13256) kvm
> [415.159211][T21255] vcpu_run (arch/x86/kvm/x86.c:11311) kvm
> [415.167028][T21255] kvm_arch_vcpu_ioctl_run (arch/x86/kvm/x86.c:11537) kvm
> [415.176327][T21255] ? __pfx_do_vfs_ioctl (fs/ioctl.c:805)
> [415.184065][T21255] kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4440) kvm
> [415.192450][T21255] ? __pfx_kvm_vcpu_ioctl (arch/x86/kvm/../../../virt/kvm/kvm_main.c:4394) kvm
> [415.201351][T21255] ? down_read_trylock (arch/x86/include/asm/atomic64_64.h:20 include/linux/atomic/atomic-arch-fallback.h:2629 include/linux/atomic/atomic-long.h:79 include/linux/atomic/atomic-instrumented.h:3224 kernel/locking/rwsem.c:176 kernel/locking/rwsem.c:181 kernel/locking/rwsem.c:1288 kernel/locking/rwsem.c:1565)
> [415.209117][T21255] ? __fget_light (fs/file.c:1154)
> [415.216411][T21255] ? fput (arch/x86/include/asm/atomic64_64.h:61 (discriminator 1) include/linux/atomic/atomic-arch-fallback.h:4404 (discriminator 1) include/linux/atomic/atomic-long.h:1571 (discriminator 1) include/linux/atomic/atomic-instrumented.h:4540 (discriminator 1) fs/file_table.c:473 (discriminator 1))
> [415.222864][T21255] ? __fget_light (fs/file.c:1154)
> [415.230119][T21255] __x64_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:907 fs/ioctl.c:893 fs/ioctl.c:893)
> [415.237407][T21255] do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1) arch/x86/entry/common.c:83 (discriminator 1))
> [415.244400][T21255] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> [  415.252801][T21255] RIP: 0033:0x7f12912f8c5b
> [ 415.259801][T21255] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
> All code
> ========
>     0:	00 48 89             	add    %cl,-0x77(%rax)
>     3:	44 24 18             	rex.R and $0x18,%al
>     6:	31 c0                	xor    %eax,%eax
>     8:	48 8d 44 24 60       	lea    0x60(%rsp),%rax
>     d:	c7 04 24 10 00 00 00 	movl   $0x10,(%rsp)
>    14:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
>    19:	48 8d 44 24 20       	lea    0x20(%rsp),%rax
>    1e:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
>    23:	b8 10 00 00 00       	mov    $0x10,%eax
>    28:	0f 05                	syscall
>    2a:*	89 c2                	mov    %eax,%edx		<-- trapping instruction
>    2c:	3d 00 f0 ff ff       	cmp    $0xfffff000,%eax
>    31:	77 1c                	ja     0x4f
>    33:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
>    38:	64                   	fs
>    39:	48                   	rex.W
>    3a:	2b                   	.byte 0x2b
>    3b:	04 25                	add    $0x25,%al
>    3d:	28 00                	sub    %al,(%rax)
> 	...
>
> Code starting with the faulting instruction
> ===========================================
>     0:	89 c2                	mov    %eax,%edx
>     2:	3d 00 f0 ff ff       	cmp    $0xfffff000,%eax
>     7:	77 1c                	ja     0x25
>     9:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
>     e:	64                   	fs
>     f:	48                   	rex.W
>    10:	2b                   	.byte 0x2b
>    11:	04 25                	add    $0x25,%al
>    13:	28 00                	sub    %al,(%rax)
> 	...
> [  415.282007][T21255] RSP: 002b:00007f128e7ff5e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  415.293025][T21255] RAX: ffffffffffffffda RBX: 000055cecae83b00 RCX: 00007f12912f8c5b
> [  415.303708][T21255] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 000000000000000c
> [  415.314228][T21255] RBP: 000000000000ae80 R08: 0000000000000000 R09: 0000000000000000
> [  415.324787][T21255] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> [  415.335326][T21255] R13: 0000000000000001 R14: 00000000000003f8 R15: 0000000000000000
> [  415.345809][T21255]  </TASK>
> [  415.351386][T21255] ---[ end trace ]---
>
>
>
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20240724/202407242159.893be500-oliver.sang@intel.com
>
>
>