From patchwork Sun Nov 20 10:44:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 60892 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 0E565395A065 for ; Sun, 20 Nov 2022 10:44:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0E565395A065 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668941082; bh=OAIlYmABy8JQzzA4AdbySuL4EuYAiNxUmnETehAG+Jk=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=AJrufbSaLjiJcHoHbZeZQrtExMGdnaG1FE+RwXHb/BmpNMnWJHl3uoj+zFRONvsNQ gejtbKV0Pbcdku45xf4k3EZYQX4NMF0pu4iwTCnAz29b3VicB6poJGlzjc1eF8UJXb Rc2K0uLnWuirLrKNQDRMXqxvWP7jFP3gmvVrfCUQ= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mailsec111.isp.belgacom.be (mailsec111.isp.belgacom.be [195.238.20.107]) by sourceware.org (Postfix) with ESMTPS id 19B1C395A058 for ; Sun, 20 Nov 2022 10:44:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 19B1C395A058 X-ExtLoop: 1 X-IPAS-Result: A2DWBADqA3pj/1uGgG1agRKcN58yDwEBAQEBAQEBAQlEBAEBhQWFBCY4EwECBAEBAQEDAgMBAQEBAQEDAQEGAQEBAQEBBgQBgRuFL0aCNSKDfysLAUaEToMjsz0zgQGEcJpmgWeBQItOhXmBVUSBFYJ6i3AEmDADCQMHBUlAAwsYDRYyChM3G1gOCR8cJQ0FBhIDIGwFBzoPKC9nEBscGweBDCooFQMEBAMCBhMDIgINKTEUBCkTDSsnbwkCAyFqAwMEKCwDCSEfBycmPAdWOgUDAg8gOAYDCQMCIhNCdC8SFAUDCxUlCAVLBAg5BQZTEgIKEQMSDyxFDkg+ORYGJ0IBMQ4OFANeSx2BAQSCJgqbBIEBBy0EKDUUgRMKFYEukjEur1Q0B4NrgUcGDJ5hGjKpFQEtlhJ1oyeEUYF5gX5tgztRGQ+OLBaOL4EvAgcLAQEDCYpiAQE IronPort-PHdr: A9a23:WXhzVRw98H7jYPLXCzJHylBlVkEcU1XcAAcZ59Idhq5Udez7ptK+Z hCZuK4m1QOUFcWDsrQY0bGQ6/ihEUU7or+5+EgYd5JNUxJXwe43pCcHRPC/NEvgMfTxZDY7F skRHHVs/nW8LFQHUJ2mPw6arXK99yMdFQviPgRpOOv1BpTSj8Oq3Oyu5pHfeQpFiCSybL9oL Bi7owrdutQZjIZiN609zgfFrmZSd+lZ229lK0ifkwrg6su14ZVu7zlet/U9+sBaTK70Zb44T btWDDQnN2A6+sjmvgTdQAWM+3URTHwYngJHDAbZ4h76WIzxsjbhuepmxCaaJ8z2QqsqVjmk8 qxmVQXniCYDNz4+7WHXlsl9h79VrR69uxByxZPfbYeIP/R8Y6zdZ8sXS2pfUMtPSiJPDICyY YwAAOQOJutUs4rwqkESoRakGQWgGOXiwSJIiH/s2q061vwsHxvG3AwhG9IOsWzUrM3rO6wPU e+61rPIzTLab/NL2Dfy9pLIcgs8qvyLRbJwccvRyU0uFwPdllWft5bpPj2P2eQXtGib9vdgV eOxhG49sAF8uSOvxsQsi4nPmI0V1krI+j5nz4ssI9CzVUF0b8K+HpRKqyGaK5V5QtkkQ2xwt is317wLtJGmcCYKyJopxxrSZv+af4WV/B7tWumfLCl2in55er+yiQu//Emvx+PyWce5zkhGo ytYntTDqn0A1hre4dWERPtl5kqtxCqD2gTJ5u1ZP0w5lrDXJ4Mvz7M/jJYetVnPEynrk0vsl qCWbF8r+u2w5uTiZbXpu4GTOpdvigH7LqQugsu/AfkkMgQWX2iU5+C81Lr78E38XbpGlP02k q7csJ/EPcgbp6i5DBFJ0os79hqzEzOr3M4FkXUZL19JYg+LgobmNl3UJP30EO+zg1G2nzdqw /DGMKfhApLILnXbiLfhfbd960pdyAor1dBQ+YhYC78bL/LpXU/xrcHYDh4nPAyu2ObqE8591 oAeWGKJHKCZLLnevkSW6e43JemDf5cauCzhJPg9+/7ukXg5lEcDcaWxx5sYdGi4Huh6I0Wee XfsjcoOHnwTsgomVuPqlEGNUT5NaHapRK88/TY7CJ+8DYjfWI+sjqaO3D2lEZFMYWBGEF+MH W/yd4qYQ/cMdD6SIsh5nzwcVbihSosh1RC2tA/i1bVrNOTV9TcCtZLkzdh1+uzTmg8o9TxvF MmdyGKNTmFynmwWWz86xrtwrlIugmuEhKd0iblAHMBY5/5Sehw9KITXwvNzEd20XRjOLfmTT 1PzetWnBTApVt95/NYUZF9gGti4lViXxyqrB74Nj7HNG5Uu9bvB3nXrPO5myGfA2bVnhVRwE ZgHDnGvmqMqr1ubPIXOiUjMz86X IronPort-Data: A9a23:YuQLxKmM4IkaCHj9+dkUJfvo5gzsJ0RdPkR7XQ2eYbSJt1+Wr1Gzt xIfCGqHOv2CN2Dyf4hyaN6xpEsHvcLQzINjSlE9/yg0RVtH+JHPbTi7wuccHM8zwunrFh8PA xA2M4GYRCwMZiaA4E3ra9ANlFEkvYmQXL3wFeXYDS54QA5gWU8JhAlq8wIDqtcAbeORXkXc5 7sen+WFYAX/gmcuajpNg06+gEoHUMra6WtwUmMWOKgjUG/2zxE9EJ8ZLKetGHr0KqE88jmSH rurIBmRpws1zj91Yj+Xuu+Tnn4iHtY+CTOzZk9+AMBOtPTtShsaic7XPNJEAateZq7gc9pZk L2hvrToIesl0zGldOk1C3Fl/y9C0aJu6bKWDyCukdWo6lDqUnrz4v53IHEMIthNkgp3KTkmG f0wLTxLbheGiopawpriErgq355zapCwYMVO4xmMzhmAZRoiaYjDQqHL/cdVmig5nMdXAPfTf dExcjl+ahncJRdCUrsSIMtjzLvx3ymlLFW0rnq6n5YK/3jewDUv7+fMGvjuRoaQGth8yxPwS mXuuj6R7gshHNOTw3+d+26nhuLUtTj8RZgZGaKx7PMsh0ecrkQLCBwSVEOjrL+mg1S5Qs9eJ lYP0jEtvK4/6AqhQ7HAswaQ8SfC50VAHoMKQ6hjsFDLw6bP50OVF25CTyVZLtYrsMA/Tjsvk FWE9z/0OQFSXHSuYSr13t+pQfmaY0D58UdqieQ4ocfpLjUtTEzfTv4Cczq7LJOIsw== IronPort-HdrOrdr: A9a23:7WnRvav8W1cOwoQtB1MnetHX7skDSdV00zEX/kB9WHVpm62j5q WTdZEgvyMc5wxwZJhNo7+90cq7MBHhHPxOgLX5VI3KNGLbUQCTXeJfBOXZrQHIKmne+ulika Zte6VzE7TLYGRSvILa7A6HV9ctyNSK65qUo9629RtQcT0= X-IronPort-Anti-Spam-Filtered: true X-ProximusIPWarmup: true Received: from 91.134-128-109.adsl-dyn.isp.belgacom.be (HELO md.home) ([109.128.134.91]) by relay.proximus.be with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Nov 2022 11:44:14 +0100 To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFA] Fix use after free introduced by $_hit_bpnum/$_hit_locno variables. Date: Sun, 20 Nov 2022 11:44:06 +0100 Message-Id: <20221120104406.2134561-1-philippe.waroquiers@skynet.be> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 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" If the commands of the bpstat bs contain commands such as step or next or continue, the BS and its commands are freed by execute_control_command. So, we cannot remember the BS that was printed. Instead, remember the bpnum and locno. Regtested on debian/amd64 and re-run a few tests under valgrind. --- gdb/breakpoint.c | 62 ++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 7f6400db624..5b691673a0e 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4574,18 +4574,17 @@ command_line_is_silent (struct command_line *cmd) return cmd && (strcmp ("silent", cmd->line) == 0); } -/* Sets the $_hit_bpnum and $_hit_locno to the bpnum and locno of bs. */ +/* Sets the $_hit_bpnum and $_hit_locno to bpnum and locno. + A locno 0 is changed to 1 to e.g. let the user do + (gdb) disable $_hit_bpnum.$_hit_locno + for a single location breakpoint. */ + static void -set_hit_convenience_vars (bpstat *bs) +set_hit_convenience_vars (int bpnum, int locno) { - const struct breakpoint *b = bs->breakpoint_at; - if (b != nullptr) - { - int locno = bpstat_locno (bs); - set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), b->number); - set_internalvar_integer (lookup_internalvar ("_hit_locno"), - (locno > 0 ? locno : 1)); - } + set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), bpnum); + set_internalvar_integer (lookup_internalvar ("_hit_locno"), + (locno > 0 ? locno : 1)); } /* Execute all the commands associated with all the breakpoints at @@ -4602,7 +4601,6 @@ bpstat_do_actions_1 (bpstat **bsp) { bpstat *bs; bool again = false; - bpstat *bs_print_hit_var; /* Avoid endless recursion if a `source' command is contained in bs->commands. */ @@ -4618,29 +4616,39 @@ bpstat_do_actions_1 (bpstat **bsp) bs = *bsp; /* The $_hit_* convenience variables are set before running the - commands of bs. In case we have several bs, after the loop, - we set again the variables to the first bs to print. */ - bs_print_hit_var = nullptr; + commands of BS. In case we have several bs, after the loop, + we set again the variables to the first printed bpnum and locno. + For multiple breakpoints, this ensures the variables are set to the + breakpoint printed for the user. */ + int printed_hit_bpnum = -1; + int printed_hit_locno = -1; breakpoint_proceeded = 0; for (; bs != NULL; bs = bs->next) { struct command_line *cmd = NULL; - /* Set the _hit_* convenience variables before running the commands of - each bs. If this is the first bs to be printed, remember it so as to - set the convenience variable again to this bs after the loop so that in - case of multiple breakpoints, the variables are set to the breakpoint - printed for the user. */ - set_hit_convenience_vars (bs); - if (bs_print_hit_var == nullptr && bs->print) - bs_print_hit_var = bs; + /* Set the _hit_* convenience variables before running BS's commands. */ + { + const struct breakpoint *b = bs->breakpoint_at; + if (b != nullptr) + { + int locno = bpstat_locno (bs); + + set_hit_convenience_vars (b->number, locno); + if (printed_hit_locno == -1 && bs->print) + { + printed_hit_bpnum = b->number; + printed_hit_locno = locno; + } + } + } /* Take ownership of the BSP's command tree, if it has one. The command tree could legitimately contain commands like 'step' and 'next', which call clear_proceed_status, which - frees stop_bpstat's command tree. To make sure this doesn't + frees the bpstat BS and its command tree. To make sure this doesn't free the tree we're executing out from under us, we need to take ownership of the tree ourselves. Since a given bpstat's commands are only executed once, we don't need to copy it; we @@ -4659,6 +4667,8 @@ bpstat_do_actions_1 (bpstat **bsp) while (cmd != NULL) { execute_control_command (cmd); + /* After execute_control_command, if breakpoint_proceeded is true, + BS has been freed and cannot be accessed anymore. */ if (breakpoint_proceeded) break; @@ -4693,9 +4703,9 @@ bpstat_do_actions_1 (bpstat **bsp) } /* Now that we have executed the commands of all bs, set the _hit_* - convenience variables to the printed bs. */ - if (bs_print_hit_var != nullptr) - set_hit_convenience_vars (bs_print_hit_var); + convenience variables to the printed values. */ + if (printed_hit_locno != -1) + set_hit_convenience_vars (printed_hit_bpnum, printed_hit_locno); return again; }