Re: [PATCH] KVM: x86: do not fail if software breakpoint has already been removed

[Stefano Garzarella]

On Mon, Mar 01, 2021 at 02:56:40PM +0200, Maxim Levitsky wrote:
> On Mon, 2021-03-01 at 12:17 +0100, Paolo Bonzini wrote:
> > If kvm_arch_remove_sw_breakpoint finds that a software breakpoint does not
> > have an INT3 instruction, it fails.  This can happen if one sets a
> > software breakpoint in a kernel module and then reloads it.  gdb then
> > thinks the breakpoint cannot be deleted and there is no way to add it
> > back.
> >
> > Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  target/i386/kvm/kvm.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> > index 0b5755e42b..c8d61daf68 100644
> > --- a/target/i386/kvm/kvm.c
> > +++ b/target/i386/kvm/kvm.c
> > @@ -4352,8 +4352,13 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct 
> > kvm_sw_breakpoint *bp)
> >  {
> >      uint8_t int3;
> >
> > -    if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
> > -        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) {
> > +    if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0)) {
> > +        return -EINVAL;
> > +    }
> > +    if (int3 != 0xcc) {
> > +        return 0;
> > +    }
> > +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) {
> >          return -EINVAL;
> >      }
> >      return 0;
>
> There still remains a philosopical question if kvm_arch_remove_sw_breakpoint
> should always return 0, since for the usual case of kernel debugging where
> a breakpoint is in unloaded module, the above will probably still fail
> as paging for this module is removed as well by the kernel.
> It is still better though so:
>
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>
> Note that I managed to make lx-symbols to work in a very stable way
> with attached WIP patch I hacked on this Sunday.
> I will send a cleaned up version of it to upstream when I have time.
>
> Since I make gdb unload the symbols, it works even without this patch.
>
> Added Stefano Garzarella to CC as I know that he tried to make this work as 
> well.
> https://lkml.org/lkml/2020/10/5/514

Thanks Maxim for CCing me!

Just a report when I tried these patches, but I'm not sure they are 
related.

I found that gdb 10 has some problem with QEMU:

    $ gdb --version
    GNU gdb (GDB) Fedora 10.1-2.fc33

    (gdb) lx-symbols
    loading vmlinux
    scanning for modules in linux/build
    ../../gdb/dwarf2/frame.c:1085: internal-error: Unknown CFA rule.

With gdb 9 'lx-symbols' works well, but I still have some issue when I 
put a breakpoint to a symbol not yet loaded (vsock_core_register in this 
example), then I load the module (vsock_loopback in this example) in the 
guest.

Whit your patch gdb stuck after loading the symbols of the first new 
module:
    (gdb) b vsock_core_register
    Function "vsock_core_register" not defined.
    Make breakpoint pending on future shared library load? (y or [n]) y
    Breakpoint 1 (vsock_core_register) pending.
    (gdb) c
    Continuing.
    loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko

Without your patch, gdb loops infinitely reloading the new module:
    refreshing all symbols to reload module 'vsock'
    loading vmlinux
    loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko
    loading @0xffffffffc00ad000: linux/build/drivers/net/tun.ko
    loading @0xffffffffc007e000: linux/build/net/bridge/bridge.ko
    loading @0xffffffffc0077000: linux/build/net/802/stp.ko
    loading @0xffffffffc0007000: linux/build/net/llc/llc.ko
    loading @0xffffffffc0013000: linux/build/net/sunrpc/sunrpc.ko
    loading @0xffffffffc000d000: linux/build/net/ipv4/netfilter/ip_tables.ko
    loading @0xffffffffc0000000: linux/build/net/netfilter/x_tables.ko
    refreshing all symbols to reload module 'vsock'
    loading vmlinux
    loading @0xffffffffc00a1000: linux/build/net/vmw_vsock/vsock.ko
    loading @0xffffffffc00ad000: linux/build/drivers/net/tun.ko
    ...

I'll try to get a better look at what's going on.

I'm using Linux v5.11 in the guest, and the master of QEMU (commit 
51db2d7cf26d05a961ec0ee0eb773594b32cc4a1) with Paolo's patch applied.

Thanks,
Stefano