All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxim Levitsky <mlevitsk@redhat.com>
To: kvm@vger.kernel.org
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Wanpeng Li <wanpengli@tencent.com>,
	Kieran Bingham <kbingham@kernel.org>,
	Jessica Yu <jeyu@kernel.org>, Jan Kiszka <jan.kiszka@siemens.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	x86@kernel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)),
	Joerg Roedel <joro@8bytes.org>,
	Sean Christopherson <seanjc@google.com>,
	Jim Mattson <jmattson@google.com>, Borislav Petkov <bp@alien8.de>,
	Stefano Garzarella <sgarzare@redhat.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>
Subject: [PATCH 1/3] scripts/gdb: rework lx-symbols gdb script
Date: Tue, 16 Mar 2021 00:10:18 +0200	[thread overview]
Message-ID: <20210315221020.661693-2-mlevitsk@redhat.com> (raw)
In-Reply-To: <20210315221020.661693-1-mlevitsk@redhat.com>

Fix several issues that are present in lx-symbols script:

* Track module unloads by placing another software breakpoint at 'free_module'
  (force uninline this symbol just in case), and use remove-symbol-file
  gdb command to unload the symobls of the module that is unloading.

  That gives the gdb a chance to mark all software breakpoints from
  this module as pending again.
  Also remove the module from the 'known' module list once it is unloaded.

* Since we now track module unload, we don't need to reload all
  symbols anymore when 'known' module loaded again (that can't happen anymore).
  This allows reloading a module in the debugged kernel to finish much faster,
  while lx-symbols tracks module loads and unloads.

* Disable/enable all gdb breakpoints on both module load and unload breakpoint
  hits, and not only in 'load_all_symbols' as was done before.
  (load_all_symbols is no longer called on breakpoint hit)
  That allows gdb to avoid getting confused about the state of the (now two)
  internal breakpoints we place.

  Otherwise it will leave them in the kernel code segment, when continuing
  which triggers a guest kernel panic as soon as it skips over the 'int3'
  instruction and executes the garbage tail of the optcode on which
  the breakpoint was placed.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 kernel/module.c              |   8 ++-
 scripts/gdb/linux/symbols.py | 106 +++++++++++++++++++++++++----------
 2 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 30479355ab850..ea81fc06ea1f5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -901,8 +901,12 @@ int module_refcount(struct module *mod)
 }
 EXPORT_SYMBOL(module_refcount);
 
-/* This exists whether we can unload or not */
-static void free_module(struct module *mod);
+/* This exists whether we can unload or not
+ * Keep it uninlined to provide a reliable breakpoint target,
+ * e.g. for the gdb helper command 'lx-symbols'.
+ */
+
+static noinline void free_module(struct module *mod);
 
 SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		unsigned int, flags)
diff --git a/scripts/gdb/linux/symbols.py b/scripts/gdb/linux/symbols.py
index 1be9763cf8bb2..4ce879548a1ae 100644
--- a/scripts/gdb/linux/symbols.py
+++ b/scripts/gdb/linux/symbols.py
@@ -17,6 +17,24 @@ import re
 
 from linux import modules, utils
 
+def save_state():
+        breakpoints = []
+        if hasattr(gdb, 'breakpoints') and not gdb.breakpoints() is None:
+            for bp in gdb.breakpoints():
+                breakpoints.append({'breakpoint': bp, 'enabled': bp.enabled})
+                bp.enabled = False
+
+        show_pagination = gdb.execute("show pagination", to_string=True)
+        pagination = show_pagination.endswith("on.\n")
+        gdb.execute("set pagination off")
+
+        return {"breakpoints":breakpoints, "show_pagination": show_pagination}
+
+def load_state(state):
+    for breakpoint in state["breakpoints"]:
+        breakpoint['breakpoint'].enabled = breakpoint['enabled']
+    gdb.execute("set pagination %s" % ("on" if state["show_pagination"] else "off"))
+
 
 if hasattr(gdb, 'Breakpoint'):
     class LoadModuleBreakpoint(gdb.Breakpoint):
@@ -30,26 +48,38 @@ if hasattr(gdb, 'Breakpoint'):
             module_name = module['name'].string()
             cmd = self.gdb_command
 
+            # module already loaded, false alarm
+            if module_name in cmd.loaded_modules:
+                return False
+
             # enforce update if object file is not found
             cmd.module_files_updated = False
 
             # Disable pagination while reporting symbol (re-)loading.
             # The console input is blocked in this context so that we would
             # get stuck waiting for the user to acknowledge paged output.
-            show_pagination = gdb.execute("show pagination", to_string=True)
-            pagination = show_pagination.endswith("on.\n")
-            gdb.execute("set pagination off")
+            state = save_state()
+            cmd.load_module_symbols(module)
+            load_state(state)
+            return False
 
-            if module_name in cmd.loaded_modules:
-                gdb.write("refreshing all symbols to reload module "
-                          "'{0}'\n".format(module_name))
-                cmd.load_all_symbols()
-            else:
-                cmd.load_module_symbols(module)
+    class UnLoadModuleBreakpoint(gdb.Breakpoint):
+        def __init__(self, spec, gdb_command):
+            super(UnLoadModuleBreakpoint, self).__init__(spec, internal=True)
+            self.silent = True
+            self.gdb_command = gdb_command
+
+        def stop(self):
+            module = gdb.parse_and_eval("mod")
+            module_name = module['name'].string()
+            cmd = self.gdb_command
 
-            # restore pagination state
-            gdb.execute("set pagination %s" % ("on" if pagination else "off"))
+            if not module_name in cmd.loaded_modules:
+                return False
 
+            state = save_state()
+            cmd.unload_module_symbols(module)
+            load_state(state)
             return False
 
 
@@ -64,8 +94,9 @@ lx-symbols command."""
     module_paths = []
     module_files = []
     module_files_updated = False
-    loaded_modules = []
-    breakpoint = None
+    loaded_modules = {}
+    module_load_breakpoint = None
+    module_unload_breakpoint = None
 
     def __init__(self):
         super(LxSymbols, self).__init__("lx-symbols", gdb.COMMAND_FILES,
@@ -129,21 +160,32 @@ lx-symbols command."""
                 filename=module_file,
                 addr=module_addr,
                 sections=self._section_arguments(module))
+
             gdb.execute(cmdline, to_string=True)
-            if module_name not in self.loaded_modules:
-                self.loaded_modules.append(module_name)
+            self.loaded_modules[module_name] = {"module_file": module_file,
+                                                "module_addr": module_addr}
         else:
             gdb.write("no module object found for '{0}'\n".format(module_name))
 
+    def unload_module_symbols(self, module):
+        module_name = module['name'].string()
+
+        module_file = self.loaded_modules[module_name]["module_file"]
+        module_addr = self.loaded_modules[module_name]["module_addr"]
+
+        gdb.write("unloading @{addr}: {filename}\n".format(
+            addr=module_addr, filename=module_file))
+        cmdline = "remove-symbol-file {filename}".format(
+            filename=module_file)
+
+        gdb.execute(cmdline, to_string=True)
+        del self.loaded_modules[module_name]
+
+
     def load_all_symbols(self):
         gdb.write("loading vmlinux\n")
 
-        # Dropping symbols will disable all breakpoints. So save their states
-        # and restore them afterward.
-        saved_states = []
-        if hasattr(gdb, 'breakpoints') and not gdb.breakpoints() is None:
-            for bp in gdb.breakpoints():
-                saved_states.append({'breakpoint': bp, 'enabled': bp.enabled})
+        state = save_state()
 
         # drop all current symbols and reload vmlinux
         orig_vmlinux = 'vmlinux'
@@ -153,15 +195,14 @@ lx-symbols command."""
         gdb.execute("symbol-file", to_string=True)
         gdb.execute("symbol-file {0}".format(orig_vmlinux))
 
-        self.loaded_modules = []
+        self.loaded_modules = {}
         module_list = modules.module_list()
         if not module_list:
             gdb.write("no modules found\n")
         else:
             [self.load_module_symbols(module) for module in module_list]
 
-        for saved_state in saved_states:
-            saved_state['breakpoint'].enabled = saved_state['enabled']
+        load_state(state)
 
     def invoke(self, arg, from_tty):
         self.module_paths = [os.path.expanduser(p) for p in arg.split()]
@@ -174,11 +215,18 @@ lx-symbols command."""
         self.load_all_symbols()
 
         if hasattr(gdb, 'Breakpoint'):
-            if self.breakpoint is not None:
-                self.breakpoint.delete()
-                self.breakpoint = None
-            self.breakpoint = LoadModuleBreakpoint(
-                "kernel/module.c:do_init_module", self)
+            if self.module_load_breakpoint is not None:
+                self.module_load_breakpoint.delete()
+                self.module_load_breakpoint = None
+            self.module_load_breakpoint = \
+                LoadModuleBreakpoint("kernel/module.c:do_init_module", self)
+
+            if self.module_unload_breakpoint is not None:
+                self.module_unload_breakpoint.delete()
+                self.module_unload_breakpoint = None
+            self.module_unload_breakpoint = \
+                UnLoadModuleBreakpoint("kernel/module.c:free_module", self)
+
         else:
             gdb.write("Note: symbol update on module loading not supported "
                       "with this gdb version\n")
-- 
2.26.2


  reply	other threads:[~2021-03-15 22:11 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15 22:10 [PATCH 0/3] KVM: my debug patch queue Maxim Levitsky
2021-03-15 22:10 ` Maxim Levitsky [this message]
2021-03-16 13:38   ` [PATCH 1/3] scripts/gdb: rework lx-symbols gdb script Jan Kiszka
2021-03-16 14:12     ` Maxim Levitsky
2021-03-15 22:10 ` [PATCH 2/3] KVM: x86: guest debug: don't inject interrupts while single stepping Maxim Levitsky
2021-03-15 23:37   ` Sean Christopherson
2021-03-16  9:16     ` Jan Kiszka
2021-03-16 10:59       ` Maxim Levitsky
2021-03-16 11:27         ` Jan Kiszka
2021-03-16 12:34           ` Maxim Levitsky
2021-03-16 13:46             ` Jan Kiszka
2021-03-16 14:34               ` Maxim Levitsky
2021-03-16 15:31                 ` Jan Kiszka
     [not found]                   ` <e2cd978e357155dbab21a523bb8981973bd10da7.camel@redhat.com>
2021-03-16 15:56                     ` Jan Kiszka
2021-03-16 16:50                     ` Sean Christopherson
2021-03-16 17:01                       ` Jan Kiszka
2021-03-16 17:26                         ` Sean Christopherson
2021-03-17  9:20                           ` Jan Kiszka
2021-03-16 18:02                         ` Maxim Levitsky
2021-03-18 16:02                           ` Jan Kiszka
2021-03-18 12:21             ` Jan Kiszka
2021-03-16 10:55     ` Maxim Levitsky
2021-03-15 22:10 ` [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug Maxim Levitsky
2021-03-16  8:32   ` Joerg Roedel
2021-03-16 10:51     ` Maxim Levitsky
2021-03-18  9:19       ` Joerg Roedel
2021-03-18  9:24         ` Maxim Levitsky
2021-03-18 15:47           ` Joerg Roedel
2021-03-18 16:35             ` Sean Christopherson
2021-03-18 16:41               ` Maxim Levitsky
2021-03-18 17:22                 ` Sean Christopherson
2021-03-16  8:34   ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210315221020.661693-2-mlevitsk@redhat.com \
    --to=mlevitsk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jeyu@kernel.org \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kbingham@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=sgarzare@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.