From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x234.google.com (mail-oi1-x234.google.com [IPv6:2607:f8b0:4864:20::234]) by sourceware.org (Postfix) with ESMTPS id 6C5563858D20 for ; Fri, 1 Sep 2023 14:23:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6C5563858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oi1-x234.google.com with SMTP id 5614622812f47-3a9b41ffe11so1242339b6e.2 for ; Fri, 01 Sep 2023 07:23:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1693578192; x=1694182992; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=vtVcpz7tz/UeOy1/q3mnRRTOkLtB9mPavTmoLr6QM08=; b=ZNznkk/GO/fi6EorVxbdW9iM+WCNLkqNu/bGM2JSsyrAmAmx+Yew5tYyWdd964p0BS UMMTVg8XEAM9YqL9mk1OPilkZdLKj0HpR0wbf+ytNK4f+o3/2xj7iKZaY/zROgrlr+sO WmKdkfxmu1fZugcm4nCEOnaO168GlPke+RgV3jg7d3KEuWu+SrAqWe5A7OdC72Es05dS AKeeEN0kbORnO885YjDrxds09u+F0u49x7brv5/NLit2vj0Q5xMccZG0em3Gez+BWxxS kYCr2IRcXvxkPWbt3ti5SvB8dbh5IJOlirBujC10BeZXzII1kUTiKStnSgTmOZykIwqH 4C5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693578192; x=1694182992; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vtVcpz7tz/UeOy1/q3mnRRTOkLtB9mPavTmoLr6QM08=; b=hez7Zydrbezm1XFx2UBVG/F/P/9lwMTDlYeFRCE4F8D81Y0y4LvUaaXTkFMe46kYPN rh+idkbx9TjOPM87CQ1+wR2dzk+5K6HsFgy+jpWXb3E2qymKsThm8zp2ClmyVYBkjtMJ l1hk6/LUDJEIHRvNTofjHUW8OuD1LILMqXsVwVFeMeGhpPm3xpR7cQOIz2iZRSOz9flu MdMT2AMHXxDITJwUn0exBbXsVxVjlm32ramXw8Y68I8eK/SL4D3kaVR3briaGmBi0Rcu ci98SpMPuCICzaV+0TksehBvjpqgWJY6Loj2BwoNuugx0hW5ZgzEptzhtpxtGqe3bLz4 +S8g== X-Gm-Message-State: AOJu0Yx84WQe7Z7lHO13VyBhHJcCEiFCmYu9CM/YcX5IgOCGuOWvHKkh FKYVqn+n5EfLPiACCMHRafwIpA== X-Google-Smtp-Source: AGHT+IEcuYZA3JO73AxVCANbtUQ6D3sPy47QBp0NpqQ/JAqt2ztuk+CpTxEP8e9LMhAJcWc2GDECsA== X-Received: by 2002:a54:4389:0:b0:3a1:df16:2eed with SMTP id u9-20020a544389000000b003a1df162eedmr2576929oiv.30.1693578191665; Fri, 01 Sep 2023 07:23:11 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c3:578c:2809:35ff:14c6:cc12? ([2804:1b3:a7c3:578c:2809:35ff:14c6:cc12]) by smtp.gmail.com with ESMTPSA id bj16-20020a056808199000b003a3645e9ebbsm2003349oib.40.2023.09.01.07.23.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Sep 2023 07:23:11 -0700 (PDT) Message-ID: <7a018de3-c2e8-f7f3-af4b-229995e683c1@linaro.org> Date: Fri, 1 Sep 2023 11:23:08 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH v2] libc_fatal: Get rid of alloca Content-Language: en-US To: Joe Simmons-Talbott , libc-alpha@sourceware.org References: <20230831202122.2239619-1-josimmon@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20230831202122.2239619-1-josimmon@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,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 List-Id: On 31/08/23 17:20, Joe Simmons-Talbott via Libc-alpha wrote: > Use fixed size arrays in place of alloca to avoid potential stack overflow. > Limit the number of varargs to __libc_message to 10. I think we enforce the maximum number of arguments internally with some macro tricks, so there is no need to bail out to abort without printing the message. > --- > Changes to v1: > * Use a fixed size array rather than scratch_buffers since we can only > call async signal safe functions. > > sysdeps/posix/libc_fatal.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c > index 70edcc10c1..16929addab 100644 > --- a/sysdeps/posix/libc_fatal.c > +++ b/sysdeps/posix/libc_fatal.c > @@ -45,6 +45,9 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total) > } > #endif > > +/* The maximum number of varargs allowed in a __libc_message format string */ > +#define MAX_NLIST 10 > + > struct str_list > { > const char *str; > @@ -58,6 +61,7 @@ __libc_message (const char *fmt, ...) > { > va_list ap; > int fd = -1; > + struct str_list _newp[MAX_NLIST]; There are no need to track the string list, it is used essentially to construct the iovec struct to call writev. You can construct the iovec directly. > > va_start (ap, fmt); > > @@ -70,6 +74,7 @@ __libc_message (const char *fmt, ...) > > struct str_list *list = NULL; > int nlist = 0; > + struct iovec iov[MAX_NLIST]; > > const char *cp = fmt; > while (*cp != '\0') > @@ -100,17 +105,18 @@ __libc_message (const char *fmt, ...) > cp = next; > } > > - struct str_list *newp = alloca (sizeof (struct str_list)); > + struct str_list *newp = &_newp[nlist]; > newp->str = str; > newp->len = len; > newp->next = list; > list = newp; > ++nlist; > + if (nlist > MAX_NLIST) > + goto fail_out; > } > > if (nlist > 0) > { > - struct iovec *iov = alloca (nlist * sizeof (struct iovec)); > ssize_t total = 0; > > for (int cnt = nlist - 1; cnt >= 0; --cnt) > @@ -146,6 +152,7 @@ __libc_message (const char *fmt, ...) > > va_end (ap); > > +fail_out: > /* Kill the application. */ > abort (); > } Below is a patch on top of your which enforces the maximum number of supported variadic arguments with a similar trick I used for INLINE_SYSCALL_CALL. One will need to explicit implement a new macro for each __libc_message usage with a number of arguments larger than LIBC_MESSAGE_MAX_ARGS, but it will fail at build time if you try to use __libc_message with 5 or more arguments. I think this is best we can do without a compiler attribute to help us to enforce it. -- diff --git a/include/stdio.h b/include/stdio.h index 6755877911..e2100066ed 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -172,10 +172,36 @@ extern int __gen_tempname (char *__tmpl, int __suffixlen, int __flags, and abort. */ extern void __libc_fatal (const char *__message) __attribute__ ((__noreturn__)); -_Noreturn void __libc_message (const char *__fnt, ...) attribute_hidden; extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__)); libc_hidden_proto (__fortify_fail) +/* The maximum number of varargs allowed in a __libc_message format string */ +#define LIBC_MESSAGE_MAX_ARGS 4 + +_Noreturn void __libc_message_impl (const char *__fnt, ...) attribute_hidden; + +#define __libc_message0(fmt) \ + __libc_message_impl (fmt) +#define __libc_message1(fmt, a1) \ + __libc_message_impl (fmt, a1) +#define __libc_message2(fmt, a1, a2) \ + __libc_message_impl (fmt, a1, a2) +#define __libc_message3(fmt, a1, a2, a3) \ + __libc_message_impl (fmt, a1, a2, a3) +#define __libc_message4(fmt, a1, a2, a3, a4) \ + __libc_message_impl (fmt, a1, a2, a3, a4) + +#define __libc_message_concat_x(a,b) a##b +#define __libc_message_concat(a,b) __libc_message_concat_x (a, b) + +#define __libc_message_nargs_x(a0,a1,a2,a3,a4,a5,a6,...) a6 +#define __libc_message_nargs(b, ...) \ + __libc_message_nargs_x (__VA_ARGS__,6,5,4,3,2,1,0,) +#define __libc_message_disp(b, ...) \ + __libc_message_concat (b, __libc_message_nargs (__VA_ARGS__))(__VA_ARGS__) +#define __libc_message(...) \ + __libc_message_disp (__libc_message, __VA_ARGS__) + /* Acquire ownership of STREAM. */ extern void __flockfile (FILE *__stream) attribute_hidden; diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c index 16929addab..0816dd7752 100644 --- a/sysdeps/posix/libc_fatal.c +++ b/sysdeps/posix/libc_fatal.c @@ -45,25 +45,12 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total) } #endif -/* The maximum number of varargs allowed in a __libc_message format string */ -#define MAX_NLIST 10 - -struct str_list -{ - const char *str; - size_t len; - struct str_list *next; -}; - /* Abort with an error message. */ void -__libc_message (const char *fmt, ...) +__libc_message_impl (const char *fmt, ...) { va_list ap; int fd = -1; - struct str_list _newp[MAX_NLIST]; - - va_start (ap, fmt); #ifdef FATAL_PREPARE FATAL_PREPARE; @@ -72,10 +59,11 @@ __libc_message (const char *fmt, ...) if (fd == -1) fd = STDERR_FILENO; - struct str_list *list = NULL; - int nlist = 0; - struct iovec iov[MAX_NLIST]; + struct iovec iov[LIBC_MESSAGE_MAX_ARGS]; + int iovcnt = 0; + ssize_t total = 0; + va_start (ap, fmt); const char *cp = fmt; while (*cp != '\0') { @@ -105,29 +93,16 @@ __libc_message (const char *fmt, ...) cp = next; } - struct str_list *newp = &_newp[nlist]; - newp->str = str; - newp->len = len; - newp->next = list; - list = newp; - ++nlist; - if (nlist > MAX_NLIST) - goto fail_out; + iov[iovcnt].iov_base = (char *) str; + iov[iovcnt].iov_len = len; + total += len; + iovcnt++; } + va_end (ap); - if (nlist > 0) + if (iovcnt > 0) { - ssize_t total = 0; - - for (int cnt = nlist - 1; cnt >= 0; --cnt) - { - iov[cnt].iov_base = (char *) list->str; - iov[cnt].iov_len = list->len; - total += list->len; - list = list->next; - } - - WRITEV_FOR_FATAL (fd, iov, nlist, total); + WRITEV_FOR_FATAL (fd, iov, iovcnt, total); total = (total + 1 + GLRO(dl_pagesize) - 1) & ~(GLRO(dl_pagesize) - 1); struct abort_msg_s *buf = __mmap (NULL, total, @@ -137,7 +112,7 @@ __libc_message (const char *fmt, ...) { buf->size = total; char *wp = buf->msg; - for (int cnt = 0; cnt < nlist; ++cnt) + for (int cnt = 0; cnt < iovcnt; ++cnt) wp = mempcpy (wp, iov[cnt].iov_base, iov[cnt].iov_len); *wp = '\0'; @@ -150,9 +125,6 @@ __libc_message (const char *fmt, ...) } } - va_end (ap); - -fail_out: /* Kill the application. */ abort (); }