public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Fix use after free caused by redefining a python prefix cmd that has aliases.
@ 2022-12-26 17:08 Philippe Waroquiers
  2023-01-09 23:37 ` Philippe Waroquiers
  2023-01-10 12:16 ` Andrew Burgess
  0 siblings, 2 replies; 4+ messages in thread
From: Philippe Waroquiers @ 2022-12-26 17:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

GDB gets a segfault when redefining a python prefix command that has aliases.
This is reproduced by running the modified py-cmd.exp (see SEGV stacktrace
below and some valgring reported errors).

Fix the problem by ensuring the aliases of the redefined python command get a
copy of all the new member values of the redefined command.

Tested on amd64/debian. Also run all tests under valgrind.

Fatal signal: Segmentation fault
  ----- Backtrace -----
  0x564d0f2cff7e gdb_internal_backtrace_1
          ../../termours/gdb/bt-utils.c:122
  0x564d0f2d0023 _Z22gdb_internal_backtracev
          ../../termours/gdb/bt-utils.c:168
  0x564d0f4bc969 handle_fatal_signal
          ../../termours/gdb/event-top.c:956
  0x564d0f4bcae4 handle_sigsegv
          ../../termours/gdb/event-top.c:1029
  0x7fb82fe0613f ???
          ./nptl/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
  0x564d0f3290a5 lookup_cmd_with_subcommands
          ../../termours/gdb/cli/cli-decode.c:80
  0x564d0f3290b6 lookup_cmd_with_subcommands
          ../../termours/gdb/cli/cli-decode.c:80
  0x564d0f329647 do_add_cmd
          ../../termours/gdb/cli/cli-decode.c:225
  0x564d0f32968f _Z7add_cmdPKc13command_classS0_PP16cmd_list_element
          ../../termours/gdb/cli/cli-decode.c:236
  0x564d0f6965e4 cmdpy_init
          ../../termours/gdb/python/py-cmd.c:520
  0x564d0fb4a87b wrap_init

Valgrind reports:
  ==1309561== Invalid read of size 8
  ==1309561==    at 0x48F12E: lookup_cmd_with_subcommands (cli-decode.c:80)
  ==1309561==    by 0x48F12E: do_add_cmd(char const*, command_class, char const*, cmd_list_element**) (cli-decode.c:225)
  ==1309561==    by 0x4906D9: add_cmd (cli-decode.c:247)
  ==1309561==    by 0x4906D9: add_prefix_cmd(char const*, command_class, void (*)(char const*, int), char const*, cmd_list_element**, int, cmd_list_element**) (cli-decode.c:362)
  ==1309561==    by 0x6C04DD: cmdpy_init(_object*, _object*, _object*) (py-cmd.c:514)
  ==1309561==    by 0xA14D2B: wrap_init (typeobject.c:5993)
  ==1309561==    by 0x9C663F: _PyObject_MakeTpCall (call.c:159)
  ==1309561==    by 0x3B4052: _PyObject_Vectorcall (abstract.h:125)
  ==1309561==    by 0x3B4052: _PyObject_Vectorcall (abstract.h:115)
  ==1309561==    by 0x3B4052: call_function (ceval.c:4987)
  ...
  ==1309561==  Address 0x7808f38 is 40 bytes inside a block of size 64 free'd
  ==1309561==    at 0x483BF4C: free (vg_replace_malloc.c:884)
  ==1309561==    by 0xA155AA: subtype_dealloc (typeobject.c:1287)
  ==1309561==    by 0x6BF4E7: _Py_DECREF (object.h:478)
  ==1309561==    by 0x6BF4E7: decref (py-ref.h:36)
  ==1309561==    by 0x6BF4E7: ~ref_ptr (gdb_ref_ptr.h:91)
  ==1309561==    by 0x6BF4E7: cmdpy_destroyer(cmd_list_element*, void*) (py-cmd.c:96)
  ==1309561==    by 0x48EEFF: delete_cmd (cli-decode.c:1253)
  ==1309561==    by 0x48EEFF: do_add_cmd(char const*, command_class, char const*, cmd_list_element**) (cli-decode.c:190)
  ==1309561==    by 0x4906D9: add_cmd (cli-decode.c:247)
  ==1309561==    by 0x4906D9: add_prefix_cmd(char const*, command_class, void (*)(char const*, int), char const*, cmd_list_element**, int, cmd_list_element**) (cli-decode.c:362)
  ==1309561==    by 0x6C04DD: cmdpy_init(_object*, _object*, _object*) (py-cmd.c:514)
  ==1309561==    by 0xA14D2B: wrap_init (typeobject.c:5993)
  ==1309561==    by 0x9C663F: _PyObject_MakeTpCall (call.c:159)
  ==1309561==    by 0x3B4052: _PyObject_Vectorcall (abstract.h:125)
  ...
  ==1309561==  Block was alloc'd at
  ==1309561==    at 0x483979B: malloc (vg_replace_malloc.c:393)
  ==1309561==    by 0xAD2C4D: _PyObject_GC_Alloc (gcmodule.c:1964)
  ==1309561==    by 0xAD2C4D: _PyObject_GC_Malloc (gcmodule.c:1987)
  ==1309561==    by 0xA16DE9: PyType_GenericAlloc (typeobject.c:1010)
  ==1309561==    by 0xA18F44: type_call (typeobject.c:969)
  ==1309561==    by 0x9C663F: _PyObject_MakeTpCall (call.c:159)
  ==1309561==    by 0x3B069F: _PyObject_Vectorcall (abstract.h:125)
  ==1309561==    by 0x3B069F: _PyObject_Vectorcall (abstract.h:115)
  ==1309561==    by 0x3B069F: call_function (ceval.c:4987)
  ==1309561==    by 0x3B069F: _PyEval_EvalFrameDefault (ceval.c:3500)
  ==1309561==    by 0xA75253: PyEval_EvalFrameEx (ceval.c:741)
  ==1309561==    by 0xA75253: _PyEval_EvalCodeWithName (ceval.c:4298)
  ==1309561==    by 0xA755B2: PyEval_EvalCodeEx (ceval.c:4327)
  ==1309561==    by 0xA755B2: PyEval_EvalCode (ceval.c:718)
  ==1309561==    by 0xAB1402: run_eval_code_obj (pythonrun.c:1117)
  ==1309561==    by 0xAB1402: run_mod (pythonrun.c:1139)
  ==1309561==    by 0xAB27F0: PyRun_StringFlags (pythonrun.c:1026)
  ==1309561==    by 0xAB27F0: PyRun_SimpleStringFlags (pythonrun.c:460)
  ==1309561==    by 0x6F689B: gdbpy_eval_from_control_command(extension_language_defn const*, command_line*) (python.c:434)
  ==1309561==    by 0x49F54C: execute_control_command_1(command_line*, int) (cli-script.c:683)
---
 gdb/cli/cli-decode.c                     | 19 ++++++++++------
 gdb/python/py-cmd.c                      |  9 +++++++-
 gdb/testsuite/gdb.python/py-cmd-alias.py | 28 ++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-cmd.exp      | 17 ++++++++++++++
 4 files changed, 65 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-cmd-alias.py

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 7c98029f9f4..132b0ca659a 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -179,18 +179,26 @@ cmd_list_element::command_components () const
 
 static struct cmd_list_element *
 do_add_cmd (const char *name, enum command_class theclass,
+	    cmd_simple_func_ftype *fun,
 	    const char *doc, struct cmd_list_element **list)
 {
   struct cmd_list_element *c = new struct cmd_list_element (name, theclass,
 							    doc);
 
+  set_cmd_simple_func (c, fun);
+
   /* Turn each alias of the old command into an alias of the new
      command.  */
   c->aliases = delete_cmd (name, list, &c->hook_pre, &c->hookee_pre,
 			   &c->hook_post, &c->hookee_post);
-
   for (cmd_list_element &alias : c->aliases)
-    alias.alias_target = c;
+    {
+      alias.func = c->func;
+      alias.function = c->function;
+      alias.subcommands = c->subcommands;
+      alias.allow_unknown = c->allow_unknown;
+      alias.alias_target = c;
+    }
 
   if (c->hook_pre)
     c->hook_pre->hookee_pre = c;
@@ -233,9 +241,7 @@ struct cmd_list_element *
 add_cmd (const char *name, enum command_class theclass,
 	 const char *doc, struct cmd_list_element **list)
 {
-  cmd_list_element *result = do_add_cmd (name, theclass, doc, list);
-  result->func = NULL;
-  result->function.simple_func = NULL;
+  cmd_list_element *result = do_add_cmd (name, theclass, NULL, doc, list);
   return result;
 }
 
@@ -244,8 +250,7 @@ add_cmd (const char *name, enum command_class theclass,
 	 cmd_simple_func_ftype *fun,
 	 const char *doc, struct cmd_list_element **list)
 {
-  cmd_list_element *result = do_add_cmd (name, theclass, doc, list);
-  set_cmd_simple_func (result, fun);
+  cmd_list_element *result = do_add_cmd (name, theclass, fun, doc, list);
   return result;
 }
 
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 5cc392af175..d66311d623b 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -524,8 +524,15 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
          name_allocated, so release it.  */
       cmd_name.release ();
 
-      /* There appears to be no API to set this.  */
+      /* There appears to be no API to set the below members.
+	 In particular, we cannot pass cdmpy_function to add_prefix_cmd and
+	 add_cmd above.  This means that if cmd has aliases, its aliases have
+	 been re-targeted to this freshly built CMD with a null func and
+	 function.simple_func (see cli-decode.c do_add_cmd).  So, we need to set
+	 the 'func' of the aliases of CMD.  */
       cmd->func = cmdpy_function;
+      for (cmd_list_element &alias : cmd->aliases)
+	alias.func = cmd->func;
       cmd->destroyer = cmdpy_destroyer;
       cmd->doc_allocated = 1;
       cmd->name_allocated = 1;
diff --git a/gdb/testsuite/gdb.python/py-cmd-alias.py b/gdb/testsuite/gdb.python/py-cmd-alias.py
new file mode 100644
index 00000000000..d62628841f4
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-cmd-alias.py
@@ -0,0 +1,28 @@
+class some_prefix_cmd (gdb.Command):
+  def __init__ (self):
+    super (some_prefix_cmd, self).__init__ ("some_prefix_cmd", gdb.COMMAND_OBSCURE, gdb.COMPLETE_NONE, True)
+  def invoke (self, arg, from_tty):
+    print ("some_prefix_cmd output, arg = %s" % arg)
+
+some_prefix_cmd ()
+
+def def_alias(alias : str, command_name : str) -> None:
+    """Defines an alias -a ALIAS = COMMAND_NAME.
+Traps the error if ALIAS is already defined (so as to be able to source
+this file again)."""
+    d = "alias " + alias + ' = ' + command_name
+    try:
+        gdb.execute (d)
+    except Exception as inst:
+        print(str(inst))
+
+
+def_alias("alias_of_some_prefix_cmd", "some_prefix_cmd")
+def_alias("other_alias_of_some_prefix_cmd", "some_prefix_cmd")
+
+class some_subcmd (gdb.Command):
+  def __init__ (self):
+    super (some_subcmd, self).__init__ ("some_prefix_cmd some_subcmd", gdb.COMMAND_OBSCURE)
+  def invoke (self, arg, from_tty):
+    print ("some_subcmd output, arg = %s" % arg)
+some_subcmd ()
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index 48c3e18f1cc..c7398889fd4 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -111,6 +111,23 @@ gdb_test_multiline "input new subcommand" \
 
 gdb_test "info newsubcmd ugh" "newsubcmd output, arg = ugh" "call newsubcmd"
 
+# Test sourcing twice a file that redefines a prefix command with aliases.
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/py-cmd-alias.py]
+foreach initial_load {1 0} {
+    if $initial_load {
+	gdb_test_no_output "source ${pyfile}" "load python file"
+    } else {
+	gdb_test "source ${pyfile}" \
+	    "Alias already exists: alias_of_some_prefix_cmd\r\nAlias already exists: other_alias_of_some_prefix_cmd" \
+	    "load again python file"
+    }
+    foreach cmd {"some_prefix_cmd" "alias_of_some_prefix_cmd" "other_alias_of_some_prefix_cmd"} {
+	gdb_test "$cmd ugh $initial_load" \
+	    "some_prefix_cmd output, arg = ugh $initial_load" \
+	    "call $cmd $initial_load"
+    }
+}
+
 # Test a command that throws gdb.GdbError.
 
 gdb_test_multiline "input command to throw error" \
-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-11 10:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-26 17:08 [RFA] Fix use after free caused by redefining a python prefix cmd that has aliases Philippe Waroquiers
2023-01-09 23:37 ` Philippe Waroquiers
2023-01-10 12:16 ` Andrew Burgess
2023-01-11 10:58   ` Philippe Waroquiers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).