]> xenbits.xensource.com Git - xen.git/commit
xen/sched: Fix UB shift in compat_set_timer_op()
authorAndrew Cooper <andrew.cooper3@citrix.com>
Tue, 5 Mar 2024 10:57:02 +0000 (11:57 +0100)
committerJan Beulich <jbeulich@suse.com>
Tue, 5 Mar 2024 10:57:02 +0000 (11:57 +0100)
commitb75bee183210318150e678e14b35224d7c73edb6
tree1fe10df0cd98fc95eebb06102347abe4542896b2
parent9c0d518eb8dc69430e6a8d767bd101dad19b846a
xen/sched: Fix UB shift in compat_set_timer_op()

Tamas reported this UBSAN failure from fuzzing:

  (XEN) ================================================================================
  (XEN) UBSAN: Undefined behaviour in common/sched/compat.c:48:37
  (XEN) left shift of negative value -2147425536
  (XEN) ----[ Xen-4.19-unstable  x86_64  debug=y ubsan=y  Not tainted ]----
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d040307c1c>] R ubsan.c#ubsan_epilogue+0xa/0xd9
  (XEN)    [<ffff82d040308afb>] F __ubsan_handle_shift_out_of_bounds+0x11a/0x1c5
  (XEN)    [<ffff82d040307758>] F compat_set_timer_op+0x41/0x43
  (XEN)    [<ffff82d04040e4cc>] F hvm_do_multicall_call+0x77f/0xa75
  (XEN)    [<ffff82d040519462>] F arch_do_multicall_call+0xec/0xf1
  (XEN)    [<ffff82d040261567>] F do_multicall+0x1dc/0xde3
  (XEN)    [<ffff82d04040d2b3>] F hvm_hypercall+0xa00/0x149a
  (XEN)    [<ffff82d0403cd072>] F vmx_vmexit_handler+0x1596/0x279c
  (XEN)    [<ffff82d0403d909b>] F vmx_asm_vmexit_handler+0xdb/0x200

Left-shifting any negative value is strictly undefined behaviour in C, and
the two parameters here come straight from the guest.

The fuzzer happened to choose lo 0xf, hi 0x8000e300.

Switch everything to be unsigned values, making the shift well defined.

As GCC documents:

  As an extension to the C language, GCC does not use the latitude given in
  C99 and C11 only to treat certain aspects of signed '<<' as undefined.
  However, -fsanitize=shift (and -fsanitize=undefined) will diagnose such
  cases.

this was deemed not to need an XSA.

Note: The unsigned -> signed conversion for do_set_timer_op()'s s_time_t
parameter is also well defined.  C makes it implementation defined, and GCC
defines it as reduction modulo 2^N to be within range of the new type.

Fixes: 2942f45e09fb ("Enable compatibility mode operation for HYPERVISOR_sched_op and HYPERVISOR_set_timer_op.")
Reported-by: Tamas K Lengyel <tamas@tklengyel.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ae6d4fd876765e6d623eec67d14f5d0464be09cb
master date: 2024-02-01 19:52:44 +0000
xen/common/sched/compat.c
xen/include/hypercall-defs.c