* [PATCH] x86/monitor: minor left-shift undefined behavior checks
@ 2016-02-18 10:53 Corneliu ZUZU
2016-02-18 14:03 ` Razvan Cojocaru
0 siblings, 1 reply; 2+ messages in thread
From: Corneliu ZUZU @ 2016-02-18 10:53 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Tamas K Lengyel, Keir Fraser, Jan Beulich,
Razvan Cojocaru
This minor patch adds a range-check to avoid left-shift caused undefined
behavior. Also replaces '1 <<' w/ '1U <<' @ x86 monitor.h in an effort to avoid
a future potential '1 << 31' that would cause a similar issue.
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
---
xen/arch/x86/monitor.c | 13 +++++++++----
xen/include/asm-x86/monitor.h | 10 +++++-----
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index a507edb..b4bd008 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -32,10 +32,15 @@ int arch_monitor_domctl_event(struct domain *d,
{
case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
{
- unsigned int ctrlreg_bitmask =
- monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
- bool_t old_status =
- !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
+ unsigned int ctrlreg_bitmask;
+ bool_t old_status;
+
+ /* sanity check: avoid left-shift undefined behavior */
+ if ( unlikely(mop->u.mov_to_cr.index > 31) )
+ return -EINVAL;
+
+ ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
+ old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
if ( unlikely(old_status == requested_status) )
return -EEXIST;
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c789f71..f1bf4bd 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -40,14 +40,14 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
if ( !is_hvm_domain(d) || !cpu_has_vmx )
return capabilities;
- capabilities = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
- (1 << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+ capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
/* Since we know this is on VMX, we can just call the hvm func */
if ( hvm_is_singlestep_supported() )
- capabilities |= (1 << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+ capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
return capabilities;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] x86/monitor: minor left-shift undefined behavior checks
2016-02-18 10:53 [PATCH] x86/monitor: minor left-shift undefined behavior checks Corneliu ZUZU
@ 2016-02-18 14:03 ` Razvan Cojocaru
0 siblings, 0 replies; 2+ messages in thread
From: Razvan Cojocaru @ 2016-02-18 14:03 UTC (permalink / raw)
To: Corneliu ZUZU, xen-devel
Cc: Andrew Cooper, Tamas K Lengyel, Keir Fraser, Jan Beulich
On 02/18/2016 12:53 PM, Corneliu ZUZU wrote:
> This minor patch adds a range-check to avoid left-shift caused undefined
> behavior. Also replaces '1 <<' w/ '1U <<' @ x86 monitor.h in an effort to avoid
> a future potential '1 << 31' that would cause a similar issue.
>
> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
> ---
> xen/arch/x86/monitor.c | 13 +++++++++----
> xen/include/asm-x86/monitor.h | 10 +++++-----
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index a507edb..b4bd008 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -32,10 +32,15 @@ int arch_monitor_domctl_event(struct domain *d,
> {
> case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
> {
> - unsigned int ctrlreg_bitmask =
> - monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
> - bool_t old_status =
> - !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
> + unsigned int ctrlreg_bitmask;
> + bool_t old_status;
> +
> + /* sanity check: avoid left-shift undefined behavior */
> + if ( unlikely(mop->u.mov_to_cr.index > 31) )
> + return -EINVAL;
> +
> + ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
> + old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
>
> if ( unlikely(old_status == requested_status) )
> return -EEXIST;
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index c789f71..f1bf4bd 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -40,14 +40,14 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> if ( !is_hvm_domain(d) || !cpu_has_vmx )
> return capabilities;
>
> - capabilities = (1 << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> - (1 << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> - (1 << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
> - (1 << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
> + capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
> + (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
>
> /* Since we know this is on VMX, we can just call the hvm func */
> if ( hvm_is_singlestep_supported() )
> - capabilities |= (1 << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> + capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
>
> return capabilities;
> }
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-02-18 14:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-18 10:53 [PATCH] x86/monitor: minor left-shift undefined behavior checks Corneliu ZUZU
2016-02-18 14:03 ` Razvan Cojocaru
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.