From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10050.outbound.protection.outlook.com [40.107.1.50]) by sourceware.org (Postfix) with ESMTPS id 1390B3858C20 for ; Tue, 16 Aug 2022 06:06:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1390B3858C20 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iHSXxTRTNiugrx0Ip0HsK/PNoDhmFKDNiwlye/Q3aDy6XJsdMXw/ULKpO1Q/oTJTfLd5e4NczYXQedbOA46K7H76Hxnc6mrmZpMGFVJjbFl8e3aTNHj/mC1dBjCyPcDxgJedQxi3B8RLpZxyj16HPktdCSgXJDt/42439Xpjko8dlwi6UZh/wXv395zAsHLoZhugbUyJucQqNaY/+exNs4jrLiAF34Uk2hgmWIKaVRm+ZSrmELFCS0cpKaMuWWqYOsuROR5rOTdJJaP5vLUCOuw3IIM8R9NY0e82el+j2Vuth7v60EvwbUQeHe2lYnKcqjstK1P0qYSmazHbsIWtGg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=XoSdNENuKZ+cSuyff1HCk6rr1MTZPh9JGLmj3EQvH8U=; b=TcChV5QLHCTTMGIODXj7yX9yF8Y6zpdlAAZislimurIU8pS0lrPt0WduyENwUJQTJ2u8G3HTEu/FrmZkwd46UYWMEGZhVdheM4dxD+mCJOnGLJp+4FmxYdVZySY+M1BSKLZ9Pa3YCQKM6LTQgNCu/RxU0O2sOrXMadcXTgJZd3dTmp/efcg1F3IIbDrwkaYIIzSwWXHe8pLvc3mqxVJX/TaDzHhSyU8N+ykJqmRz1bPKTv1UwMNknq04NjV6dSjAY7mgHREf/DQq6H3958/93LJhe6O9av8NMvfZlG6E/SoV1+hQJ5QjF8+nXMJGZuI5if70TMxhaUy08w8HFDjjNg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none Received: from VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) by DB6PR0401MB2581.eurprd04.prod.outlook.com (2603:10a6:4:3a::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.28; Tue, 16 Aug 2022 06:06:23 +0000 Received: from VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::2d5d:bae0:430f:70ad]) by VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::2d5d:bae0:430f:70ad%4]) with mapi id 15.20.5525.010; Tue, 16 Aug 2022 06:06:23 +0000 Message-ID: <02a83431-ad0a-be06-f7bc-7757f80faa7c@suse.com> Date: Tue, 16 Aug 2022 08:06:20 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v2] x86: avoid i386_dis_printf()'s staging area for potentially long strings Content-Language: en-US To: "H.J. Lu" Cc: Binutils , Alan Modra References: <86a8594d-d3f5-a895-75c3-6a751c398b4e@suse.com> From: Jan Beulich In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR3P281CA0169.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a0::8) To VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c201abbb-9fd1-4633-dd23-08da7f4d7230 X-MS-TrafficTypeDiagnostic: DB6PR0401MB2581:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: prridheuFWwUJyJQ/AAad05sL7bXIjDPjkeU3t85B0EPPqCBpwpPXLn6fiGaAyw8VT1M1JWyzAMMeIfiFISDA4byFuYoo1D4XVlCaqScgC0YdADNTBrWnfJ5N9SbToTISyqx3WYgrZejekZj6Abjhh82lsuQ7MIw7BNXWPXVb2FpiDXL8UFnWKLxB5q0IewGmMiPU8FLNYKK2u7Y3/T1PPEfJ90hwXflFk6L+tSVFTULTQru1DLe6jiPSFdY2k4rPepF9bGd4y/0YodKFOyNOtlAnrq1WvKQKPHQiRUkVgUYieNGuzkCB5595gbLgJ0O31N1DcLH6tclJSCEPvQUOznfPGtXhx6/l1+9A/fkk8VbvhXx/xfuUL9o0ejeGFmD6tVURD2gA06IiSXc58gqklcj/c4W9xDPX2gyuVgO9o1NOT8AVApdtZDLq8Ue9gMMwvV4LhWTqiGikjb3WWeL0UXPDGMSlEx01hLM6SzuxfrqeNzPQMNnJ8KxG3GDVTSvbDg7RBqS+bcWI05xINgJgIB8AEHbz79dF0GOOKx8uI6jegdOdGWskAOyqauXcKlWCgu2WWX8uGe4k1EWzGQYNy2VgCAlgDWBjRWDPsVAgj+zgCFnV2yQ8bYAHxV3jZWglqhru+aI9BHpn0D3XZIsur84ceBXtfTmmxOQWAhOXIdLrPIVonwq+RJQMxmYM5J8JUqLWYOxUdCO2WgGTh1PD0+o0VgLJ14oo4V1pWU2bmnkDGT8EMPVnpzUhQAy8hV55XGwxaCR8hWzpfl20B3RsBI8DvJOW++DvfBVWEOR5MkJZ6ttieWp9Ik1Xgyr+GUiGbLbsbjCbXgpBrpONMfwxQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VE1PR04MB6560.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230016)(396003)(376002)(136003)(39860400002)(346002)(366004)(4326008)(316002)(66476007)(66556008)(6916009)(54906003)(8676002)(66946007)(5660300002)(2906002)(8936002)(38100700002)(31696002)(36756003)(86362001)(478600001)(31686004)(6506007)(53546011)(6486002)(41300700001)(83380400001)(26005)(2616005)(6512007)(186003)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NVVYamhJaGJhNGI1ZnpvY0VGMGJMd1hVWVVNN2xBZ2RibmpDZVkwekNFUmhY?= =?utf-8?B?ZmJoOVhpRUdBMTBFdVRQbDFIQnltOCtZQy82OTlmUWtjZGRvN0tLQmF6ZW56?= =?utf-8?B?cTIrSFFjcEJ2ZDRYYVJ3TVIzaXNzRUVVT0xkYzFyUFV0QTVuWG1ybFYySVl6?= =?utf-8?B?bE9XU1g0cHdJa2FsL1Y5QTR0TWNOQm9TWE1KMHNtTkxyK1NtYW5yZDVIS2E2?= =?utf-8?B?RHNJSzAzZlByS1NMbWMxRHFVVVpPSlEyT1J0bDlqQUV1NWcwTldnUGtqZStD?= =?utf-8?B?TW8vQy9vZlFhQ3JhTmUrQ1MyZXRpZm5yejdLWndXVE5BZUh1cmsrZG5Rb3RS?= =?utf-8?B?QTJLam1wN2FRbkVmYXJwRlZqVnZHM05GMzhMWjR2RW1KTjZXL29jQWlVdUZw?= =?utf-8?B?UmhtbmVRd0NsbHNURCtPUVdYWk00Zy95Qjh2aldDdDJ5bldwSkRqaVp2dk9U?= =?utf-8?B?eE5aalNlVk4wZ0hyWUEvQnI1NHNMTEJERjRIVlArVk5HK1EzYkw5TTdqdWNS?= =?utf-8?B?Y0R0ME5Eblc1bFdoZ2RHRGFYWG5uVTRERVZocWlFTy82QWhLS3FLNlNuaEVS?= =?utf-8?B?LzNBWVJka0M2YUFjZnBQVVp3bkVHaDRZWFF1ZUczOVVQNXJ5MUljRG5sc1lO?= =?utf-8?B?bEwvSXYxWklUU0pGU0F3d0FJZFpTNFBzckpTWFhMWW91RHBMVVFLZXRhZTJp?= =?utf-8?B?WTZ1WWttSURvR1RUWGovVjlxbHZMZmVhS2k1aXdJSjRQamNSM095ZmhIOUV1?= =?utf-8?B?M3ozVHB5YjBja2VHVFptQkFZU0hSNDJKczhaUzNjdzNNY2p2aTRJbWF0VEVx?= =?utf-8?B?OWFzWXJEYlduNjVyY2wxOWNTNGNId2ZCVE81ejRSWi9NWjkxSmFybnVSQ2JZ?= =?utf-8?B?TkVLZWQ0ZThNNjFCeUY3clRhdFBBaUFSeWg4TmlyV1VKQ2p6YzhFeVAwY2Fq?= =?utf-8?B?VjZRSGx6b1RacllBcmVFUmZPc3hoL3JTRVZab1FoS09QaURLUGNPWFErUlNE?= =?utf-8?B?QzBVU3B6UFN5M2duSDBrZFd5cjFqbG15c2I0R2V1S2xoeThwa3FsTFhTUEVj?= =?utf-8?B?OGJWbUVJTXpwYkZjSCtycGZMQ05BbitGSllCYTQwUW1KTmhxTERVUjltZS9y?= =?utf-8?B?ZEdaVnFwandqd0tPZ0wzL1Z5b0RlSXozdlVUTEcyZU5CYi80Q0R0MEtUUURw?= =?utf-8?B?QUJPNWVxeUgvZnZ1bm1VUmJTNDhKcDJQbThmTnBDZ3JsVkpyakJ4eFN2Q3o4?= =?utf-8?B?Ym1LMi93TjdsUUdxV2dJSkZaNHlUSkx5ZmIzZENZWGczNXQrQU94QzJWeDNl?= =?utf-8?B?N2pPQ1U4MytBU1lMMVZDc05pZDdEQXFxTE5VZlVsaEZWZXdXekg2aHFtZC9q?= =?utf-8?B?RHRKK205ODdmdlc2WHV4MVVDSWJ4YjBKTGlDNjUySHlWRUk2UzdSTU5tSzVZ?= =?utf-8?B?SUZEYW5TTzF6WkwyYno5QkF0ZFhvaXl4ajdoaDZVdDk3SDF0N0xwZXVMQzV5?= =?utf-8?B?dlYySmVWOWkrQXUxaUh1UmFLYnBuOXlMdEZrOWRZQmRqSEd5NExhRFFlYmE3?= =?utf-8?B?dDAzMHhYdlhRTERrWk9WeXEra2orMkkrNmdiUmxpbGhQcnk3MU02aElocndT?= =?utf-8?B?ZmVrVlhTcWt2aUl1aW4wcE5YWVpGWGh5NHpHMmhTdVZKR1VNNVFMZUQ3T1FL?= =?utf-8?B?T2ZqRy9CRVVhRVlDdkQ5dE0zNEN4ci9VQ2R1VlNFNWlPTjk2Q2Jickhya2Yx?= =?utf-8?B?NTdycWdIYmNyWUVVRFNPaE9xSUU0bUphdUJtU0ROUktYWnNWUFZFOG1BMDdM?= =?utf-8?B?NGNka3RTMzdTSjVTUVV4eEdMQjFHTklwM3QwTWhDK0E1UnhVV2tiakptY2cw?= =?utf-8?B?ZDd4b2xWNjFhWG5rN2hsM0NoQXczV1Zxb0ZIZmFtTStrSXo5cXBkeWdFaFl6?= =?utf-8?B?bEo4STlZVlVkaGN2SXVlUkxyNEhvMnpLb2c3QzVTTUpVNk1wcENmNFl0empW?= =?utf-8?B?WkRmUWFYZGxQdVBpbW1tOURDZnUvNmc0OTdFV0hGRWZEcEE2UTRzWUpCRDh2?= =?utf-8?B?L1Yxb1g4elp5VVJnaFRHNFFONk0waDRUY3FxYWg5Yk9PZ0RBd295ZUJ3MVdu?= =?utf-8?Q?VejX2aofP9Xl5RngkzUtErARY?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: c201abbb-9fd1-4633-dd23-08da7f4d7230 X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Aug 2022 06:06:23.2766 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 9UK0E7ePIS1uzKl/jD7CpRqZ+3x199QUTgeq1NYej9OsSjLr/v91pQ0aWrLhCQ/0YfpAKYOO5J2L188v6KEMHA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0401MB2581 X-Spam-Status: No, score=-3030.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Aug 2022 06:06:29 -0000 On 15.08.2022 21:19, H.J. Lu wrote: > On Mon, Aug 15, 2022 at 8:38 AM Jan Beulich wrote: >> >> PR binutils/29483 >> >> When print_insn() processes op_txt[], it may pass strings into >> i386_dis_printf() which staging_area[] cannot fit; this was observed for >> an invalid form of VPSCATTERDD (both broadcast and zeroing-masking bits >> set). Rather than arbitrarily enlarging that local array, avoid its use >> altogether when the format string is simply "%s". This merely requires >> two local variables to have their type constified. >> >> While limiting the scope of "res" it became apparent that >> - no caller cares about the function's return value, >> - the comment about the return value was wrong, >> - a particular positive return value would have been meaningless to the >> caller. >> Therefore convert the function to return "void" at the same time. >> --- >> An alternative to the special casing would be to introduce something >> like i386_dis_puts(), then to be used by all call sites which currently >> pass "%s" or format strings without any format characters at all (plus, >> of course, i386_dis_printf() itself). >> --- >> v2: Add testcase. >> >> --- a/gas/testsuite/gas/i386/i386.exp >> +++ b/gas/testsuite/gas/i386/i386.exp >> @@ -1349,6 +1349,7 @@ if [gas_64_check] then { >> run_dump_test ehinterp >> } >> run_dump_test pr27198 >> + run_dump_test pr29483 >> >> set ASFLAGS "$old_ASFLAGS --64" >> >> --- /dev/null >> +++ b/gas/testsuite/gas/i386/pr29483.d >> @@ -0,0 +1,11 @@ >> +#objdump: -dw >> +#name: x86-64 PR binutils/29483 >> + >> +.*: +file format .* >> + >> +Disassembly of section .text: >> + >> +0+ : >> +[ ]*[a-f0-9]+: 65 62 62 7d 97 a0 94 ff 20 20 20 ae vpscatterdd .* >> + >> +0x65,0x62,0x62,0x7d,0x97,0xa0,0x94,0xff,0x20,0x20,0x20,0xae >> --- /dev/null >> +++ b/gas/testsuite/gas/i386/pr29483.s >> @@ -0,0 +1,5 @@ >> + .text >> +pr29483: >> + # This (VPSCATTERDD with EVEX.br and EVEX.z invalidly set) should not >> + # crash the disassembler. >> + .byte 0x65,0x62,0x62,0x7d,0x97,0xa0,0x94,0xff,0x20,0x20,0x20,0xae >> --- a/opcodes/i386-dis.c >> +++ b/opcodes/i386-dis.c >> @@ -9264,31 +9264,40 @@ oappend_register (instr_info *ins, const >> STYLE is the default style to use in the fprintf_styled_func calls, >> however, FMT might include embedded style markers (see oappend_style), >> these embedded markers are not printed, but instead change the style >> - used in the next fprintf_styled_func call. >> + used in the next fprintf_styled_func call. */ >> >> - Return non-zero to indicate the print call was a success. */ >> - >> -static int ATTRIBUTE_PRINTF_3 >> +static void ATTRIBUTE_PRINTF_3 >> i386_dis_printf (instr_info *ins, enum disassembler_style style, >> const char *fmt, ...) >> { >> va_list ap; >> enum disassembler_style curr_style = style; >> - char *start, *curr; >> + const char *start, *curr; >> char staging_area[100]; >> - int res; >> >> va_start (ap, fmt); >> - res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); >> - va_end (ap); >> + /* In particular print_insn()'s processing of op_txt[] can hand rather long >> + strings here. Bypass vsnprintf() in such cases to avoid capacity issues >> + with the staging area. */ >> + if (strcmp (fmt, "%s")) >> + { >> + int res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); >> >> - if (res < 0) >> - return res; >> + va_end (ap); >> >> - if ((size_t) res >= sizeof (staging_area)) >> - abort (); >> + if (res < 0) >> + return; >> >> - start = curr = staging_area; >> + if ((size_t) res >= sizeof (staging_area)) >> + abort (); >> + >> + start = curr = staging_area; >> + } >> + else >> + { >> + start = curr = va_arg (ap, const char *); >> + va_end (ap); >> + } >> >> do >> { >> @@ -9303,10 +9312,7 @@ i386_dis_printf (instr_info *ins, enum d >> curr_style, >> "%.*s", len, start); >> if (n < 0) >> - { >> - res = n; >> - break; >> - } >> + break; >> >> if (*curr == '\0') >> break; >> @@ -9340,8 +9346,6 @@ i386_dis_printf (instr_info *ins, enum d >> ++curr; >> } >> while (true); >> - >> - return res; >> } >> >> static int > > I prefer something like this with the disassembler output: > > 0: 65 62 62 7d 97 a0 94 ff 20 20 20 ae vpscatterdd > %xmm26,%gs:-0x51dfdfe0(%rdi,%xmm23,8){bad}{%k7}{z}/(bad) But the precise representation of the "badness" isn't relevant to this testcase. All we care about is that there's no crash (and perhaps, see below, no overrun of the other buffer, but I view this as a separate aspect). As to the attachment - I figure honoring op_out[]'s 2nd dimension is a necessary thing. I don't think I agree with bumping the buffer size, though: 128 is as arbitrary as 100. Or else there would want to be a comment next to MAX_OPERAND_BUFFER_SIZE explaining how the value was determined and hence making clear under what conditions it might need further bumping. Yet you're the maintainer - if you want to go with your fix, I won't be able to stop you. Jan