Re: [Xen-devel] [BUGFIX][PATCH v2 2/5] dbg_rw_guest_mem: need to call put_gfn in error path.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/01/2014 00:25, Don Slutz wrote:
> Using a 1G hvm domU (in grub) and gdbsx:
>
> (gdb) set arch i8086
> warning: A handler for the OS ABI "GNU/Linux" is not built into this 
> configuration
> of GDB.  Attempting to continue with the default i8086 settings.
>
> The target architecture is assumed to be i8086
> (gdb) target remote localhost:9999
> Remote debugging using localhost:9999
> Remote debugging from host 127.0.0.1
> 0x0000d475 in ?? ()
> (gdb) x/1xh 0x6ae9168b
>
> Will reproduce this bug.
>
> With a debug=y build you will get:
>
> Assertion '!preempt_count()' failed at preempt.c:37
>
> For a debug=n build you will get a dom0 VCPU hung (at some point) in:
>
>          [ffff82c4c0126eec] _write_lock+0x3c/0x50
>           ffff82c4c01e43a0  __get_gfn_type_access+0x150/0x230
>           ffff82c4c0158885  dbg_rw_mem+0x115/0x360
>           ffff82c4c0158fc8  arch_do_domctl+0x4b8/0x22f0
>           ffff82c4c01709ed  get_page+0x2d/0x100
>           ffff82c4c01031aa  do_domctl+0x2ba/0x11e0
>           ffff82c4c0179662  do_mmuext_op+0x8d2/0x1b20
>           ffff82c4c0183598  __update_vcpu_system_time+0x288/0x340
>           ffff82c4c015c719  continue_nonidle_domain+0x9/0x30
>           ffff82c4c012938b  add_entry+0x4b/0xb0
>           ffff82c4c02223f9  syscall_enter+0xa9/0xae
>
> And gdb output:
>
> (gdb) x/1xh 0x6ae9168b
> 0x6ae9168b:     0x3024
> (gdb) x/1xh 0x6ae9168b
> 0x6ae9168b:     Ignoring packet error, continuing...
> Reply contains invalid hex digit 116
>
> The 1st one worked because the p2m.lock is recursive and the PCPU
> had not yet changed.
>
> crash reports (for example):
>
> crash> mm_rwlock_t 0xffff83083f913010
> struct mm_rwlock_t {
>   lock = {
>     raw = {
>       lock = 2147483647
>     },
>     debug = {<No data fields>}
>   },
>   unlock_level = 0,
>   recurse_count = 1,
>   locker = 1,
>   locker_function = 0xffff82c4c022c640 <__func__.13514> 
> "__get_gfn_type_access"
> }
>
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>

Technically this should include by Signed-off-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> tag (being the author of the code) as well
as your own (being the discoverer of the bug and author of the commit
message), but I notice I accidentally omitted it from the original email
thread, so my apologies.

It should probably also include your Tested-by: tag

Ian (with RM hat on):
  This is a hypervisor reference counting error on a toolstack hypercall
path.  Irrespective of any of the other patches in this series, I think
this should be included ASAP (although probably subject to review from a
third person), which will fix the hypervisor crashes from gdbsx usage.

~Andrew

> ---
>  xen/arch/x86/debug.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
> index a67a192..ba6a64d 100644
> --- a/xen/arch/x86/debug.c
> +++ b/xen/arch/x86/debug.c
> @@ -63,10 +63,17 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int 
> toaddr,
>      if ( p2m_is_readonly(gfntype) && toaddr )
>      {
>          DBGP2("kdb:p2m_is_readonly: gfntype:%x\n", gfntype);
> -        return INVALID_MFN;
> +        mfn = INVALID_MFN;
>      }
>  
>      DBGP2("X: vaddr:%lx domid:%d mfn:%lx\n", vaddr, dp->domain_id, mfn);
> +
> +    if ( mfn == INVALID_MFN )
> +    {
> +        put_gfn(dp, *gfn);
> +        *gfn = INVALID_GFN;
> +    }
> +
>      return mfn;
>  }
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel