From patchwork Mon Apr 11 17:49:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joan Bruguera X-Patchwork-Id: 52796 X-Patchwork-Delegate: siddhesh@gotplt.org 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 63B623858009 for ; Mon, 11 Apr 2022 17:50:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 63B623858009 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1649699424; bh=hzrEj/Yv9jf/nLIlxllgyd+0PuICSEiHBFnFdOq5mhg=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=RJvBDgYeEY8CaMO6n1ISZ3T8S2tS//B4Au0ohyGG1Id9OE8euvO4n0vvEeKFChrHz jVLwGVHwixpOIeOMRqOsFK9PYY5ytuIbIMpq9LiFqh6d807EXC+LulqKE2liJspj+w /QtW8cBHiOEO9FCPloAROQ3RQdCfz3MOAOK7Y0VA= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id EFB923858D1E for ; Mon, 11 Apr 2022 17:50:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EFB923858D1E Received: by mail-wr1-x435.google.com with SMTP id e8so7753790wra.7 for ; Mon, 11 Apr 2022 10:50:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=hzrEj/Yv9jf/nLIlxllgyd+0PuICSEiHBFnFdOq5mhg=; b=Xs9mV25a0tvub4OS50n5ieFqR8VGiIDlZ+ttZd3uRS3RCkieTrlFeXuhlmzO/OxEp0 r4k6pCqyhz9FjQZG/DXpR6VeSFManfZqhH/YvCXFhiJwVOGh9C01lBwU4xu58gaZvH9a YXrhOHI84Fk2lIIkIlheOBFLTwn3o8Y95tpB6/0SX2RqL+JLDCrHqSnDbhLGUZvfHS2I kXxtH/tnhVdHmStF1M85HzkEV4YORKcVtaIb8r/6jmHy/2s8HpqrVs/D7J38C/M82IOU QmW1vyl0I/BqjH8VFg8yrlUUsxkSR9dP0UqIXPN0mPkT91CqxwikRP2pEKx2cyqgfPU1 GClg== X-Gm-Message-State: AOAM530FypxIWCYj2OWqYYxjJx5WelOD21jtRcFWqlBRC6D3Fhy+SQIF 4WTaQjbk8p57Zfdkud7gdnpzVjrvudNsog== X-Google-Smtp-Source: ABdhPJxPPMtmwlF474yA78fRSKjd54xILSrx1M2n0tg2ZlrhEBRSk6JDwjPlzfjd2FbxkkFlVATU3w== X-Received: by 2002:a05:6000:1789:b0:206:2d1:6f35 with SMTP id e9-20020a056000178900b0020602d16f35mr26028531wrg.613.1649699399322; Mon, 11 Apr 2022 10:49:59 -0700 (PDT) Received: from solpc.. ([84.77.58.140]) by smtp.gmail.com with ESMTPSA id m7-20020adfe0c7000000b002060e7bbe49sm27687116wri.45.2022.04.11.10.49.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Apr 2022 10:49:58 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH] misc: Fix rare fortify crash on wchar funcs. [BZ 29030] Date: Mon, 11 Apr 2022 19:49:56 +0200 Message-Id: <20220411174956.2657622-1-joanbrugueram@gmail.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Joan Bruguera via Libc-alpha From: Joan Bruguera Reply-To: Joan Bruguera Cc: Joan Bruguera Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" If `__glibc_objsize (__o) == (size_t) -1` (i.e. `__o` is unknown size), fortify checks should pass, and `__whatever_alias` should be called. Previously, `__glibc_objsize (__o) == (size_t) -1` was explicitly checked, but on commit a643f60c53876b, this was moved into `__glibc_safe_or_unknown_len`. A comment says the -1 case should work as: "The -1 check is redundant because since it implies that __glibc_safe_len_cond is true.". But this fails when: * `__s > 1` * `__osz == -1` (i.e. unknown size at compile time) * `__l` is big enough * `__l * __s <= __osz` can be folded to a constant (I only found this to be true for `mbsrtowcs` and other functions in wchar2.h) In this case `__l * __s <= __osz` is false, and `__whatever_chk_warn` will be called by `__glibc_fortify` or `__glibc_fortify_n` and crash the program. This commit adds the explicit `__osz == -1` check again. moc crashes on startup due to this, see: https://bugs.archlinux.org/task/74041 Minimal test case (test.c): #include int main (void) { const char *hw = "HelloWorld"; mbsrtowcs (NULL, &hw, (size_t)-1, NULL); return 0; } Build with: gcc -O2 -Wp,-D_FORTIFY_SOURCE=2 test.c -o test && ./test Output: *** buffer overflow detected ***: terminated Fixes: a643f60c53876b ("Make sure that the fortified function conditionals are constant") Fixes: https://sourceware.org/bugzilla/show_bug.cgi?id=29030 Signed-off-by: Joan Bruguera --- debug/tst-fortify.c | 5 +++++ misc/sys/cdefs.h | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c index d65a2fe6e1..03c9867714 100644 --- a/debug/tst-fortify.c +++ b/debug/tst-fortify.c @@ -1504,6 +1504,11 @@ do_test (void) CHK_FAIL_END #endif + /* Bug 29030 regresion check */ + cp = "HelloWorld"; + if (mbsrtowcs (NULL, &cp, (size_t)-1, &s) != 10) + FAIL (); + cp = "A"; if (mbstowcs (wenough, cp, 10) != 1 || wcscmp (wenough, L"A") != 0) diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 44d3826bca..b8419e7e6c 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -156,14 +156,14 @@ variants. These conditions should get evaluated to constant and optimized away. */ -#define __glibc_safe_len_cond(__l, __s, __osz) ((__l) <= (__osz) / (__s)) +#define __glibc_safe_len_cond(__l, __s, __osz) \ + ((__osz) == (size_t) -1 || ((__l) <= (__osz) / (__s))) #define __glibc_unsigned_or_positive(__l) \ ((__typeof (__l)) 0 < (__typeof (__l)) -1 \ || (__builtin_constant_p (__l) && (__l) > 0)) /* Length is known to be safe at compile time if the __L * __S <= __OBJSZ - condition can be folded to a constant and if it is true. The -1 check is - redundant because since it implies that __glibc_safe_len_cond is true. */ + condition can be folded to a constant and if it is true, or unknown (-1) */ #define __glibc_safe_or_unknown_len(__l, __s, __osz) \ (__glibc_unsigned_or_positive (__l) \ && __builtin_constant_p (__glibc_safe_len_cond ((__SIZE_TYPE__) (__l), \