|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v5 03/15] xen: arm: correctly handle vtimer traps from userspace
Previously 32-bit userspace on 32-bit kernel and 64-bit userspace on
64-bit kernel could access these registers irrespective of whether the
kernel had configured them to be allowed to. To fix this:
- Userspace access to CNTP_CTL_EL0 and CNTP_TVAL_EL0 should be gated
on CNTKCTL_EL1.EL0PTEN.
- Userspace access to CNTPCT_EL0 should be gated on
CNTKCTL_EL1.EL0PCTEN.
When we do not handle an access we now silently inject an undef even
in debug mode since the debugging messages are not helpful (we have
handled the access, by explicitly choosing not to).
The usermode accessibility check is rather repetitive, so a helper
macro is introduced.
Since HSR_EC_CP15_64 cannot be taken from a guest in AArch64 mode
except due to a hardware bug switch the associated check to a BUG_ON
(which will be switched to something more appropriate in a subsequent
patch)
Fix a coding style issue in HSR_CPREG64(CNTPCT) while touching similar
code.
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Reviewed-by: Julien Grall <julien.grall@xxxxxxxxxx>
---
This should be backported
v4: Moved earlier in series. Use a helper macro to check for allowed
accesses
v2: Replaces "xen: arm: turn vtimer traps for cp32/64 and sysreg into #undef"
---
xen/arch/arm/traps.c | 27 ++++++++----------------
xen/arch/arm/vtimer.c | 43 +++++++++++++++++++++++++--------------
xen/include/asm-arm/processor.h | 6 ++++++
3 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c4780f4..fe2c30b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1598,11 +1598,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
case HSR_CPREG32(CNTP_CTL):
case HSR_CPREG32(CNTP_TVAL):
if ( !vtimer_emulate(regs, hsr) )
- {
- dprintk(XENLOG_ERR,
- "failed emulation of 32-bit vtimer CP register access\n");
- domain_crash_synchronous();
- }
+ goto undef_cp15_32;
break;
case HSR_CPREG32(ACTLR):
if ( cp32.read )
@@ -1643,6 +1639,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
gdprintk(XENLOG_ERR, "unhandled 32-bit CP15 access %#x\n",
hsr.bits & HSR_CP32_REGS_MASK);
+ undef_cp15_32:
inject_undef_exception(regs, hsr.len);
return;
}
@@ -1662,11 +1659,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
{
case HSR_CPREG64(CNTPCT):
if ( !vtimer_emulate(regs, hsr) )
- {
- dprintk(XENLOG_ERR,
- "failed emulation of 64-bit vtimer CP register access\n");
- domain_crash_synchronous();
- }
+ goto undef_cp15_64;
break;
default:
{
@@ -1678,6 +1671,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
gdprintk(XENLOG_ERR, "unhandled 64-bit CP15 access %#x\n",
hsr.bits & HSR_CP64_REGS_MASK);
+ undef_cp15_64:
inject_undef_exception(regs, hsr.len);
return;
}
@@ -1836,11 +1830,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
case HSR_SYSREG_CNTP_CTL_EL0:
case HSR_SYSREG_CNTP_TVAL_EL0:
if ( !vtimer_emulate(regs, hsr) )
- {
- dprintk(XENLOG_ERR,
- "failed emulation of 64-bit vtimer sysreg access\n");
- domain_crash_synchronous();
- }
+ goto undef_sysreg;
break;
case HSR_SYSREG_ICC_SGI1R_EL1:
if ( !vgic_emulate(regs, hsr) )
@@ -1871,8 +1861,8 @@ static void do_sysreg(struct cpu_user_regs *regs,
sysreg.reg, regs->pc);
gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
hsr.bits & HSR_SYSREG_REGS_MASK);
-
- inject_undef_exception(regs, sysreg.len);
+ undef_sysreg:
+ inject_undef_exception(regs, hsr.sysreg.len);
return;
}
}
@@ -2048,8 +2038,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs
*regs)
do_cp15_32(regs, hsr);
break;
case HSR_EC_CP15_64:
- if ( !is_32bit_domain(current->domain) )
- goto bad_trap;
+ BUG_ON(!psr_mode_is_32bit(regs->cpsr));
perfc_incr(trap_cp15_64);
do_cp15_64(regs, hsr);
break;
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 287bb93..fac7c3e 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -31,6 +31,14 @@
extern s_time_t ticks_to_ns(uint64_t ticks);
extern uint64_t ns_to_ticks(s_time_t ns);
+/*
+ * Check if regs is allowed access, user_gate is tail end of a
+ * CNTKCTL_EL1_ bit name which gates user access
+ */
+#define ACCESS_ALLOWED(regs, user_gate) \
+ ( !psr_mode_is_user(regs) || \
+ (READ_SYSREG(CNTKCTL_EL1) & CNTKCTL_EL1_##user_gate) )
+
static void phys_timer_expired(void *data)
{
struct vtimer *t = data;
@@ -154,9 +162,13 @@ int virt_timer_restore(struct vcpu *v)
return 0;
}
-static void vtimer_cntp_ctl(struct cpu_user_regs *regs, uint32_t *r, int read)
+static int vtimer_cntp_ctl(struct cpu_user_regs *regs, uint32_t *r, int read)
{
struct vcpu *v = current;
+
+ if ( !ACCESS_ALLOWED(regs, EL0PTEN) )
+ return 0;
+
if ( read )
{
*r = v->arch.phys_timer.ctl;
@@ -176,13 +188,17 @@ static void vtimer_cntp_ctl(struct cpu_user_regs *regs,
uint32_t *r, int read)
else
stop_timer(&v->arch.phys_timer.timer);
}
+ return 1;
}
-static void vtimer_cntp_tval(struct cpu_user_regs *regs, uint32_t *r, int read)
+static int vtimer_cntp_tval(struct cpu_user_regs *regs, uint32_t *r, int read)
{
struct vcpu *v = current;
s_time_t now;
+ if ( !ACCESS_ALLOWED(regs, EL0PTEN) )
+ return 0;
+
now = NOW() - v->domain->arch.phys_timer_base.offset;
if ( read )
@@ -200,6 +216,7 @@ static void vtimer_cntp_tval(struct cpu_user_regs *regs,
uint32_t *r, int read)
v->domain->arch.phys_timer_base.offset);
}
}
+ return 1;
}
static int vtimer_cntpct(struct cpu_user_regs *regs, uint64_t *r, int read)
@@ -210,6 +227,8 @@ static int vtimer_cntpct(struct cpu_user_regs *regs,
uint64_t *r, int read)
if ( read )
{
+ if ( !ACCESS_ALLOWED(regs, EL0PCTEN) )
+ return 0;
now = NOW() - v->domain->arch.phys_timer_base.offset;
ticks = ns_to_ticks(now);
*r = ticks;
@@ -236,12 +255,10 @@ static int vtimer_emulate_cp32(struct cpu_user_regs
*regs, union hsr hsr)
switch ( hsr.bits & HSR_CP32_REGS_MASK )
{
case HSR_CPREG32(CNTP_CTL):
- vtimer_cntp_ctl(regs, r, cp32.read);
- return 1;
+ return vtimer_cntp_ctl(regs, r, cp32.read);
case HSR_CPREG32(CNTP_TVAL):
- vtimer_cntp_tval(regs, r, cp32.read);
- return 1;
+ return vtimer_cntp_tval(regs, r, cp32.read);
default:
return 0;
@@ -263,7 +280,7 @@ static int vtimer_emulate_cp64(struct cpu_user_regs *regs,
union hsr hsr)
switch ( hsr.bits & HSR_CP64_REGS_MASK )
{
case HSR_CPREG64(CNTPCT):
- if (!vtimer_cntpct(regs, &x, cp64.read))
+ if ( !vtimer_cntpct(regs, &x, cp64.read) )
return 0;
if ( cp64.read )
@@ -293,12 +310,14 @@ static int vtimer_emulate_sysreg(struct cpu_user_regs
*regs, union hsr hsr)
switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
{
case HSR_SYSREG_CNTP_CTL_EL0:
- vtimer_cntp_ctl(regs, &r, sysreg.read);
+ if ( !vtimer_cntp_ctl(regs, &r, sysreg.read) )
+ return 0;
if ( sysreg.read )
*x = r;
return 1;
case HSR_SYSREG_CNTP_TVAL_EL0:
- vtimer_cntp_tval(regs, &r, sysreg.read);
+ if ( !vtimer_cntp_tval(regs, &r, sysreg.read) )
+ return 0;
if ( sysreg.read )
*x = r;
return 1;
@@ -318,17 +337,11 @@ int vtimer_emulate(struct cpu_user_regs *regs, union hsr
hsr)
switch (hsr.ec) {
case HSR_EC_CP15_32:
- if ( !is_32bit_domain(current->domain) )
- return 0;
return vtimer_emulate_cp32(regs, hsr);
case HSR_EC_CP15_64:
- if ( !is_32bit_domain(current->domain) )
- return 0;
return vtimer_emulate_cp64(regs, hsr);
#ifdef CONFIG_ARM_64
case HSR_EC_SYSREG:
- if ( is_32bit_domain(current->domain) )
- return 0;
return vtimer_emulate_sysreg(regs, hsr);
#endif
default:
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index fcd26fb..b7e88a6 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -558,6 +558,12 @@ union hsr {
#define CNTHCTL_PA (1u<<0) /* Kernel/user access to physical counter */
#define CNTHCTL_TA (1u<<1) /* Kernel/user access to CNTP timer */
+/* Time counter kernel control register */
+#define CNTKCTL_EL1_EL0PCTEN (1u<<0) /* Expose phys counters to EL0 */
+#define CNTKCTL_EL1_EL0VCTEN (1u<<1) /* Expose virt counters to EL0 */
+#define CNTKCTL_EL1_EL0VTEN (1u<<8) /* Expose virt timer registers to EL0 */
+#define CNTKCTL_EL1_EL0PTEN (1u<<9) /* Expose phys timer registers to EL0 */
+
/* Timer control registers */
#define CNTx_CTL_ENABLE (1u<<0) /* Enable timer */
#define CNTx_CTL_MASK (1u<<1) /* Mask IRQ */
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |