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: 2942f45e09 ("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>
This commit is contained in:
Andrew Cooper committed 2024-02-01 19:52:44 +00:00
1 parent 9d701bc9ab
commit ae6d4fd876
2 files changed
+3 -3

No files matched your search

+2 -2
View File
@@ -43,9 +43,9 @@ static int compat_poll(struct compat_sched_poll *compat)
#include "core.c"
int compat_set_timer_op(uint32_t lo, int32_t hi)
int compat_set_timer_op(uint32_t lo, uint32_t hi)
{
return do_set_timer_op(((s64)hi << 32) | lo);
return do_set_timer_op(((uint64_t)hi << 32) | lo);
}
#endif /* __COMMON_SCHED_COMPAT_C__ */
+1 -1
View File
@@ -134,7 +134,7 @@ xenoprof_op(int op, void *arg)
#ifdef CONFIG_COMPAT
prefix: compat
set_timer_op(uint32_t lo, int32_t hi)
set_timer_op(uint32_t lo, uint32_t hi)
multicall(multicall_entry_compat_t *call_list, uint32_t nr_calls)
memory_op(unsigned int cmd, void *arg)
#ifdef CONFIG_IOREQ_SERVER
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.