From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailsec212.isp.belgacom.be (mailsec212.isp.belgacom.be [195.238.22.108]) by sourceware.org (Postfix) with ESMTPS id D81AC3858D38 for ; Mon, 9 Jan 2023 23:37:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D81AC3858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=skynet.be Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=skynet.be DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=rmail; t=1673307477; x=1704843477; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=X6QdG4vCnYl5af+0zj0tw/hHHDglMu1KCiatw4Z/ffw=; b=eL01uowDIP7+Nbf2pHDURD/xlzKGshTNtagYnZyYnPmwnSP/ntMo0oM4 /XRSJPcjGzGpB1KBVo8Nas9Z+Rda1fUzw8CArbDoQFbYgUkMSeb4MOgtq zFiaxASR17FLsfzT9lnXtFzLzZsdN56iXw83f5awbIo5LKfM4kOriu/h8 4=; X-ExtLoop: 1 X-IPAS-Result: =?us-ascii?q?A2AQAQD6pLxj/1uGgG0NTYEJCYFGggWBL4FYAoRMkR8Dn?= =?us-ascii?q?TmBfg8BAQEBAQEBAQEJMRMEAQGFBQKFFCY0CQ4BAgQBAQEBAwIDAQEBAQEBA?= =?us-ascii?q?wEBBgEBAQEBAQYEAYEchS85gkUig30BAQEDIwQLAVYJAhgCAiYCAleCP1mYK?= =?us-ascii?q?5sden8zgQGEc5plgWeBFCyJDYgUN4FVRIEVgTyBPjA+iBuCZwSBCYtujQqBP?= =?us-ascii?q?X2BJw6BTRYcNwMZKx1AAwttCkA1CwtLKxobB4EKKigVAwQEAwIGEwMgAg0oM?= =?us-ascii?q?RQEKRMNJyZrCQIDImEFAwMEKC0JIR8HFREkPAdWNwEEAwIPHzcGAwkDAh9Pc?= =?us-ascii?q?i4REwUDCxUqRwQINgUGHDYSAggPEg8GJkMOQjc2EwaBBwsOEwNQgU8EL4FdC?= =?us-ascii?q?gZRm2GBLlIgMUoTARMvgQklk2ABjlqfJoIBNAeDcoFKBgyJeZUNMoN5k0KRa?= =?us-ascii?q?pZUdagCgWKCFW1TgmcJSSiOKQMWFYkghHp0CDMCBwEKAQEDCYwjAQE?= IronPort-PHdr: A9a23:3a4BfRO7Zgwl46T4jgYl6nbHDRdPi9zP1u491JMrhvp0f7i5+Ny6Z QqDv6gr3ACCBNyTwskHotSVmpioYXYH75eFvSJKW713fDhBt/8rmRc9CtWOE0zxIa2iRSU7G MNfSA0tpCnjYgBaF8nkelLdvGC54yIMFRXjLwp1Ifn+FpLPg8it2O2+5Z3ebx9LiTe/br9+M Qi6phjNu8cLhodvNqk+xhzVr3VSZu9Y33loJVWdnxb94se/4ptu+DlOtvwi6sBNT7z0c7w3Q rJEAjsmNXs15NDwuhnYUQSP/HocXX4InRdOHgPI8Qv1Xpb1siv9q+p9xCyXNtD4QLwoRTiv6 bpgRRn1gykFKjE56nnahM9tgqxbvhyvqR5ww4DIb46bO/VwcbjQfc8ZSGdbQspdSzZMD4G6Y oASD+QBJ+FYr4zlqlUUrRu+BA2sBOL3yj9QmHD9wK013P47EQHBxQwgAtMOv2rOrNXuKawfV vi1zKnJzTXHbvNWwS/955bSfhEvu/6MR7VwcdPMyUkuCQzFlE6QpJf+PzOW1uUNtXaU7/Z6W e20lWEqsgd8qSWgyckwkIfGnJ4Vykza+iVjxoY4Pdy1RU51b9CrDpddtS6XO5VqTs0tQWxlu CY3xqAYtJKmfiUH1okrywDCZ/GZc4WF/g7uWeefLDtmin9pZrayiwq0/EO9yeP8TtG53EtWo idBiNXAq24B2h/J5sSaRPZw8V2t1DKS2w3V9+pKO1o7lbDBJJ4k2rMwk50TvlndESLug0X2i bOWdl0j+ui19+TrerXmqYGYN49zkgzxNrkil8ihDegiLwQDXmuW9f6h2LH54EH0RLNFguU3k qnfrp/aOdwWqrOlDwJRyIov9gizAjS83NgFk3QKI0hJdRaag4TxPlHBOvH4DfOxg1S2lzdrw ujLPqXhApXMKnjDka/hfa5j5EJGxgs818pf55VNCrEcIfL8RFXxtMfDAx8iMAy1w+DnB8th1 o8EQm2AHLeVMKXJvlCQ/OIgP/GMZJMJuDb6M/Up+ubijWUlll8FYampwZwXZWi3Hvt7OEqVe GLsjc0dEWgWvgoxUvfqhUaZUT5UenayRb4z6S81CY28F4fMWJqhgLub3Ce0TdVqYTVJBlrJC X70fIWJQN8Xbz+IKcJ+myYJE7+7RNwPzxar4Tf6yr5mNvLZshIRr5X6ydl4/fabwQky9Dh1F 9yQlX6EVWZtg2IFXSQex6Nup0FhjF2Oh/sry8dEHMBesqsaGjwxMoTRmrQSNg== IronPort-Data: A9a23:kr54Ba2FKoh+12QM/PbD5Zlwkn2cJEfYwER7XKvMYLTBsI5bpzUAy mIdWWvVOPuNYDH3f9p2bdjk80NTu5DUmtJqGQVk3Hw8FHgiRegppTi6wuYcGwvIc6UvmWo+t 512huHodZxyFDmFzvuUGuCJhWFm0q2VTabLBufBOyRgLSdpUy5JZShLwobVuaY1x4nna++xk Ymq+ZaHYQf0g2Ic3l88sspvljs+5JwehxtF5jTSVdgT1HfCmn8cCo4oJK3ZBxMUlaENQ4ZW7 86apF2I1juxEyUFU7tJoZ6nGqE+eYM+CCDV4pZgtwdOtTAZzsA6+v5T2PPx8i67gR3R9zx64 I0lWZBd1W7FM4WU8NnxXSW0HAlXAqJW3e7OLUSxnvSC80zYUXjW+dZxWRRe0Y0woo6bAEl9r K1CbmlTPlbawbvn9dpXSME13pVldZO7etpE5jcxlFk1Dt5/KXzHa73K5NtZxC89wN9HB/HHe ssUcyFHdxfRZRBTfF0aYH47tLz32COnI2QDwL6TjYcs/UOU5VVj6bPSa9f7J9C4FMhtmX/N8 woq+Ey8WHn2Lue3xjaPt2qlmuTPkDjTQ4UPDra16fdwjRuU3GN7NfENfQri56Di1gvnAYIac hRFvCMitqx3+F2tCNjmR1uyrXqLtxUbXpxaHoXW9T2w90Yd2C7BbkBsc9KLQIFOWBMeLdDh6 rNFcx4Fy9CiXH15hE9xLoupkA4= IronPort-HdrOrdr: A9a23:COVAraNGv7/+K8BcTtSjsMiBIKoaSvp037BN7TEVdfU1SKylfq +V88jzuSWYtN9VYh8dcLO7Scu9qBHnnqKdiLN5VYtKHjOGhILCFutfBOXZrwEIVxeOldJg6Q == X-IronPort-Anti-Spam-Filtered: true Received: from 91.134-128-109.adsl-dyn.isp.belgacom.be (HELO [192.168.1.19]) ([109.128.134.91]) by relay.proximus.be with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Jan 2023 00:37:48 +0100 Message-ID: <23959381458f70e6c7fa2a4a1880bdba7e39b0cd.camel@skynet.be> Subject: Re: [RFA] Fix use after free caused by redefining a python prefix cmd that has aliases. From: Philippe Waroquiers To: gdb-patches@sourceware.org Date: Tue, 10 Jan 2023 00:37:47 +0100 In-Reply-To: <20221226170823.2744157-1-philippe.waroquiers@skynet.be> References: <20221226170823.2744157-1-philippe.waroquiers@skynet.be> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1+deb11u1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,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 List-Id: Ping ? Thanks Philippe On Mon, 2022-12-26 at 18:08 +0100, Philippe Waroquiers wrote: > 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" \