Re: [PATCH v2] x86/traps: fix an off-by-one error

2020-05-05 Thread Jan Beulich
On 05.05.2020 16:17, Hongyan Xia wrote:
> From: Hongyan Xia 
> 
> stack++ can go into the next page and unmap_domain_page() will unmap the
> wrong one, causing mapcache and memory corruption. Fix.
> 
> Signed-off-by: Hongyan Xia 

Reviewed-by: Jan Beulich 



[PATCH v2] x86/traps: fix an off-by-one error

2020-05-05 Thread Hongyan Xia
From: Hongyan Xia 

stack++ can go into the next page and unmap_domain_page() will unmap the
wrong one, causing mapcache and memory corruption. Fix.

Signed-off-by: Hongyan Xia 

---
Changed in v2:
- tweak how the unmap is handled.
- fix the bug in compat as well.
- remove part of the commit message which was not accurate.
---
 xen/arch/x86/traps.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 33e5d21ece..73c6218660 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -236,6 +236,7 @@ static void compat_show_guest_stack(struct vcpu *v,
 int debug_stack_lines)
 {
 unsigned int i, *stack, addr, mask = STACK_SIZE;
+void *stack_page = NULL;
 
 stack = (unsigned int *)(unsigned long)regs->esp;
 printk("Guest stack trace from esp=%08lx:\n ", (unsigned long)stack);
@@ -258,7 +259,7 @@ static void compat_show_guest_stack(struct vcpu *v,
 break;
 if ( !vcpu )
 {
-stack = do_page_walk(v, (unsigned long)stack);
+stack_page = stack = do_page_walk(v, (unsigned long)stack);
 if ( (unsigned long)stack < PAGE_SIZE )
 {
 printk("Inaccessible guest memory.\n");
@@ -285,11 +286,9 @@ static void compat_show_guest_stack(struct vcpu *v,
 printk(" %08x", addr);
 stack++;
 }
-if ( mask == PAGE_SIZE )
-{
-BUILD_BUG_ON(PAGE_SIZE == STACK_SIZE);
-unmap_domain_page(stack);
-}
+
+UNMAP_DOMAIN_PAGE(stack_page);
+
 if ( i == 0 )
 printk("Stack empty.");
 printk("\n");
@@ -300,6 +299,7 @@ static void show_guest_stack(struct vcpu *v, const struct 
cpu_user_regs *regs)
 int i;
 unsigned long *stack, addr;
 unsigned long mask = STACK_SIZE;
+void *stack_page = NULL;
 
 /* Avoid HVM as we don't know what the stack looks like. */
 if ( is_hvm_vcpu(v) )
@@ -328,7 +328,7 @@ static void show_guest_stack(struct vcpu *v, const struct 
cpu_user_regs *regs)
 vcpu = maddr_get_owner(read_cr3()) == v->domain ? v : NULL;
 if ( !vcpu )
 {
-stack = do_page_walk(v, (unsigned long)stack);
+stack_page = stack = do_page_walk(v, (unsigned long)stack);
 if ( (unsigned long)stack < PAGE_SIZE )
 {
 printk("Inaccessible guest memory.\n");
@@ -355,11 +355,9 @@ static void show_guest_stack(struct vcpu *v, const struct 
cpu_user_regs *regs)
 printk(" %p", _p(addr));
 stack++;
 }
-if ( mask == PAGE_SIZE )
-{
-BUILD_BUG_ON(PAGE_SIZE == STACK_SIZE);
-unmap_domain_page(stack);
-}
+
+UNMAP_DOMAIN_PAGE(stack_page);
+
 if ( i == 0 )
 printk("Stack empty.");
 printk("\n");
-- 
2.17.1