From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95250 invoked by alias); 18 Oct 2016 15:50:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 95222 invoked by uid 89); 18 Oct 2016 15:50:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 spammy=1,9, messy, lionel, Lionel X-HELO: sessmg23.ericsson.net Received: from sessmg23.ericsson.net (HELO sessmg23.ericsson.net) (193.180.251.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Oct 2016 15:50:18 +0000 Received: from ESESSHC008.ericsson.se (Unknown_Domain [153.88.183.42]) by (Symantec Mail Security) with SMTP id BF.78.02551.7B446085; Tue, 18 Oct 2016 17:50:16 +0200 (CEST) Received: from EUR01-VE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.42) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 18 Oct 2016 17:49:19 +0200 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [142.133.110.144] (192.75.88.130) by DBXPR07MB400.eurprd07.prod.outlook.com (10.141.14.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.649.16; Tue, 18 Oct 2016 15:49:17 +0000 Subject: Re: Check for truncated registers in process_g_packet To: Lionel Flandrin , References: <20161018111023.4hzeyfzzpaneyfds@localhost.localdomain> From: Simon Marchi Message-ID: <33a1f569-995b-342a-dbb9-ea14ab377d1a@ericsson.com> Date: Tue, 18 Oct 2016 15:50:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20161018111023.4hzeyfzzpaneyfds@localhost.localdomain> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: DM5PR03CA0010.namprd03.prod.outlook.com (10.175.104.20) To DBXPR07MB400.eurprd07.prod.outlook.com (10.141.14.152) X-MS-Office365-Filtering-Correlation-Id: 57c99f27-8997-4bf5-487d-08d3f76e5206 X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB400;2:tcGgS25L5rqZCYVhLw0h0gGmtKRaNkME8Q7p8K59q93WgNnDPnHTqlULYKP6TBw9FXLoYUEdRd8hgOMER7joFNbuOWFFkK8nflWXwvTucnUnyPJCku51+FUAVFjNy2jhzKrXDEd4jGt6KGNkPMdmlN5V86eQpQOSar/1YxLmT3QzKj9X5+w+uoMkv54LUqPS3lv9YwSILFSVjV19MK+lvg==;3:RhZS3B55JNqnspvJ/Y9Dm1UhNZDuXmkxnS0Sqfa2yrMQh7/3fmtKfbEaeIsjVJuKwy2NpYK8sqLArxA5+g7wlxOR15zB009JRFXwMYbvfCH7u9oVUO+axgu8yNsK/TVj9LXu3p7lW+ZmxQcIC/hUig== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DBXPR07MB400; X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB400;25:g2gjPLhN+n/6RUtTXMNEgXlDbbINPplzSPZZsgjNCQmNLEoDmR6LIpaO93R2ppsn9Uc+ACicOKiQQDO/CfgwbCI9u3/Ex1YHB/6irNpWm127WLkiSEGKouvycie+Vnj5YHuN9xYCklmb7RN7tqmgwuKqWxK9T6ZIuZSd2IbyBeX4O9oLHO6ngCYzTVWBh60zLLLJR2L+efdFN3j+x4rC4kiAV449lxy5TWe/B1w0hCUYqONVIFAYV7RdZr+z//IZ+FV6127SGW/3IkT0OB4MDUy9BChx1Tc4NauB7Ik+tcRIXP2gBCmxOBQizokk6E6TNXi9HFApSVt9WjRlUnCyYXmiAJ5NUZDL0v2H7hsKSs0/4jNDT3D66BzY2fpBwEEvbCm0iMYKA5ql2DSrlLUyEwfCohKbNoZyF3HVmd3yEeudaBuksNHPOSc7MgZCOi3Y3ERSQDJUEi+l9jUDOCWsGVp0NAP4Aofbua8RsSuqS6yNetaolBGKHDEtoowWwH6NSNSXUeeWIgF7uAFvqrexphayV93q850AaCuGmZ/M1JNUbqVy0p9tYZ1N6SYODg8HW3F8vvQ8R2fS7LMw8O3iMK3ATFFzM7FgRgKCAPMKaNolSMPR73MgnjENdBt9zwAUBmkuXA1a3F5ivRaaknDl1Gy0zFxXHsE+3qwTMnSrR6dqo8N1zUa+2LQmNmfZTK+hciV0ls6j3G0VMCjE7lq7dbsOGjClsuG0OBrF/ex0rOomGYOaH/3AudDaNgHgOOA3 X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB400;31:tMPoIyQHbp7aiT2t/rYJ+LUeTdM6qAvMcLD+nzbl+Du2JqnForpUUAksf8qCsXajvjOwREKag/mlH+9PYzeogQCf/TD3gaHwLHMVTsJiiclf/UxpJY3Te5vH5sPg/wZhHX/d9czBniWUgwzh4VokFJ9ABoJaxrZCraktUWd+uLVQ2iv34FFnWBvNxtr+okJjSb7N2we1Y06TTm5B+8orfFpNghPCM9hHcoRxHoV/a4rxpD9ydseFQf1DOe6X6jKH;20:Xwj9qq4Jt6J4HB9kZysIkf4f00i+xdTOh+OXYOSqAnIWcoM87uhJAY8BFDiBHbTGQnnmUvJOhHv991NwvYXNfVJmmmMPBLHcK6JX+fMXvmXYU6KGgo5nmkFZg8cb3Y+HumrEV1GK4nrMZdUSHLVpXBMrcTYCN698p8GMJCatfoqjRjd9xqSR0616wRyUr5RJo6HUZvbvZD+v3/7TR0miSaPePEhAVZygJvgtOzltXcexb1cNquoMkOzVNAF5uiFg/JGTmXaeIhqC/5ng+tJEiN4ZjUpOvO4ULNJYugQad4YlwQOL/vGCS9PAmSRYW2d4BsK7gKLmnHH/+C9aJLZwnx/PWJcjWWoAexgSxAMil4/73InWZvH4smpzemkvszjEzpzcGIssjh5Jfos/lIF0dSyzHSWQ2sjyRGbRImDZfMI2xBgkebHuw/WSt06CRstIRwG7fg78fIf3i8ujW05e92PfqYzMH19ISnvbYy6tGb9JDiooKHag/ZvyBUwr0KaY X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:DBXPR07MB400;BCL:0;PCL:0;RULEID:;SRVR:DBXPR07MB400; X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB400;4:Ysm/+nGSuf9h2aNdZgGeQqQHiDUpnURtH8tzpwm7m9gkmDXl9DHDNu5cpupN4BQmV9+jCos7cuDvordMy9tAMciUCA7/mbXXfn51CbrF20ZbEXX1QuEjnB4Ltm4Evp86e8yBE15THZpGoGwpAlOagEreTs2bllbNWFVH8yajefR5rTzXVDRvayoueYlf1eaC1So3YEknLzzCjfpvEtUebJrCzQXtKZbAMuWEhM2xfbdGg2zB2pSNKSxr7H71X2x+I0dRcYt3lyip/jYpbX6n/z5YxmQ7vjo07jE49T3hqaPX2ZNt0szqpYVHMEDh/F+NUwylYX9XCnRSlIquC1ezKc02z58bIOWQ7o0UK+80bDMeWyNcfRG520HfMB0yBm8Ihg/a7vycNlbfs2ILnZUyLQ== X-Forefront-PRVS: 00997889E7 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(979002)(6009001)(6049001)(7916002)(377454003)(199003)(54534003)(24454002)(377424004)(189002)(5660300001)(4001350100001)(7846002)(97736004)(3846002)(65956001)(189998001)(31686004)(6116002)(83506001)(64126003)(31696002)(586003)(68736007)(50466002)(36756003)(92566002)(47776003)(106356001)(305945005)(66066001)(50986999)(105586002)(2950100002)(81166006)(33646002)(4001150100001)(65806001)(6666003)(5001770100001)(77096005)(107886002)(8676002)(2906002)(65826007)(7736002)(19580405001)(230700001)(101416001)(86362001)(19580395003)(23746002)(81156014)(76176999)(42186005)(54356999)(969003)(989001)(999001)(1009001)(1019001);DIR:OUT;SFP:1101;SCL:1;SRVR:DBXPR07MB400;H:[142.133.110.144];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;DBXPR07MB400;23:cWXR0KuoNa6XIBk5/X/KLETQJJ0WQTIxPTdub8?= =?Windows-1252?Q?qFU+Eub1kGGaSpslPW+4ZykqFdxrYm3KBu/JSMe/sAhBQlCc92vxABco?= =?Windows-1252?Q?W6XTccw76X66tuIhBcTsmNdE4i7jTvE23vSX1JMVPRt9DOJuu5WJj0DH?= =?Windows-1252?Q?zwEG1Cc3LrLlqC8UOi+EENWzbN1QWnbMMLEzAP7p2ZteXbhtUzeaRgmr?= =?Windows-1252?Q?EA+Fz7K7yd6Ffh4aIsqh5pCcfRR5rVH8NNN32e/jdqD9ZWRyvWir3gzu?= =?Windows-1252?Q?3TKc0rNmRPb1gS7AZAfW8Y4h98d+kXHHBb+XzB8psbjv9vER2gbN3DSg?= =?Windows-1252?Q?lTKz5zszfqt0bJnXfZj7nI/qLg2E0/L5tsMTkXSBUQrIEtQkiaFDZ7Q5?= =?Windows-1252?Q?LSOV63CMYn3pojBWXo0Juq+Yr1Ztplfoo0n7qT9/paZ/i0w5c4erC49F?= =?Windows-1252?Q?Ookj9oQV9QPxF3PmtzKrkQQO3eJ3uXFN68SprBKBQEfh/GtGjJmHhRPn?= =?Windows-1252?Q?JyTd3b+SPnqiN4xtoV+bVH9Cml4GtcbfAbqMlFi2DVq0t8nZXoZBD+B2?= =?Windows-1252?Q?MI9yh5XLgXLsZ2uy6nuqrr2HW4jhjGXNpAjjG2RhHQbosWmy6xekYWYj?= =?Windows-1252?Q?OiLJjHc2cBYhIJPVlQfYN6YOy7rBl7UI4oauLDMm2yTFpYZn3SAhb3ue?= =?Windows-1252?Q?qWnKqOMYClOY/Ru6CRyVs3AKIc20X/4eSdb8A4K450O2NSeg7vrWxNmM?= =?Windows-1252?Q?tYte86zbpX6w9wAscF0oikI8d+eRKpjnGwZZZA9Ycfkp1NgqmZ8kOOse?= =?Windows-1252?Q?x2Mi+5qm2J3Ul1jGovOG4eMxBUQpBhR8Xq8y995DhcF1cDtzIjKKe+Ec?= =?Windows-1252?Q?c8h0nuNp3hRmvIOUgBoJ+OxE0TXKIGLQ5hz2sNcKWX831UUZTrgIlMAk?= =?Windows-1252?Q?/4YMYdxuFTfk9oG38ssoS6BDDp7NfEfeOsTZnoDej3JU5toGCKInfVTS?= =?Windows-1252?Q?w101LI4MRQ4BOc1tVUHYsgIWeiLBWWnVaggUrLb0+7Xi+rbRfiu++8Wf?= =?Windows-1252?Q?7xc0PVtigrmIS5RDfT8Bg066VSE/On+VcWUsTA9VycSCEnefhNcWmbJI?= =?Windows-1252?Q?IkDGVxpM66EraWV1d9PHdqu7drIrIW4QyLmJqngkPecFpLbKvOuaZwRv?= =?Windows-1252?Q?wm1YYSBFafQ9xPr0G9e+zHFbOXFZIjoFSFDYVLjJHyR5a+q9PSMY5PIr?= =?Windows-1252?Q?z9nCzxroCXl2yQfUOuHzXsmxZ1OaYSFzQMYjx1IDkEBPxFK9OyGM4HVj?= =?Windows-1252?Q?2C2flnZM6QC58xS/9Jo8lHXfno09jPNkq2Y9/UK+/DEghdrsVZsmepst?= =?Windows-1252?Q?Oi3GbcESdtqJ5NgDczhpXuCqmN4qeu4gbdnR1AmWjPJ6eN6vX+F+ZDBM?= =?Windows-1252?Q?al6jJllH8o9/JXosz27nOvRsgI2m1OY4B2uaqxIbthsPIk3SVeOnTmpL?= =?Windows-1252?Q?v04PjuAlz5B+b8MZNHrPQLBHDu+rzJ6EksATkMAwFYBZABnVdd6V933o?= =?Windows-1252?Q?fHeRY4JLdrGycOApf+xiDi+tnr/xAt41JivFNzXQl+taCP30fUhVlBzf?= =?Windows-1252?Q?WSMA4SPgtlZ84h1FjQPwk=3D?= X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB400;6:vjJCcM/jgZbENq/OXjFNsfwWtCL7ThH+03uF0dtW2Y9q1nD/LeP5VY1o9pma5sMBGNw8aaZH3bwx04r1BhS65Xj1oZHIvMqkyOTEXncES3Qrfpvzj/18hQa6SJ5DOdOPEO15EW5bI5BTq/pWvlshXz7hm/R3avNdw/R4Kpzgf38HdmAP+IF2PcXu/pMIluAuasdsfsiHXEs9nhKW/GBXWnS/uehJKnjE7JRag/Gk2tsdD7Vk68n2fkwyOHC1JT+vxUJa1R+LT0YirPzxk3KEEJjC/YyCUljRCOJJd8Ao0jwvPXjpEIa0BmwWrsCgAk5a;5:njWSacVy5FMtrDZ2Dqs9pP5jElg5wav6z0DZarShcAGvkuVrPJvxl5/DLW+zfqGif2bY2Bfxt2ERHSGzxfN01wwGBfjitr/LOrPQsCB5XC5lTDr9+fGXtg6NEgn1P4iuCYjL/synUH4EGCYjKeOyThn+sJayumIsVCa/LFXn2yk=;24:AgD03UCMLyXA53VVAALqfe2wMFQmIBBdx54H38IhNMMJ3cMhmkgSThR+dLa6GzYIGeh2qfK8l4CpX66d0vCpLfGMfjFapLJnNq7TFJwVEQw=;7:ZaWNMifPnRGAk2117u8+zl0V+2cbi/p0bcind2du0FqzdneFFuVz9me/K7Jr7ltWTwsgC5UK6OscrS09mdx0EQIw3zkOQ7vBxpyxQhB08DcXZa96XS5B9jOHNCd/2Sohre40VlmUqD7r5pKakVY6LCOoj6caiV5SvnGXcI5KH7wqILin0R+fHxXD02zndZna+7oBDckX9M5RsZvciaS4tG51kgxJ+wGQ1BEUCpS2nt61n6el63eeEs69jKct34Pcx7nO0LbPIZK97O7DIJ0VXnT1rcrlXTulgH1GWbHsDHWCLnox647PFadJT07Jhuq1VwyYqDY0uxeye6ZzHB7JNOXzV1xyay0dl8VOtSk84ok= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2016 15:49:17.8414 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBXPR07MB400 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00513.txt.bz2 On 16-10-18 07:10 AM, Lionel Flandrin wrote: > Hello, > > While investigating an unrelated issue in remote.c I noticed that the > bound checking for 'g' packets was bogus: > > The previous code would only check that the first byte of the register > was within bounds before passing the buffer to regcache_raw_supply. If > it turned out that the register in the 'g' packet was incomplete then > regcache_raw_supply would proceed to memcpy out-of-bounds. > > Since the buffer is allocated with alloca it's relatively unlikely to > crash (you just end up dumping gdb's stack into the cache) but it's > still a bit messy. > > I changed this logic to check for truncated registers and raise an > error if one is encountered. Hopefully it should make debugging remote > stubs a bit easier. Hi Lionel, This patch looks good to me, a few minor comments below about formatting. Someone else with the approval stamp must look at it, but hopefully it will save them a bit of work. > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 4b642b8..73b9b9e 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,9 @@ > +2016-10-18 Lionel Flandrin Missing space between name and email. > + > + * remote.c (process_g_packet): Detect truncated registers in 'g' > + packets and raise an error. Fixes a potential out-of-bounds buffer > + access if the remote sent a truncated 'g' packet. > + The ChangeLog should only contain "what has changed", and not the "why". In your case, I think the first sentence would be sufficient: + * remote.c (process_g_packet): Detect truncated registers in 'g' + packets and raise an error. If somebody wants to know why that was changed, they would go look at the commit message, where you explained the issue in details. It's easy to go from the ChangeLog entry to the commit using git blame. > @@ -7163,18 +7163,31 @@ process_g_packet (struct regcache *regcache) > the 'p' packet must be used. */ > if (buf_len < 2 * rsa->sizeof_g_packet) > { > - rsa->sizeof_g_packet = buf_len / 2; > + long sizeof_g_packet = buf_len / 2; > > for (i = 0; i < gdbarch_num_regs (gdbarch); i++) > { > + long offset = rsa->regs[i].offset; > + long reg_size = register_size(gdbarch, i); Missing space after "register_size". > + /* Looks valid enough, we can assume this is the correct length > + for a 'g' packet. It's important not to adjust > + rsa.sizeof_g_packet if we have truncated registers otherwise rsa->sizeof_g_packet ? > + this "if" won't be run the next time the method is called > + with a packet of the same size and one of the internal errors > + below will trigger instead. */ Use two spaces after each period (including the final one, before the */. > + rsa->sizeof_g_packet = sizeof_g_packet; > } > > regs = (char *) alloca (rsa->sizeof_g_packet); > @@ -7204,10 +7217,11 @@ process_g_packet (struct regcache *regcache) > for (i = 0; i < gdbarch_num_regs (gdbarch); i++) > { > struct packet_reg *r = &rsa->regs[i]; > + long reg_size = register_size(gdbarch, i); Space after register_size. Thanks! Simon