From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0C9C93858D3C for ; Tue, 20 Feb 2024 22:06:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0C9C93858D3C 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 0C9C93858D3C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708466779; cv=none; b=p5z+/xrYWrzGESVWJ9cTjScqbj1nNBr8mUPMxwM/w8aCvJcEpdTC3YPhyXYDXppXG6qkhsfxHOXwREgEax6Py0cfVPtSlCAHmA3ivZJLOqq8WDraVT+gpb5LhJfqoVM6euC4xIWk0x/BMvMs4tlvShCEhHzhUfREmQR2EBDNwjc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708466779; c=relaxed/simple; bh=NG/lw6aglAaOIfcAeSDWzYVBW+zTZXV4apNbjDp95J4=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=gFKEpgInMDWLANeJxTKuDOB+SSdnMcBQTWWodb16qP606qlsPz4xSeHvhG4mdziTUprvPMjW4YLkRlcQe2zPZ71P3u4LvKQLng1c3895gnCwKgrcUeTWdv1tbVkLQBWrfJK7wcKCiL+fV4+IrFnTbtYd0R9Zy7sCjXNpy2TjFbc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708466775; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7wtDcoXkdIu/nb2RtPbQYU1dIua/JDtjX1i4tpWa7Ws=; b=eoCUn2qcriOKXsy7Luggll4acjlv0gJS1JQqh6KHCEHH11TLL3NjY1ReMa5aGdpvegH5k8 FG5zWz3ivTbCHie82euYOl3eo2B7LWqr9cKiA7VkNdhNG+ccevctRoptiruaGNtBycQkvu 9o/rS3/0mdpxcQu0vAXPhehhbRCe69w= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-255-r15m6gLUP8eTluCRByZaYg-1; Tue, 20 Feb 2024 17:06:14 -0500 X-MC-Unique: r15m6gLUP8eTluCRByZaYg-1 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-42c7a807fd0so39423081cf.1 for ; Tue, 20 Feb 2024 14:06:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708466773; x=1709071573; h=content-transfer-encoding:in-reply-to:organization:from:references :cc: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=7wtDcoXkdIu/nb2RtPbQYU1dIua/JDtjX1i4tpWa7Ws=; b=fxrCl8hIBLHFXi8aiJRa16Vvkx7aZTBODT26pIfsG7et+XISg8IJ+NRsL5UOhnqC/7 mfOfIzA7u5ECm7Srb3GKcwcixTMqZW82r8Thj+i8X907lxwY9Afr/JCuzJsGTAMaQ1gg 1Pab4PmWgdcmg5kMKkGlrn1GrsnYusfW7xtgE6PKkhALHZxW/hNhwt9eKHM6jjm+oR5Q Z1JhQoUASuaT9WBzbvCNXGt3nGfF07pQar/ypFrzUuVhCUg2ztvp3EAbhkoxC13Yvo9V XqMw828eZXkwmMej0Qv1uJ/+0hTZRCAv3skLMquzLf9YvlM4jrR1OpIycAvoD3sVrxjA EP2g== X-Forwarded-Encrypted: i=1; AJvYcCUHUdZebb0d2H43efG2BBS9TSHZbt9qwTRkNCsjLxoCsZllzZJGDNK5J27lI+i1YCDEbkU2OlJ9DvY9z+8hWfomYMLVGmT8iVkj X-Gm-Message-State: AOJu0YzYLY6idSOAcxYo+YY+IbQmstS/vkHb5ZhUpsgSACCap5+YkrnJ 6X93oQKbOCmA0AOlgM1LSWCAxYaY3GqgJaq7B05ZHRzjEakzNYgp7QFRmVmD2lHfSdZBgfHrxOO en1I1DuQK2x8iLlW2taZur3Ocsm+Bp5SNY7tDDla5Zd6lFriDPYUCPzKWAQ== X-Received: by 2002:a05:622a:1ba5:b0:42b:eb89:87c4 with SMTP id bp37-20020a05622a1ba500b0042beb8987c4mr18338897qtb.33.1708466773611; Tue, 20 Feb 2024 14:06:13 -0800 (PST) X-Google-Smtp-Source: AGHT+IE4MMVfY94YUfyavVVTiGqoL3Q0dL35Y1JnxDM18DXCDozejlAGS59lxJR0tiKhczwZKTAE3A== X-Received: by 2002:a05:622a:1ba5:b0:42b:eb89:87c4 with SMTP id bp37-20020a05622a1ba500b0042beb8987c4mr18338878qtb.33.1708466773210; Tue, 20 Feb 2024 14:06:13 -0800 (PST) Received: from [192.168.0.241] ([198.48.244.52]) by smtp.gmail.com with ESMTPSA id i11-20020ac871cb000000b0042dfb466c85sm3200509qtp.64.2024.02.20.14.06.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Feb 2024 14:06:12 -0800 (PST) Message-ID: <9c3980fd-692f-4a06-95b5-3c5650e29497@redhat.com> Date: Tue, 20 Feb 2024 17:06:12 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 02/10] libio: Improve fortify with clang To: Adhemerval Zanella , libc-alpha@sourceware.org Cc: Siddhesh Poyarekar References: <20240208184622.332678-1-adhemerval.zanella@linaro.org> <20240208184622.332678-3-adhemerval.zanella@linaro.org> From: Carlos O'Donell Organization: Red Hat In-Reply-To: <20240208184622.332678-3-adhemerval.zanella@linaro.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 List-Id: On 2/8/24 13:46, Adhemerval Zanella wrote: > It improve fortify checks for sprintf, vsprintf, vsnsprintf, fprintf, > dprintf, asprintf, __asprintf, obstack_printf, gets, fgets, > fgets_unlocked, fread, and fread_unlocked. The runtime checks have > similar support coverage as with GCC. LGTM. Tested on x86_64 and i686 with gcc. Reviewed-by: Carlos O'Donell Tested-by: Carlos O'Donell > For function with variadic argument (sprintf, snprintf, fprintf, printf, > dprintf, asprintf, __asprintf, obstack_printf) the fortify wrapper calls > the va_arg version since clang does not support __va_arg_pack. > > Checked on aarch64, armhf, x86_64, and i686. > --- > libio/bits/stdio2.h | 173 +++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 153 insertions(+), 20 deletions(-) > > diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h > index f9e8d37610..91a80dd7c6 100644 > --- a/libio/bits/stdio2.h > +++ b/libio/bits/stdio2.h > @@ -31,15 +31,29 @@ __NTH (sprintf (char *__restrict __s, const char *__restrict __fmt, ...)) > __glibc_objsize (__s), __fmt, > __va_arg_pack ()); > } > +#elif __fortify_use_clang > +/* clang does not have __va_arg_pack, so defer to va_arg version. */ > +__fortify_function_error_function __attribute_overloadable__ int > +__NTH (sprintf (__fortify_clang_overload_arg (char *, __restrict, __s), > + const char *__restrict __fmt, ...)) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __builtin___vsprintf_chk (__s, __USE_FORTIFY_LEVEL - 1, > + __glibc_objsize (__s), __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > #elif !defined __cplusplus > # define sprintf(str, ...) \ > __builtin___sprintf_chk (str, __USE_FORTIFY_LEVEL - 1, \ > __glibc_objsize (str), __VA_ARGS__) > #endif > > -__fortify_function int > -__NTH (vsprintf (char *__restrict __s, const char *__restrict __fmt, > - __gnuc_va_list __ap)) > +__fortify_function __attribute_overloadable__ int > +__NTH (vsprintf (__fortify_clang_overload_arg (char *, __restrict, __s), > + const char *__restrict __fmt, __gnuc_va_list __ap)) > { > return __builtin___vsprintf_chk (__s, __USE_FORTIFY_LEVEL - 1, > __glibc_objsize (__s), __fmt, __ap); > @@ -55,15 +69,33 @@ __NTH (snprintf (char *__restrict __s, size_t __n, > __glibc_objsize (__s), __fmt, > __va_arg_pack ()); > } > +# elif __fortify_use_clang > +/* clang does not have __va_arg_pack, so defer to va_arg version. */ > +__fortify_function_error_function __attribute_overloadable__ int > +__NTH (snprintf (__fortify_clang_overload_arg (char *, __restrict, __s), > + size_t __n, const char *__restrict __fmt, ...)) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1, > + __glibc_objsize (__s), __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > # elif !defined __cplusplus > # define snprintf(str, len, ...) \ > __builtin___snprintf_chk (str, len, __USE_FORTIFY_LEVEL - 1, \ > __glibc_objsize (str), __VA_ARGS__) > # endif > > -__fortify_function int > -__NTH (vsnprintf (char *__restrict __s, size_t __n, > - const char *__restrict __fmt, __gnuc_va_list __ap)) > +__fortify_function __attribute_overloadable__ int > +__NTH (vsnprintf (__fortify_clang_overload_arg (char *, __restrict, __s), > + size_t __n, const char *__restrict __fmt, > + __gnuc_va_list __ap)) > + __fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s), > + "call to vsnprintf may overflow the destination " > + "buffer") OK. > { > return __builtin___vsnprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1, > __glibc_objsize (__s), __fmt, __ap); > @@ -85,6 +117,30 @@ printf (const char *__restrict __fmt, ...) > { > return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ()); > } > +# elif __fortify_use_clang > +/* clang does not have __va_arg_pack, so defer to va_arg version. */ > +__fortify_function_error_function __attribute_overloadable__ __nonnull ((1)) int > +fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream), > + const char *__restrict __fmt, ...) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __builtin___vfprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, > + __fmt, __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > + > +__fortify_function_error_function __attribute_overloadable__ int > +printf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), ...) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __builtin___vprintf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > # elif !defined __cplusplus > # define printf(...) \ > __printf_chk (__USE_FORTIFY_LEVEL - 1, __VA_ARGS__) > @@ -92,8 +148,9 @@ printf (const char *__restrict __fmt, ...) > __fprintf_chk (stream, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__) > # endif > > -__fortify_function int > -vprintf (const char *__restrict __fmt, __gnuc_va_list __ap) > +__fortify_function __attribute_overloadable__ int > +vprintf (__fortify_clang_overload_arg (const char *, __restrict, __fmt), > + __gnuc_va_list __ap) > { > #ifdef __USE_EXTERN_INLINES > return __vfprintf_chk (stdout, __USE_FORTIFY_LEVEL - 1, __fmt, __ap); > @@ -117,6 +174,18 @@ dprintf (int __fd, const char *__restrict __fmt, ...) > return __dprintf_chk (__fd, __USE_FORTIFY_LEVEL - 1, __fmt, > __va_arg_pack ()); > } > +# elif __fortify_use_clang > +__fortify_function_error_function __attribute_overloadable__ int > +dprintf (int __fd, __fortify_clang_overload_arg (const char *, __restrict, > + __fmt), ...) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __vdprintf_chk (__fd, __USE_FORTIFY_LEVEL - 1, __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > # elif !defined __cplusplus > # define dprintf(fd, ...) \ > __dprintf_chk (fd, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__) > @@ -153,6 +222,43 @@ __NTH (obstack_printf (struct obstack *__restrict __obstack, > return __obstack_printf_chk (__obstack, __USE_FORTIFY_LEVEL - 1, __fmt, > __va_arg_pack ()); > } > +# elif __fortify_use_clang > +__fortify_function_error_function __attribute_overloadable__ int > +__NTH (asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr), > + const char *__restrict __fmt, ...)) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __vasprintf_chk (__ptr, __USE_FORTIFY_LEVEL - 1, __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > + > +__fortify_function_error_function __attribute_overloadable__ int > +__NTH (__asprintf (__fortify_clang_overload_arg (char **, __restrict, __ptr), > + const char *__restrict __fmt, ...)) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __vasprintf_chk (__ptr, __USE_FORTIFY_LEVEL - 1, __fmt, > + __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > + > +__fortify_function_error_function __attribute_overloadable__ int > +__NTH (obstack_printf (__fortify_clang_overload_arg (struct obstack *, > + __restrict, __obstack), > + const char *__restrict __fmt, ...)) > +{ > + __gnuc_va_list __fortify_ap; > + __builtin_va_start (__fortify_ap, __fmt); > + int __r = __obstack_vprintf_chk (__obstack, __USE_FORTIFY_LEVEL - 1, > + __fmt, __fortify_ap); > + __builtin_va_end (__fortify_ap); > + return __r; > +} > # elif !defined __cplusplus > # define asprintf(ptr, ...) \ > __asprintf_chk (ptr, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__) > @@ -182,8 +288,11 @@ __NTH (obstack_vprintf (struct obstack *__restrict __obstack, > #endif > > #if __GLIBC_USE (DEPRECATED_GETS) > -__fortify_function __wur char * > -gets (char *__str) > +__fortify_function __wur __attribute_overloadable__ char * > +gets (__fortify_clang_overload_arg (char *, , __str)) > + __fortify_clang_warning (__glibc_objsize (__str) == (size_t) -1, > + "please use fgets or getline instead, gets " > + "can not specify buffer size") > { > if (__glibc_objsize (__str) != (size_t) -1) > return __gets_chk (__str, __glibc_objsize (__str)); > @@ -192,48 +301,70 @@ gets (char *__str) > #endif > > __fortify_function __wur __fortified_attr_access (__write_only__, 1, 2) > -__nonnull ((3)) char * > -fgets (char *__restrict __s, int __n, FILE *__restrict __stream) > +__nonnull ((3)) __attribute_overloadable__ char * > +fgets (__fortify_clang_overload_arg (char *, __restrict, __s), int __n, > + FILE *__restrict __stream) > + __fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s) && __n > 0, > + "fgets called with bigger size than length of " > + "destination buffer") > { > size_t sz = __glibc_objsize (__s); > if (__glibc_safe_or_unknown_len (__n, sizeof (char), sz)) > return __fgets_alias (__s, __n, __stream); > +#if !__fortify_use_clang > if (__glibc_unsafe_len (__n, sizeof (char), sz)) > return __fgets_chk_warn (__s, sz, __n, __stream); > +#endif > return __fgets_chk (__s, sz, __n, __stream); > } > > -__fortify_function __wur __nonnull ((4)) size_t > -fread (void *__restrict __ptr, size_t __size, size_t __n, > - FILE *__restrict __stream) > +__fortify_function __wur __nonnull ((4)) __attribute_overloadable__ size_t > +fread (__fortify_clang_overload_arg (void *, __restrict, __ptr), > + size_t __size, size_t __n, FILE *__restrict __stream) > + __fortify_clang_warning (__fortify_clang_bos0_static_lt (__size * __n, __ptr) > + && !__fortify_clang_mul_may_overflow (__size, __n), > + "fread called with bigger size * n than length " > + "of destination buffer") > { > size_t sz = __glibc_objsize0 (__ptr); > if (__glibc_safe_or_unknown_len (__n, __size, sz)) > return __fread_alias (__ptr, __size, __n, __stream); > +#if !__fortify_use_clang > if (__glibc_unsafe_len (__n, __size, sz)) > return __fread_chk_warn (__ptr, sz, __size, __n, __stream); > +#endif > return __fread_chk (__ptr, sz, __size, __n, __stream); > } > > #ifdef __USE_GNU > __fortify_function __wur __fortified_attr_access (__write_only__, 1, 2) > -__nonnull ((3)) char * > -fgets_unlocked (char *__restrict __s, int __n, FILE *__restrict __stream) > +__nonnull ((3)) __attribute_overloadable__ char * > +fgets_unlocked (__fortify_clang_overload_arg (char *, __restrict, __s), > + int __n, FILE *__restrict __stream) > + __fortify_clang_warning (__fortify_clang_bos_static_lt (__n, __s) && __n > 0, > + "fgets called with bigger size than length of " > + "destination buffer") > { > size_t sz = __glibc_objsize (__s); > if (__glibc_safe_or_unknown_len (__n, sizeof (char), sz)) > return __fgets_unlocked_alias (__s, __n, __stream); > +#if !__fortify_use_clang > if (__glibc_unsafe_len (__n, sizeof (char), sz)) > return __fgets_unlocked_chk_warn (__s, sz, __n, __stream); > +#endif OK. Interesting difference here. > return __fgets_unlocked_chk (__s, sz, __n, __stream); > } > #endif > > #ifdef __USE_MISC > # undef fread_unlocked > -__fortify_function __wur __nonnull ((4)) size_t > -fread_unlocked (void *__restrict __ptr, size_t __size, size_t __n, > - FILE *__restrict __stream) > +__fortify_function __wur __nonnull ((4)) __attribute_overloadable__ size_t > +fread_unlocked (__fortify_clang_overload_arg0 (void *, __restrict, __ptr), > + size_t __size, size_t __n, FILE *__restrict __stream) > + __fortify_clang_warning (__fortify_clang_bos0_static_lt (__size * __n, __ptr) > + && !__fortify_clang_mul_may_overflow (__size, __n), > + "fread_unlocked called with bigger size * n than " > + "length of destination buffer") > { > size_t sz = __glibc_objsize0 (__ptr); > if (__glibc_safe_or_unknown_len (__n, __size, sz)) > @@ -261,8 +392,10 @@ fread_unlocked (void *__restrict __ptr, size_t __size, size_t __n, > # endif > return __fread_unlocked_alias (__ptr, __size, __n, __stream); > } > +# if !__fortify_use_clang > if (__glibc_unsafe_len (__n, __size, sz)) > return __fread_unlocked_chk_warn (__ptr, sz, __size, __n, __stream); > +# endif OK. > return __fread_unlocked_chk (__ptr, sz, __size, __n, __stream); > > } -- Cheers, Carlos.