Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
Please login to access the resource
SUSE:SLE-12-SP5:Update
xen.19021
xsa296.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File xsa296.patch of Package xen.19021
From: Andrew Cooper <andrew.cooper3@citrix.com> Subject: xen/hypercall: Don't use BUG() for parameter checking in hypercall_create_continuation() Since c/s 1d429034 "hypercall: update vcpu_op to take an unsigned vcpuid", which incorrectly swapped 'i' for 'u' in the parameter type list, guests have been able to hit the BUG() in next_args()'s default case. Correct these back to 'i'. In addition, make adjustments to prevent this class of issue from occurring in the future - crashing Xen is not an appropriate form of parameter checking. Capitalise NEXT_ARG() to catch all uses, to highlight that it is a macro doing non-function-like things behind the scenes, and undef it when appropriate. Implement a bad_fmt: block which prints an error, asserts unreachable, and crashes the guest. On the ARM side, drop all parameter checking of p. It is asymmetric with the x86 side, and akin to expecting memcpy() or sprintf() to check their src/fmt parameter before use. A caller passing "" or something other than a string literal will be obvious during code review. This is XSA-296. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Julien Grall <julien.grall@arm.com> Index: xen-4.7.6-testing/xen/arch/arm/domain.c =================================================================== --- xen-4.7.6-testing.orig/xen/arch/arm/domain.c +++ xen-4.7.6-testing/xen/arch/arm/domain.c @@ -310,14 +310,15 @@ void sync_vcpu_execstate(struct vcpu *v) /* Nothing to do -- no lazy switching */ } -#define next_arg(fmt, args) ({ \ +#define NEXT_ARG(fmt, args) \ +({ \ unsigned long __arg; \ switch ( *(fmt)++ ) \ { \ case 'i': __arg = (unsigned long)va_arg(args, unsigned int); break; \ case 'l': __arg = (unsigned long)va_arg(args, unsigned long); break; \ case 'h': __arg = (unsigned long)va_arg(args, void *); break; \ - default: __arg = 0; BUG(); \ + default: goto bad_fmt; \ } \ __arg; \ }) @@ -347,9 +348,6 @@ unsigned long hypercall_create_continuat unsigned int i; va_list args; - /* All hypercalls take at least one argument */ - BUG_ON( !p || *p == '\0' ); - va_start(args, format); if ( mcs->flags & MCSF_in_multicall ) @@ -357,7 +355,7 @@ unsigned long hypercall_create_continuat __set_bit(_MCSF_call_preempted, &mcs->flags); for ( i = 0; *p != '\0'; i++ ) - mcs->call.args[i] = next_arg(p, args); + mcs->call.args[i] = NEXT_ARG(p, args); /* Return value gets written back to mcs->call.result */ rc = mcs->call.result; @@ -376,7 +374,7 @@ unsigned long hypercall_create_continuat for ( i = 0; *p != '\0'; i++ ) { - arg = next_arg(p, args); + arg = NEXT_ARG(p, args); switch ( i ) { @@ -399,7 +397,7 @@ unsigned long hypercall_create_continuat for ( i = 0; *p != '\0'; i++ ) { - arg = next_arg(p, args); + arg = NEXT_ARG(p, args); switch ( i ) { @@ -420,8 +418,16 @@ unsigned long hypercall_create_continuat va_end(args); return rc; + + bad_fmt: + gprintk(XENLOG_ERR, "Bad hypercall continuation format '%c'\n", *p); + ASSERT_UNREACHABLE(); + domain_crash(current->domain); + return 0; } +#undef NEXT_ARG + void startup_cpu_idle_loop(void) { struct vcpu *v = current; Index: xen-4.7.6-testing/xen/arch/x86/domain.c =================================================================== --- xen-4.7.6-testing.orig/xen/arch/x86/domain.c +++ xen-4.7.6-testing/xen/arch/x86/domain.c @@ -2371,14 +2371,15 @@ void sync_vcpu_execstate(struct vcpu *v) flush_tlb_mask(v->vcpu_dirty_cpumask); } -#define next_arg(fmt, args) ({ \ +#define NEXT_ARG(fmt, args) \ +({ \ unsigned long __arg; \ switch ( *(fmt)++ ) \ { \ case 'i': __arg = (unsigned long)va_arg(args, unsigned int); break; \ case 'l': __arg = (unsigned long)va_arg(args, unsigned long); break; \ case 'h': __arg = (unsigned long)va_arg(args, void *); break; \ - default: __arg = 0; BUG(); \ + default: goto bad_fmt; \ } \ __arg; \ }) @@ -2417,7 +2418,7 @@ unsigned long hypercall_create_continuat __set_bit(_MCSF_call_preempted, &mcs->flags); for ( i = 0; *p != '\0'; i++ ) - mcs->call.args[i] = next_arg(p, args); + mcs->call.args[i] = NEXT_ARG(p, args); } else { @@ -2438,7 +2439,7 @@ unsigned long hypercall_create_continuat { for ( i = 0; *p != '\0'; i++ ) { - arg = next_arg(p, args); + arg = NEXT_ARG(p, args); switch ( i ) { case 0: regs->rdi = arg; break; @@ -2454,7 +2455,7 @@ unsigned long hypercall_create_continuat { for ( i = 0; *p != '\0'; i++ ) { - arg = next_arg(p, args); + arg = NEXT_ARG(p, args); switch ( i ) { case 0: regs->ebx = arg; break; @@ -2471,8 +2472,16 @@ unsigned long hypercall_create_continuat va_end(args); return op; + + bad_fmt: + gprintk(XENLOG_ERR, "Bad hypercall continuation format '%c'\n", *p); + ASSERT_UNREACHABLE(); + domain_crash(current->domain); + return 0; } +#undef NEXT_ARG + int hypercall_xlat_continuation(unsigned int *id, unsigned int nr, unsigned int mask, ...) { Index: xen-4.7.6-testing/xen/common/compat/domain.c =================================================================== --- xen-4.7.6-testing.orig/xen/common/compat/domain.c +++ xen-4.7.6-testing/xen/common/compat/domain.c @@ -81,7 +81,7 @@ int compat_vcpu_op(int cmd, unsigned int } if ( rc == -ERESTART ) - rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iuh", + rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iih", cmd, vcpuid, arg); break; Index: xen-4.7.6-testing/xen/common/domain.c =================================================================== --- xen-4.7.6-testing.orig/xen/common/domain.c +++ xen-4.7.6-testing/xen/common/domain.c @@ -1283,7 +1283,7 @@ long do_vcpu_op(int cmd, unsigned int vc rc = arch_initialise_vcpu(v, arg); if ( rc == -ERESTART ) - rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iuh", + rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iih", cmd, vcpuid, arg); break;
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor