From patchwork Mon Dec 26 17:08:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 62417 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 488553858C54 for ; Mon, 26 Dec 2022 17:08:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 488553858C54 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1672074537; bh=lRLdh6n1MNkNwuuJ1Xzz6W9PUZ2brTALWyOKTF2rhos=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=aCBOINNIIb8/0+4HLpZ3uV6G3ClP3DIBHo2awWGWXGNDEBGsha7O4OGag7rStrauL iCJ3TS5pC1fYTLC7RV2nROCpagJAFSJGefmkp8cAQWItg4/N5jRRaugBBM43km6xvc aaeooTHXzOmCNVgCNgAjf2Q9XJh81hcgyK1x//Xs= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mailsec215.isp.belgacom.be (mailsec215.isp.belgacom.be [195.238.22.111]) by sourceware.org (Postfix) with ESMTPS id 23B1D3858D33 for ; Mon, 26 Dec 2022 17:08:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 23B1D3858D33 X-ExtLoop: 1 X-IPAS-Result: A2AxAwAC1Klj/1uGgG1agRKBRoIFgS+BWAKVa5Fmi1aBfg8BAQEBAQEBAQEJMRMEAQGFBYUSJjQJDgECBAEBAQEDAgMBAQEBAQEDAQEGAQEBAQEBBgQBgRyFLzkNgjgig38rCwFGKINPWAGDIrARMxoCZYRymmaBZ4FAi2CFQzeBVUSBFYE8gT6LcASBCYsljGkKgT19gScOgRsDCQMHBUlAAwsYDRYyChMuNRdLKxobB4EKKigVAwQEAwIGEwMiAg0oMRQEKRMNJyZrCQIDImYDAwQoLQkhHwcmJDwHVjcFAwIPHzcGAwkDAh9PcS8SFAUDCxUqRwQINgUGURICCA8SDyxEDkI3NhMGXAEqCw4TA1BHGW8EggwKLyiabYEuUiAxShMBEy+BCSWTYAGOWJ8jggE0B4NxgUoGDIl3lHMaMoN5kycDGJFoAZZRdaMthFGBYoIVbVOCZwlJGY44AxYViSCEenQIMwIHCwEBAwmMIwEB IronPort-PHdr: A9a23:x/VLnhDzsOySLHI29wk9UyQU1EMY04WdBeb1wqQuh78GSKm/5ZOqZ BWZua8wygKZFtSFo9t/yMPo8InYEVQa5piAtH1QOLdtbDQizfssogo7HcSeAlf6JvO5JwYzH cBFSUM3tyrjaRsdF8nxfUDdrWOv5jAOBBr/KRB1JuPoEYLOksi7ze+/94PTbglShDewYrx+I RG3oA7MqsQYnIxuJ7o+xRfOvnZGYfldy3lyJVKUkRb858Ow84Bm/i9Npf8v9NNOXLvjcaggQ rNWEDopM2Yu5M32rhbDVheA5mEdUmoNjBVFBRXO4QzgUZfwtiv6sfd92DWfMMbrQ704RSiu4 qF2QxLulSwJNSM28HvPh8JwkqxWvg+vqRJ8zYDTb46bO+Fzcr/ecN4AWWZMRNpdWzBHD4ihb 4UPFe0BPeNAoofnp1sOrB++BQi0BOP31DBDm3/50rcg0+QmCAHGwQ0gEMwUsHTPsd74M78SU eC0zKnMzDXDd+tW1inn5InGaB8hu/aMXattccrQ10YvDRjFg06LqYzmPzKV0PoCs3SB4+V7S +2ikmgqoBx+rTaz3MkjkJXJhp4LxVDe8yV02Ig4KMC5RUN0fNKqEJteuSGeOoZ1X88vXn9kt ig1xLAYt5C3YCoHxZo6yhPeZPGJfYeG7wz9WOuQPDt1i25odbO5ih2v8kag0vXxWtSo3FtOt CZJj8fAu3MX2xDO5MWKReFx80O81TuJygvd8PtLIVoumqreM5Mhx7kwmYcNvknbBS/2nVn2j LeRdkU55uik8+Tnbavipp+bL4J0jxzxPr4umsy4BOQ3LBACX2md+euiyL3u5VD1TKlOg/Esj 6XVrpPXKd4GqqO3DAJZyIIu5wunAzejytsYnH0HLFxfeBKAiojkI0nOIPD5Dfe7glSsiC9ry O7cMrzvGJrNNH/DkK78fbZ89UFc0hEzwMtE55JXCrABJuz8WlPruNPDEBA1Kwq0zP3/B9Vny oweQX6PArOeMK7Kr1OE/vgvLPWUZI8JpDb9LOAo5/HzgnAigFMdZbOm3YcLZ3C4APtmOF6UY WHrgtccC2cFohQxTeLwh12YTzFffXGyX7gz5j0jEoKpEZ/DRpyxgLyGxCq7Bp1WZmFCClCNC Xfob5uLV+0CaS2IOM9hlSUEVaWgS4A/zxGurxT3y6FkLuvU/C0Xq47j2MJu6OLNxlkO8ml7A 87Yy2iRRGF5hUsTQCIs161gqFZwjFCZ3vtWmftdQORT5vdISh83faHV1etjFtH/QBmJKs+JS VKnWs2rRy44VNUo3t4DeV1VANaziB3fmSCnVexG34eXDYA5p/qPl0P6INxwni6u6Q== IronPort-Data: A9a23:PiO58qskJEwdRserykRlEXBIwufnVB1fMUV32f8akzHdYApBsoF/q tZmKWDQbqrbZGr9f4hxO9609UIO7ZOBxt8yGwA6pCk0FC9HgMeUXt7xwmUcn8+xwmwvaGo9s q3yv/GZdJhcokf0/0vraP64xZVF/fngbqLmD+LZMTxGSwZhSSMw4TpugOdRbrRA2LBVOCvQ/ 4KsyyHjEAX9gWQtaDhKs/jrRC5H5ZwehhtJ5jTSWtgW5Dcyp1FNZLoDKKe4KWfPQ4U8NoZWk M6akdlVVkuAl/scIovNfoTTKyXmcZaOVeS6sUe6boD56vR0SoPe5Y5gXBYUQR8/ZzxkBLmdw v0V3XC7YV9B0qEhBI3xXjEAexySM5Gq95fdIX7vr/eS4HTpYmX9//5iIEscYL8hr7Mf7WFmr ZT0KRgIYlaDgOe7qF65YrA014J6dpmtZdhD/CA5pd3aJa9OrZTrW6XL4d5AxDp2mclUGu/DZ scDchJ0bwXGbgEJMFp/5JcWx7b23yWlLWEwRFS9oJgsulCCyBZNi6nLMsGMYc2oT+lPgRPNz o7B1yGjav0AD/SQxTDA6nuwje/ChgvgX58IH7Cn/+RnxlqJyQQ7EB0XUVqjufT/lUekXMtCK kEO4QI1rrk0+VDtRNSVYvGjiCfc71hFAYoWSrZmrlDUokbJ3zuk6qE/ZmYpQLQbWAUeH1TGC nfhcxjV6fCDfVFbpb9xNltZkN9qBRUoEA== IronPort-HdrOrdr: A9a23:ap+kjatqYhYRKaRf0Kr11JTS7skDdNV00zEX/kB9WHVpm6uj5q WTdZUguiMc7Qx7ZJhOo6HlBED+ex3hHPJOgLX5RI3OYOC+ggeVxeJZnOzfKl/bak/DH4dmvM 9dmsNFeb7N5DZB7foTPmKDeeod/A== X-IronPort-Anti-Spam-Filtered: true Received: from unknown (HELO md.home) ([109.128.134.91]) by relay.proximus.be with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Dec 2022 18:08:30 +0100 To: gdb-patches@sourceware.org Cc: Philippe Waroquiers 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 Message-Id: <20221226170823.2744157-1-philippe.waroquiers@skynet.be> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Philippe Waroquiers via Gdb-patches From: Philippe Waroquiers Reply-To: Philippe Waroquiers Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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" \