From patchwork Mon Jul 29 17:09:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 94725 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 9D7163858289 for ; Mon, 29 Jul 2024 17:10:21 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 7001E3858C3A for ; Mon, 29 Jul 2024 17:09:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7001E3858C3A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7001E3858C3A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722272999; cv=none; b=aC9I81eIFz4Fn8fd83SeQelL+7QTwOxmFLp51hmbMLz02gpZbJw55droDPb7VZLgjQQBb2o/0WjvTfmSRfuj82d6/3JLq3o1jGh4T91xAWum80kkfkRtcFUDkLVvsPFatRjDApMlkStVEaIeW1rWFlNjBA2O7sngYS5Ox1FVMS8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722272999; c=relaxed/simple; bh=aiVEU84Fq2PvJ/gNfv5nMqP+7IKxFfh7zdJuUCv2sHA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=CkjfceXU8AUIgu3fsSyfyChBmCzwKKo5M9z3P36GyU0R20vTyzpIIattx15QDTg2nbM0SYLa01QTrDFWWBsaG0mDMEm6675Pg4C8FLzeiFNaxniqOvpEO4fnE0ANfuUF12/slAj6XrPLvAcEXXu3sjJ1HrW3+GfgnPB3Fgf2TsU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722272997; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=N7NdoHr22L8wLM0sbP1Nky36VTM4iGmSDShreVJYlUU=; b=R+1i4SCnS2GurvkPdXYh4FWY1oNtd0or9SDUJB67I8BQmHdsvw47caIemyZ4HLEvlZQCaz YIP2sr31Kv2ITrnEc5O4ph7bgTRXlBa60zcjIg70KZeHjaxAqvhRuJipToJcDpQ7+zihE7 XSwlBUkoNyn+E70RrdCQR4XTZECBNAc= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-458-7_mm3RE4M9-CB6YeoSmNfA-1; Mon, 29 Jul 2024 13:09:55 -0400 X-MC-Unique: 7_mm3RE4M9-CB6YeoSmNfA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 621211955D4E; Mon, 29 Jul 2024 17:09:53 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.45.224.31]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 12A0019560AA; Mon, 29 Jul 2024 17:09:49 +0000 (UTC) From: Florian Weimer To: libc-alpha@sourceware.org Cc: Dmitry V. Levin , Archie Cobbs , Solar Designer , Sam James , Andreas Schwab Subject: [PATCH v3] libio: asprintf should write NULL upon failure Date: Mon, 29 Jul 2024 19:09:46 +0200 Message-ID: <87ttg8ktut.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org This was suggested most recently by Solar Designer, noting that code replacing vsprintf with vasprintf in a security fix was subtly wrong: Re: GStreamer Security Advisory 2024-0003: Orc compiler stack-based buffer overflow Previous libc-alpha discussions: I: [PATCH] asprintf error handling fix asprintf() issue I don't think we need a compatibility symbol for this. As the most recent GStreamer example shows, this change is much more likely to fix bugs than cause compatibility issues. Suggested-by: Dmitry V. Levin Suggested-by: Archie Cobbs Suggested-by: Solar Designer Reviewed-by: Sam James --- Cc: v3: more attribute, do not change the extension status described in the manual. libio/Makefile | 1 + libio/tst-asprintf-null.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++ libio/vasprintf.c | 17 ++++++++-------- manual/stdio.texi | 4 +++- 4 files changed, 63 insertions(+), 10 deletions(-) base-commit: aedbf08891069fc029ed021e4dba933eb877b394 diff --git a/libio/Makefile b/libio/Makefile index 6a507b67ea..a2d1cde955 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -86,6 +86,7 @@ tests = \ bug-wmemstream1 \ bug-wsetpos \ test-fmemopen \ + tst-asprintf-null \ tst-atime \ tst-bz22415 \ tst-bz24051 \ diff --git a/libio/tst-asprintf-null.c b/libio/tst-asprintf-null.c new file mode 100644 index 0000000000..1eebeb200f --- /dev/null +++ b/libio/tst-asprintf-null.c @@ -0,0 +1,51 @@ +/* Test that asprintf sets the buffer pointer to NULL on failure. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static int +do_test (void) +{ + static const char sentinel[] = "sentinel"; + char *buf = (char *) sentinel; + { + /* Avoid -Wformat-overflow warning. */ + const char *volatile format = "%2000000000d %2000000000d"; + TEST_COMPARE (asprintf (&buf, format, 1, 2), -1); + } + if (errno != ENOMEM) + TEST_COMPARE (errno, EOVERFLOW); + TEST_VERIFY (buf == NULL); + + /* Force ENOMEM in the test below. */ + struct rlimit rl; + TEST_COMPARE (getrlimit (RLIMIT_AS, &rl), 0); + rl.rlim_cur = 10 * 1024 * 1024; + TEST_COMPARE (setrlimit (RLIMIT_AS, &rl), 0); + + buf = (char *) sentinel; + TEST_COMPARE (asprintf (&buf, "%20000000d", 1), -1); + TEST_COMPARE (errno, ENOMEM); + TEST_VERIFY (buf == NULL); + return 0; +} + +#include diff --git a/libio/vasprintf.c b/libio/vasprintf.c index 999ae526f4..24f2a2e175 100644 --- a/libio/vasprintf.c +++ b/libio/vasprintf.c @@ -92,7 +92,7 @@ __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf) int -__vasprintf_internal (char **result_ptr, const char *format, va_list args, +__vasprintf_internal (char **result, const char *format, va_list args, unsigned int mode_flags) { struct __printf_buffer_asprintf buf; @@ -105,23 +105,23 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args, { if (buf.base.write_base != buf.direct) free (buf.base.write_base); + *result = NULL; return done; } /* Transfer to the final buffer. */ - char *result; size_t size = buf.base.write_ptr - buf.base.write_base; if (buf.base.write_base == buf.direct) { - result = malloc (size + 1); - if (result == NULL) + *result = malloc (size + 1); + if (*result == NULL) return -1; - memcpy (result, buf.direct, size); + memcpy (*result, buf.direct, size); } else { - result = realloc (buf.base.write_base, size + 1); - if (result == NULL) + *result = realloc (buf.base.write_base, size + 1); + if (*result == NULL) { free (buf.base.write_base); return -1; @@ -129,8 +129,7 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args, } /* Add NUL termination. */ - result[size] = '\0'; - *result_ptr = result; + (*result)[size] = '\0'; return done; } diff --git a/manual/stdio.texi b/manual/stdio.texi index f9529a098d..9a7a2d4bd0 100644 --- a/manual/stdio.texi +++ b/manual/stdio.texi @@ -2524,7 +2524,9 @@ Allocation}) to hold the output, instead of putting the output in a buffer you allocate in advance. The @var{ptr} argument should be the address of a @code{char *} object, and a successful call to @code{asprintf} stores a pointer to the newly allocated string at that -location. +location. Current versions of @theglibc{} write a null pointer to +@samp{*@var{ptr}} upon failure, but this is not required by the +standard, and previous versions did not modify the pointer value. The return value is the number of characters allocated for the buffer, or less than zero if an error occurred. Usually this means that the buffer