From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
To: gdb-patches@sourceware.org
Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Subject: [RFA] Fix use after free caused by redefining a python prefix cmd that has aliases.
Date: Mon, 26 Dec 2022 18:08:23 +0100 [thread overview]
Message-ID: <20221226170823.2744157-1-philippe.waroquiers@skynet.be> (raw)
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
next reply other threads:[~2022-12-26 17:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-26 17:08 Philippe Waroquiers [this message]
2023-01-09 23:37 ` Philippe Waroquiers
2023-01-10 12:16 ` Andrew Burgess
2023-01-11 10:58 ` Philippe Waroquiers
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=20221226170823.2744157-1-philippe.waroquiers@skynet.be \
--to=philippe.waroquiers@skynet.be \
--cc=gdb-patches@sourceware.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 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).