Message ID | Y9oIhhJR9qaUEmFI@squeak.grove.modra.org |
---|---|
State | Repeat Merge |
Headers |
Return-Path: <binutils-bounces+ouuuleilei=gmail.com@sourceware.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp115184wrn; Tue, 31 Jan 2023 22:37:10 -0800 (PST) X-Google-Smtp-Source: AK7set+qm7CjEjoj9uc6vX1RsXAsklZl+rz87FHFt8TLyZ6dpAec3pLxDmq1TDnIiEYBZjT+tXhp X-Received: by 2002:a17:907:8e93:b0:881:f614:44ed with SMTP id tx19-20020a1709078e9300b00881f61444edmr1294423ejc.30.1675233429776; Tue, 31 Jan 2023 22:37:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675233429; cv=none; d=google.com; s=arc-20160816; b=J78BJSy+rhXPlmGNRq4CROYKeKeDoCxuFTZ2avUkVVI7frj4jNr4eybjCbcIwzNan5 DxGwI5yHLHFt8iBX3de7n5HixcvkgFo/nuy35E9rSFIkrmd0/T/M9E5RdXUB9uIDRDfM AivnSdKYXx6n4pYVMDFeUU0yDu/Lph2k0gx03d3gWc+OGpmcNrK6JpPlVwsS+O8hoOxO BpPlIKibOCjpIXiEINipdKpd1L2mQT+w/fnqcTAd5BKVs6wLW6KRtzZgH5YB6qySQ9WW AGSCDcnSKd4o8/hd97G/LHU9YwaJXjqn1c0xY5FVnTXq3KwoYNm5jdB3myn6UoBfBWt1 nj1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:message-id:subject:to:date :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=PU2esnKsGWo0uR46wH5ef7WtCFINTkb8kS+QB845LYA=; b=iYuZua3fuxTJuKI8nt0sBxmbjoSwEwc6uFRpSE/VNmT7EuLlFHfE/usYYilU6+C6Zz GACn169LbaNJwVuuLzvxIFF7qpas3PwPMTfhRhqYiKUY53ih98ZWNOLl+oZKeXPfL00h lK6YnsYYDYucqIbb5bwlfqnFoL9dG18KdhrfkNlPoo2lEz/scSPxOLEehpnL6DjXVClq hsLIifmVHo8sMu3NraQLgFjoCNHCG0n0zeDCwCQ+yDGfidWMYDqSY8l14ipQJD7m1QW9 ZmeSSBF+TI6fso9ES1E+tKQWz4iiQGGWoh11m8tSFAQqDlDx27sabNUeWAPQLtocc3+l mvyg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=QBDOe9m5; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id ee2-20020a056402290200b004a2134ce795si14836668edb.601.2023.01.31.22.37.09 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Jan 2023 22:37:09 -0800 (PST) Received-SPF: pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=QBDOe9m5; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CAD5A3858C5F for <ouuuleilei@gmail.com>; Wed, 1 Feb 2023 06:37:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CAD5A3858C5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1675233426; bh=PU2esnKsGWo0uR46wH5ef7WtCFINTkb8kS+QB845LYA=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=QBDOe9m5bXDn6NP6Rtk7dUpjGcRZ4/k49sMSWWuHbcNqnkzDkA1Rli2i7PU+RuKq+ dhReT5iQViy3hOOu91nNktS1DP40Z8bB5Nfffz8cjZlRKjerIQPLXbg8jW90uj3Tng +P4OgVv6HP1wOjIzaYruolof5xaxVmtvYOGT3IV8= X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by sourceware.org (Postfix) with ESMTPS id 9AC883858D38 for <binutils@sourceware.org>; Wed, 1 Feb 2023 06:36:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9AC883858D38 Received: by mail-pl1-x636.google.com with SMTP id b5so10239135plz.5 for <binutils@sourceware.org>; Tue, 31 Jan 2023 22:36:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PU2esnKsGWo0uR46wH5ef7WtCFINTkb8kS+QB845LYA=; b=vR5VzFOl9jOSSdVUoINdCdd0LL7HFuNPq3mXOXBanhgXv3lKetQXYZvw1Qj/h7uS8g b0C3eHy3kKpb46D7eEinECLJWDXFV9za56kvM3Uu8G2adF1EMYjMYxrswFaKpOSMobY+ FwCH2E2Ap/CKZ6dsilr+zOi26DJscPaH69G5+EnQKzMJmFqXO+MtKAwhyWc51osYk1ZM mG1JWdY8KgzwOHGyfvCXIfWIpTMZVN/ZycO/OLQz9l4rWIG33qEQ8S6pLIAe8ttllQtl xDShUSwTengjITc7AzPDRFvyHiaJATaOMuWHjUxCrcQRWd5tHal04S4e31ZasHB10hih G8nw== X-Gm-Message-State: AO0yUKV+2xAAKhfIWFt+L0KdJxJ4A7RyWudyqfYQVcHvFHolU/ySlCCm TdswfjUcgIjSnu8HfsAS9sS0vy2dYV4= X-Received: by 2002:a17:902:cecf:b0:196:89bc:70fe with SMTP id d15-20020a170902cecf00b0019689bc70femr2086779plg.34.1675233417499; Tue, 31 Jan 2023 22:36:57 -0800 (PST) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:35b4:d041:2beb:f8d7]) by smtp.gmail.com with ESMTPSA id 2-20020a170902c10200b0018099c9618esm10797250pli.231.2023.01.31.22.36.56 for <binutils@sourceware.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Jan 2023 22:36:56 -0800 (PST) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 687CC1140841; Wed, 1 Feb 2023 17:06:54 +1030 (ACDT) Date: Wed, 1 Feb 2023 17:06:54 +1030 To: binutils@sourceware.org Subject: Recursion in as_info_where Message-ID: <Y9oIhhJR9qaUEmFI@squeak.grove.modra.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="cua0SDu25lOytUVR" Content-Disposition: inline X-Spam-Status: No, score=-3035.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list <binutils.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/binutils>, <mailto:binutils-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/binutils/> List-Post: <mailto:binutils@sourceware.org> List-Help: <mailto:binutils-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/binutils>, <mailto:binutils-request@sourceware.org?subject=subscribe> From: Alan Modra via Binutils <binutils@sourceware.org> Reply-To: Alan Modra <amodra@gmail.com> Errors-To: binutils-bounces+ouuuleilei=gmail.com@sourceware.org Sender: "Binutils" <binutils-bounces+ouuuleilei=gmail.com@sourceware.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756609569041777871?= X-GMAIL-MSGID: =?utf-8?q?1756609569041777871?= |
Series |
Recursion in as_info_where
|
|
Checks
Context | Check | Description |
---|---|---|
snail/binutils-gdb-check | warning | Git am fail log |
Commit Message
Alan Modra
Feb. 1, 2023, 6:36 a.m. UTC
This function has a gas_assert, ie. possible call to as_abort, which calls as_report_context, which calls as_info_where. Attached fuzzer testcase managed to trigger a stack overflow. * messages.c (as_info_where): Don't gas_assert.
Comments
On 01.02.2023 07:36, Alan Modra via Binutils wrote: > This function has a gas_assert, ie. possible call to as_abort, which > calls as_report_context, which calls as_info_where. Attached fuzzer > testcase managed to trigger a stack overflow. > > * messages.c (as_info_where): Don't gas_assert. > > diff --git a/gas/messages.c b/gas/messages.c > index 0db075d779c..7c018acf69f 100644 > --- a/gas/messages.c > +++ b/gas/messages.c > @@ -141,8 +141,6 @@ as_info_where (const char *file, unsigned int line, unsigned int indent, > va_list args; > char buffer[2000]; > > - gas_assert (file != NULL && line > 0 && indent <= INT_MAX); If this go in the way, isn't it that the assertion actually triggered? In which case shouldn't the cause for it triggering be addressed instead, to avoid subsequent knock-on damage (e.g. from de-referencing "file"? (I may want to play with the testcase a little myself.) Jan
On Wed, Feb 01, 2023 at 10:24:25AM +0100, Jan Beulich wrote: > On 01.02.2023 07:36, Alan Modra via Binutils wrote: > > This function has a gas_assert, ie. possible call to as_abort, which > > calls as_report_context, which calls as_info_where. Attached fuzzer > > testcase managed to trigger a stack overflow. > > > > * messages.c (as_info_where): Don't gas_assert. > > > > diff --git a/gas/messages.c b/gas/messages.c > > index 0db075d779c..7c018acf69f 100644 > > --- a/gas/messages.c > > +++ b/gas/messages.c > > @@ -141,8 +141,6 @@ as_info_where (const char *file, unsigned int line, unsigned int indent, > > va_list args; > > char buffer[2000]; > > > > - gas_assert (file != NULL && line > 0 && indent <= INT_MAX); > > If this go in the way, isn't it that the assertion actually triggered? > In which case shouldn't the cause for it triggering be addressed > instead, to avoid subsequent knock-on damage (e.g. from de-referencing > "file"? (I may want to play with the testcase a little myself.) The testcase is really weird, something that no programmer would ever write. It failed the assert with line == 0. I'll let you discover the horrible "# line file" with embedded \0 that gets you there. :-) I'm not motivated to fix that sort of insanity.
On 01.02.2023 12:59, Alan Modra wrote: > On Wed, Feb 01, 2023 at 10:24:25AM +0100, Jan Beulich wrote: >> On 01.02.2023 07:36, Alan Modra via Binutils wrote: >>> This function has a gas_assert, ie. possible call to as_abort, which >>> calls as_report_context, which calls as_info_where. Attached fuzzer >>> testcase managed to trigger a stack overflow. >>> >>> * messages.c (as_info_where): Don't gas_assert. >>> >>> diff --git a/gas/messages.c b/gas/messages.c >>> index 0db075d779c..7c018acf69f 100644 >>> --- a/gas/messages.c >>> +++ b/gas/messages.c >>> @@ -141,8 +141,6 @@ as_info_where (const char *file, unsigned int line, unsigned int indent, >>> va_list args; >>> char buffer[2000]; >>> >>> - gas_assert (file != NULL && line > 0 && indent <= INT_MAX); >> >> If this go in the way, isn't it that the assertion actually triggered? >> In which case shouldn't the cause for it triggering be addressed >> instead, to avoid subsequent knock-on damage (e.g. from de-referencing >> "file"? (I may want to play with the testcase a little myself.) > > The testcase is really weird, something that no programmer would ever > write. It failed the assert with line == 0. I'll let you discover > the horrible "# line file" with embedded \0 that gets you there. :-) Sure. That's an interaction bug between read_a_source_file()'s bumping of line numbers and s_linefile() trying to compensate. I have a fix for that, but I'll want to give it wider testing before posting. As to the assertion here, I'd prefer if we would keep it, and do e.g. the below instead. Thoughts? Jan --- a/gas/messages.c +++ b/gas/messages.c @@ -140,6 +140,12 @@ as_info_where (const char *file, unsigne { va_list args; char buffer[2000]; + static bool active; + + /* We can come back here if e.g. the assertion below triggers. */ + if (active) + return; + active = true; gas_assert (file != NULL && line > 0 && indent <= INT_MAX); @@ -148,6 +154,8 @@ as_info_where (const char *file, unsigne va_end (args); fprintf (stderr, "%s:%u: %*s%s%s\n", file, line, (int)indent, "", _("Info: "), buffer); + + active = false; } /* Send to stderr a string as a warning, and locate warning
On Fri, Feb 03, 2023 at 02:44:06PM +0100, Jan Beulich wrote: > On 01.02.2023 12:59, Alan Modra wrote: > > On Wed, Feb 01, 2023 at 10:24:25AM +0100, Jan Beulich wrote: > >> On 01.02.2023 07:36, Alan Modra via Binutils wrote: > >>> This function has a gas_assert, ie. possible call to as_abort, which > >>> calls as_report_context, which calls as_info_where. Attached fuzzer > >>> testcase managed to trigger a stack overflow. > >>> > >>> * messages.c (as_info_where): Don't gas_assert. > >>> > >>> diff --git a/gas/messages.c b/gas/messages.c > >>> index 0db075d779c..7c018acf69f 100644 > >>> --- a/gas/messages.c > >>> +++ b/gas/messages.c > >>> @@ -141,8 +141,6 @@ as_info_where (const char *file, unsigned int line, unsigned int indent, > >>> va_list args; > >>> char buffer[2000]; > >>> > >>> - gas_assert (file != NULL && line > 0 && indent <= INT_MAX); > >> > >> If this go in the way, isn't it that the assertion actually triggered? > >> In which case shouldn't the cause for it triggering be addressed > >> instead, to avoid subsequent knock-on damage (e.g. from de-referencing > >> "file"? (I may want to play with the testcase a little myself.) > > > > The testcase is really weird, something that no programmer would ever > > write. It failed the assert with line == 0. I'll let you discover > > the horrible "# line file" with embedded \0 that gets you there. :-) > > Sure. That's an interaction bug between read_a_source_file()'s bumping > of line numbers and s_linefile() trying to compensate. I have a fix for > that, but I'll want to give it wider testing before posting. > > As to the assertion here, I'd prefer if we would keep it, and do e.g. > the below instead. Thoughts? I think it is bad practice for a low-level error handling function to assert.
diff --git a/gas/messages.c b/gas/messages.c index 0db075d779c..7c018acf69f 100644 --- a/gas/messages.c +++ b/gas/messages.c @@ -141,8 +141,6 @@ as_info_where (const char *file, unsigned int line, unsigned int indent, va_list args; char buffer[2000]; - gas_assert (file != NULL && line > 0 && indent <= INT_MAX); - va_start (args, format); vsnprintf (buffer, sizeof (buffer), format, args); va_end (args);