From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x229.google.com (mail-oi1-x229.google.com [IPv6:2607:f8b0:4864:20::229]) by sourceware.org (Postfix) with ESMTPS id 7ED803858D1E for ; Wed, 2 Aug 2023 19:46:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7ED803858D1E 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-x229.google.com with SMTP id 5614622812f47-3a37909a64eso104860b6e.1 for ; Wed, 02 Aug 2023 12:46:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1691005612; x=1691610412; 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=ljPvmnon0sfVF6gUoqxaQ9/7nnuQdwGsrGjE4e8kZOM=; b=K9Ee0ZJkMhCKwUz7hrgH21CaqvHqc2DINz8Uk5404ohWoWmbEHZmKkmT4N12tk4ZaC A7iJT2zeVfyEQGjQtgm3/7pZZM3HMxCOsQGw7S4nKu+HncULhLmRnk7rrE1WHIpIN1S8 Iyc0ZNEvRhRvi4ndUNYQIWI18G5rN9/85zSgcq6L2IktVnptx7OV5P8TCxpMr2AgieOs Ftzm5DJ4axg3QtQtsuN7EtimaACfJ5aQ+SJhZ3hAV+T5C828SeUTCpP5uGJusgX6U/Hr /KTevP9D/3g44ectNQMXPCPRoSQMpSDoST2a5xrav4E3YRnrzZn1JsiyM8EPRHhmaHqY jnxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691005612; x=1691610412; 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=ljPvmnon0sfVF6gUoqxaQ9/7nnuQdwGsrGjE4e8kZOM=; b=O6jvdWJCtOytIQfkqN6rVXBPfwiaconjWd4xjcaglweQyxNhxjE96l2Riksr8VWcLx UYLaB0JJ/jgOkc8kmaeh/Jpy/x+AIo6QGbMFW8KVzNBK1bDxB1umX8rB9wUajdCbhqLS WhjCp1hLal+IacBva8ZnHGprqS6SbxrtUe3WFb2fMlx1CJnXQrBqth9DGQoLqvjLvYZG VfNmO5y4F5A04dypwlMnzZQ68YWUWRd3vMDJzI9URJT43JnG2aPjRcb71QpcajEJoPaf oKcrUy5g08KaMMxkwFrflTfT3apb2pqQWKz/vFD3g2CEaiaPTl3jLx3KIw38eQfp1SMW TPQA== X-Gm-Message-State: ABy/qLbB4CMZQD9gmRqq9oDHclpa9dDyKjKvXpZiHqt5HAgbCD72KquM yu+bizYvpClIVuwhEx88CzyWOG98JpCN43o5g/OlEA== X-Google-Smtp-Source: APBJJlH8UQbkPPfOi38G30TiKuYX5hJOrNHhUZTLjZcXI8r4NKV6GRvzibZOfFjFeaXO7PIvaVYkYg== X-Received: by 2002:a05:6808:201a:b0:3a7:3702:fe73 with SMTP id q26-20020a056808201a00b003a73702fe73mr12940115oiw.56.1691005612071; Wed, 02 Aug 2023 12:46:52 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:9aa9:84f6:53b3:4cd0:ffe6? ([2804:1b3:a7c1:9aa9:84f6:53b3:4cd0:ffe6]) by smtp.gmail.com with ESMTPSA id n3-20020aca2403000000b003a3fa4a7b5bsm6550941oic.27.2023.08.02.12.46.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 02 Aug 2023 12:46:51 -0700 (PDT) Message-ID: Date: Wed, 2 Aug 2023 16:46:49 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v4 5/6] io: Add FORTIFY_SOURCE check for fcntl arguments Content-Language: en-US To: libc-alpha@sourceware.org, Sergey Bugaev References: <20230730192605.2423480-1-bugaevc@gmail.com> <20230730192605.2423480-6-bugaevc@gmail.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20230730192605.2423480-6-bugaevc@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 30/07/23 16:26, Sergey Bugaev via Libc-alpha wrote: > Both open () and fcntl () are "overloaded" to accept either 2 or 3 > arguments; whether the last argument is required (and, in case of fcntl, > the type of the third argument) depends on the values of the previous > arguments. Since C provides no native support for function overloading, > this is implemented by making these functions vararg. Unfortunately, > this means the compiler is unable to check the number of arguments and > their types for correctness at compile time, and will not diagnose any > mistakes. > > To help with this, when FORTIFY_SOURCE is enabled, the special fcntl2.h > header replaces open () with a wrapper that checks the passed number of > arguments, raising a compile-time or run-time error on mismatch. This > commit adds similar handling for fcntl (). Additionally, the fcntl () > wrapper tries to check that the type of the 3rd argument (if any) > matches the type that the command (passed in the 2nd argument) expects. > > tst-ofdlocks-compat is excluded from fortification, since it > intentionally calls fcntl () with an argument of a wrong type while > linking to an old symbol version. The fcntl () call in test-errno is > similarly excluded from fortification, since it intentionally contains > two errors (bad file descriptor, and missing the required 3rd argument) > and tests for getting the expected error code. > > Signed-off-by: Sergey Bugaev Looks good in general, some minor comments below. > --- > > Reposting the concern from v3: > In manual/maint.texi, how would I properly update the command for > generating the list of fortified functions to include fcntl / fcntl64 in > absence of __fcntl_2? (The next patch does make it work with __fcntl_2.) Maybe a better option would to rename __fcntl_2 to __fcntl_2_chk, since on the script at comments it already filters out such name format. > > debug/tst-fortify.c | 168 +++++++++++ > include/bits/fcntl3.h | 1 + > io/Makefile | 3 + > io/bits/fcntl3.h | 471 +++++++++++++++++++++++++++++++ > io/fcntl.h | 6 + > manual/maint.texi | 9 +- > posix/test-errno.c | 4 +- > sysdeps/unix/sysv/linux/Makefile | 4 + > 8 files changed, 663 insertions(+), 3 deletions(-) > create mode 100644 include/bits/fcntl3.h > create mode 100644 io/bits/fcntl3.h > > diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c > index 3744aada..98654299 100644 > --- a/debug/tst-fortify.c > +++ b/debug/tst-fortify.c > @@ -36,6 +36,8 @@ > #include > #include > #include > +#include > +#include > > #ifndef _GNU_SOURCE > # define MEMPCPY memcpy > @@ -78,6 +80,15 @@ do_prepare (void) > } > } > > +/* Return VALUE, but do it in a way that the compiler cannot > + see that it's a compile-time constant. */ > +static inline int > +hide_constant (int value) > +{ > + asm ("" : "+rm" (value)); > + return value; > +} > + > volatile int chk_fail_ok; > volatile int ret; > jmp_buf chk_fail_buf; > @@ -1811,6 +1822,163 @@ do_test (void) > ppoll (fds, l0 + 2, NULL, NULL); > CHK_FAIL_END > # endif > +#endif > + > + /* Check that we can do basic fcntl operations, both ones that require > + the third argument, and ones that do not. */ > + res = fcntl (STDIN_FILENO, F_GETFD); > + TEST_COMPARE (res, 0); > + res = fcntl (STDIN_FILENO, F_SETFD, 0); > + TEST_COMPARE (res, 0); > + > + /* Check that we can successfully use F_GETLK with struct flock, even though > + F_GETLK might have the same value as F_GETLK64. */ > + { > + struct flock flock; > + memset (&flock, 0, sizeof (flock)); > + flock.l_type = F_WRLCK; > + flock.l_whence = SEEK_SET; > + flock.l_start = 1234; > + flock.l_len = 5678; > + flock.l_pid = 0; > + > + (void) fcntl (STDIN_FILENO, F_GETLK, &flock); > + } > + > +#ifdef F_OFD_GETLK > + /* Check for confusion between 32- and 64-bit versions of the fcntl > + interface. */ > + int lockfd1 = xopen (temp_filename, O_RDWR, 0); > + int lockfd2 = xopen (temp_filename, O_RDWR, 0); > + > + int ofd_locks_supported = support_fcntl_support_ofd_locks (lockfd1); > + > + if (ofd_locks_supported) > + { > + struct flock flock; > + > + memset (&flock, 0, sizeof (flock)); > + flock.l_type = F_WRLCK; > + flock.l_whence = SEEK_SET; > + flock.l_start = 1234; > + flock.l_len = 5678; > + flock.l_pid = 0; > + > + res = fcntl (lockfd1, F_OFD_SETLK, &flock); > + TEST_COMPARE (res, 0); > + > + memset (&flock, 0, sizeof (flock)); > + flock.l_type = F_RDLCK; > + flock.l_whence = SEEK_SET; > + flock.l_start = 3542; > + flock.l_len = 411; > + flock.l_pid = 0; > + > + res = fcntl (lockfd2, F_OFD_GETLK, &flock); > + TEST_COMPARE (res, 0); > + /* Check that we get the expected values. */ > + TEST_COMPARE (flock.l_type, F_WRLCK); > + TEST_COMPARE (flock.l_whence, SEEK_SET); > + TEST_COMPARE (flock.l_start, 1234); > + TEST_COMPARE (flock.l_len, 5678); > + TEST_COMPARE (flock.l_pid, -1); > + } > +#endif > + > + /* Check that we can do fcntl operations with CMD that is not constant > + at compile time. */ > + res = fcntl (STDIN_FILENO, hide_constant (F_GETFD)); > + TEST_COMPARE (res, 0); > + res = fcntl (STDIN_FILENO, hide_constant (F_SETFD), 0); > + TEST_COMPARE (res, 0); > + > +#ifdef F_OFD_GETLK > + if (ofd_locks_supported) > + { > + struct flock flock; > + > + memset (&flock, 0, sizeof (flock)); > + flock.l_type = F_RDLCK; > + flock.l_whence = SEEK_SET; > + flock.l_start = 3542; > + flock.l_len = 411; > + flock.l_pid = 0; > + > + res = fcntl (lockfd2, hide_constant (F_OFD_GETLK), &flock); > + TEST_COMPARE (res, 0); > + /* Check that we get the expected values. */ > + TEST_COMPARE (flock.l_type, F_WRLCK); > + TEST_COMPARE (flock.l_whence, SEEK_SET); > + TEST_COMPARE (flock.l_start, 1234); > + TEST_COMPARE (flock.l_len, 5678); > + TEST_COMPARE (flock.l_pid, -1); > + } > +#endif > + > +#if defined (__USE_LARGEFILE64) || defined (__USE_TIME_BITS64) > + /* Also check fcntl64 (). */ > + res = fcntl64 (STDIN_FILENO, F_GETFD); > + TEST_COMPARE (res, 0); > + res = fcntl64 (STDIN_FILENO, F_SETFD, 0); > + TEST_COMPARE (res, 0); > + res = fcntl64 (STDIN_FILENO, hide_constant (F_GETFD)); > + TEST_COMPARE (res, 0); > + res = fcntl64 (STDIN_FILENO, hide_constant (F_SETFD), 0); > + TEST_COMPARE (res, 0); > + > + /* Check that we can successfully use F_GETLK64 with struct flock64, even > + though F_GETLK might have the same value as F_GETLK64. */ > + { > + struct flock64 flock64; > + memset (&flock64, 0, sizeof (flock64)); > + flock64.l_type = F_WRLCK; > + flock64.l_whence = SEEK_SET; > + flock64.l_start = 1234; > + flock64.l_len = 5678; > + flock64.l_pid = 0; > + > + (void) fcntl64 (STDIN_FILENO, F_GETLK64, &flock64); > + } > + > +#ifdef F_OFD_GETLK Add a whitespace for the identation: # ifdef F_OFD_GETLK Same for the #endif. > + if (ofd_locks_supported) > + { > + struct flock64 flock64; > + > + memset (&flock64, 0, sizeof (flock64)); > + flock64.l_type = F_RDLCK; > + flock64.l_whence = SEEK_SET; > + flock64.l_start = 3542; > + flock64.l_len = 411; > + flock64.l_pid = 0; > + > + res = fcntl64 (lockfd2, F_OFD_GETLK, &flock64); > + TEST_COMPARE (res, 0); > + /* Check that we get the expected values. */ > + TEST_COMPARE (flock64.l_type, F_WRLCK); > + TEST_COMPARE (flock64.l_whence, SEEK_SET); > + TEST_COMPARE (flock64.l_start, 1234); > + TEST_COMPARE (flock64.l_len, 5678); > + TEST_COMPARE (flock64.l_pid, -1); > + > + memset (&flock64, 0, sizeof (flock64)); > + flock64.l_type = F_RDLCK; > + flock64.l_whence = SEEK_SET; > + flock64.l_start = 3542; > + flock64.l_len = 411; > + flock64.l_pid = 0; > + > + res = fcntl64 (lockfd2, hide_constant (F_OFD_GETLK), &flock64); > + TEST_COMPARE (res, 0); > + /* Check that we get the expected values. */ > + TEST_COMPARE (flock64.l_type, F_WRLCK); > + TEST_COMPARE (flock64.l_whence, SEEK_SET); > + TEST_COMPARE (flock64.l_start, 1234); > + TEST_COMPARE (flock64.l_len, 5678); > + TEST_COMPARE (flock64.l_pid, -1); > + } > +#endif > + > #endif > > return ret; > diff --git a/include/bits/fcntl3.h b/include/bits/fcntl3.h > new file mode 100644 > index 00000000..d31154f6 > --- /dev/null > +++ b/include/bits/fcntl3.h > @@ -0,0 +1 @@ > +#include "../../io/bits/fcntl3.h" I think it would be clear that se use absolute paths instead of relative. > diff --git a/io/Makefile b/io/Makefile > index 6ccc0e86..0c43881f 100644 > --- a/io/Makefile > +++ b/io/Makefile > @@ -25,6 +25,7 @@ include ../Makeconfig > headers := \ > bits/fcntl.h \ > bits/fcntl2.h \ > + bits/fcntl3.h \ > bits/poll.h \ > bits/poll2.h \ > bits/stat.h \ > @@ -153,6 +154,8 @@ routines := \ > routines_no_fortify += \ > getcwd \ > getwd \ > + fcntl \ > + fcntl64 \ > open \ > open64 \ > openat \ > diff --git a/io/bits/fcntl3.h b/io/bits/fcntl3.h > new file mode 100644 > index 00000000..9016d37e > --- /dev/null > +++ b/io/bits/fcntl3.h > @@ -0,0 +1,471 @@ > +/* Checking macros for fcntl functions. > + Copyright (C) 2007-2023 Free Software Foundation, Inc. I think this should be only 2023. > + 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 > + . */ > + > +#ifndef _FCNTL_H > +# error "Never include directly; use instead." > +#endif > + > +#ifndef __USE_TIME_BITS64 > + > +# ifndef __USE_FILE_OFFSET64 > +extern int __REDIRECT (__fcntl_alias, (int __fd, int __cmd, ...), fcntl); > +extern int __REDIRECT (__fcntl_warn, (int __fd, int __cmd, ...), fcntl) > + __warnattr ("fcntl argument has wrong type for this command"); > +# else > +extern int __REDIRECT (__fcntl_alias, (int __fd, int __cmd, ...), fcntl64); > +extern int __REDIRECT (__fcntl_warn, (int __fd, int __cmd, ...), fcntl64) > + __warnattr ("fcntl argument has wrong type for this command"); > +# endif /* __USE_FILE_OFFSET64 */ > + > +# ifdef __USE_LARGEFILE64 > +extern int __REDIRECT (__fcntl64_alias, (int __fd, int __cmd, ...), fcntl64); > +extern int __REDIRECT (__fcntl64_warn, (int __fd, int __cmd, ...), fcntl64) > + __warnattr ("fcntl64 argument has wrong type for this command"); > +# endif > + > +#else /* __USE_TIME_BITS64 */ > + > +extern int __REDIRECT_NTH (__fcntl_alias, (int __fd, int __cmd, ...), > + __fcntl_time64); > +extern int __REDIRECT_NTH (__fcntl64_alias, (int __fd, int __cmd, ...), > + __fcntl_time64); > +extern int __REDIRECT (__fcntl_warn, (int __fd, int __cmd, ...), > + __fcntl_time64) > + __warnattr ("fcntl argument has wrong type for this command"); > +extern int __REDIRECT (__fcntl64_warn, (int __fd, int __cmd, ...), > + __fcntl_time64) > + __warnattr ("fcntl64 argument has wrong type for this command"); > + > +#endif /* __USE_TIME_BITS64 */ > + > + > +/* Whether the fcntl CMD is known to require an argument. */ > +__extern_always_inline int > +__fcntl_requires_arg (int __cmd) > +{ > + switch (__cmd) > + { > + case F_DUPFD: > +#ifdef F_DUPFD_CLOEXEC > + case F_DUPFD_CLOEXEC: > +#endif > + case F_SETFD: > + case F_SETFL: > +#ifdef F_SETLK > + case F_SETLK: > + case F_SETLKW: > + case F_GETLK: > +#endif > +#ifdef F_OFD_SETLK > + case F_OFD_SETLK: > + case F_OFD_SETLKW: > + case F_OFD_GETLK: > +#endif > +#ifdef F_SETOWN > + case F_SETOWN: > +#endif > +#ifdef F_GETOWN_EX > + case F_GETOWN_EX: > + case F_SETOWN_EX: > + case F_SETSIG: > +#endif > +#ifdef F_SETLEASE > + case F_SETLEASE: > + case F_NOTIFY: > + case F_SETPIPE_SZ: I don't think assuring that F_SETLEASE is defined F_ADD_SEALS is also defined. The F_SETLEASE and F_NOTIFY was added on Linux 2.4, and F_SETPIPE_SZ on 2.6.35; however F_ADD_SEALS was added on 3.17 and we stil support 3.2 as the minimum version for some architectures. So I think it would be better to check for '#ifdef F_ADD_SEALS' instead. > + case F_ADD_SEALS: > + case F_GET_RW_HINT: > + case F_SET_RW_HINT: Same for F_GET_RW_HINT, that was added on 4.13. > + case F_GET_FILE_RW_HINT: > + case F_SET_FILE_RW_HINT: > +#endif > + return 1; > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires an int argument. */ > +__extern_always_inline int > +__fcntl_is_int (int __cmd) > +{ > + switch (__cmd) > + { > + case F_DUPFD: > +#ifdef F_DUPFD_CLOEXEC > + case F_DUPFD_CLOEXEC: > +#endif > + case F_SETFD: > + case F_SETFL: > +#ifdef F_SETOWN > + case F_SETOWN: > +#endif > +#ifdef F_SETSIG > + case F_SETSIG: > +#endif > +#ifdef F_SETLEASE > + case F_SETLEASE: > + case F_NOTIFY: > + case F_SETPIPE_SZ: > + case F_ADD_SEALS: Same as before. > +#endif > + return 1; > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (const uint64_t *) argument. */ > +__extern_always_inline int > +__fcntl_is_const_uint64_t_ptr (int __cmd) > +{ > + switch (__cmd) > + { > +#ifdef F_SET_RW_HINT > + case F_SET_RW_HINT: > + case F_SET_FILE_RW_HINT: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > + > +/* Whether the fcntl CMD requires an (uint64_t *) argument. */ > +__extern_always_inline int > +__fcntl_is_uint64_t_ptr (int __cmd) > +{ > + switch (__cmd) > + { > +#ifdef F_GET_RW_HINT > + case F_GET_RW_HINT: > + case F_GET_FILE_RW_HINT: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (const struct f_owner_ex *) argument. */ > +__extern_always_inline int > +__fcntl_is_const_fowner_ex (int __cmd) > +{ > + switch (__cmd) > + { > +#ifdef F_SETOWN_EX > + case F_SETOWN_EX: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (struct f_owner_ex *) argument. */ > +__extern_always_inline int > +__fcntl_is_fowner_ex (int __cmd) > +{ > + switch (__cmd) > + { > +#ifdef F_GETOWN_EX > + case F_GETOWN_EX: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires either a (const struct flock *) argument > + or a (const struct flock64 *) argument. */ > +__extern_always_inline int > +__fcntl_is_const_flock_or_64 (int __cmd) > +{ > + switch (__cmd) > + { > +#if defined (F_SETLK) && defined (F_SETLK64) && F_SETLK == F_SETLK64 > + case F_SETLK: > + case F_SETLKW: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (const struct flock *) argument. */ > +__extern_always_inline int > +__fcntl_is_const_flock (int __cmd, int __is_fcntl64) > +{ > + (void) __is_fcntl64; > + switch (__cmd) > + { > +#ifdef F_SETLK > + case F_SETLK: > + case F_SETLKW: > + return 1; > +#endif > + > +#ifdef F_OFD_SETLK > + case F_OFD_SETLK: > + case F_OFD_SETLKW: > + return !__is_fcntl64; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires either a (struct flock *) argument > + or a (struct flock64 *) argument. */ > +__extern_always_inline int > +__fcntl_is_flock_or_64 (int __cmd) > +{ > + switch (__cmd) > + { > +#if defined (F_GETLK) && defined (F_GETLK64) && F_GETLK == F_GETLK64 > + case F_GETLK: > + return 1; > +#endif > + > + default: > + return 0; > + } > +} > + > + > +/* Whether the fcntl CMD requires a (struct flock *) argument. */ > +__extern_always_inline int > +__fcntl_is_flock (int __cmd, int __is_fcntl64) > +{ > + (void) __is_fcntl64; > + switch (__cmd) > + { > +#ifdef F_GETLK > + case F_GETLK: > + return 1; > +#endif > + > +#ifdef F_OFD_GETLK > + case F_OFD_GETLK: > + return !__is_fcntl64; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (const struct flock64 *) argument. */ > +__extern_always_inline int > +__fcntl_is_const_flock64 (int __cmd, int __is_fcntl64) > +{ > + (void) __is_fcntl64; > + switch (__cmd) > + { > +#ifdef F_SETLK64 > + case F_SETLK64: > + case F_SETLKW64: > + return 1; > +#endif > + > +#ifdef F_OFD_SETLK > + case F_OFD_SETLK: > + case F_OFD_SETLKW: > + return __is_fcntl64; > +#endif > + > + default: > + return 0; > + } > +} > + > +/* Whether the fcntl CMD requires a (struct flock64 *) argument. */ > +__extern_always_inline int > +__fcntl_is_flock64 (int __cmd, int __is_fcntl64) > +{ > + (void) __is_fcntl64; > + switch (__cmd) > + { > +#ifdef F_GETLK64 > + case F_GETLK64: > + return 1; > +#endif > + > +#ifdef F_OFD_GETLK > + case F_OFD_GETLK: > + return __is_fcntl64; > +#endif > + > + default: > + return 0; > + } > +} > + > +#ifndef __cplusplus > + > +# define __fcntl_types_compatible(arg, type) \ > + __builtin_types_compatible_p (__typeof (arg), type) > + > +#else > + > +extern "C++" { > + > +template > +struct __fcntl_types_compatible_helper > +{ > + __always_inline static int > + __compatible () > + { > + return 0; > + } > +}; > + > +template > +struct __fcntl_types_compatible_helper<__T, __T> > +{ > + __always_inline static int > + __compatible () > + { > + return 1; > + } > +}; > + > +# define __fcntl_types_compatible(arg, type) \ > + __fcntl_types_compatible_helper<__typeof (arg), type>::__compatible () > + > +} /* extern C++ */ > + > +#endif /* __cplusplus */ > + > +#define __fcntl_type_check_int(arg) __fcntl_types_compatible (arg, int) > + > +#define __fcntl_type_check_const_uint64_t_ptr(arg) \ > + (__fcntl_types_compatible (arg, const __uint64_t *) \ > + || __fcntl_types_compatible (arg, __uint64_t *)) > + > +#define __fcntl_type_check_uint64_t_ptr(arg) \ > + __fcntl_types_compatible (arg, __uint64_t *) > + > +#define __fcntl_type_check_const_fowner_ex(arg) \ > + (__fcntl_types_compatible (arg, const struct f_owner_ex *) \ > + || __fcntl_types_compatible (arg, struct f_owner_ex *)) > + > +#define __fcntl_type_check_fowner_ex(arg) \ > + __fcntl_types_compatible (arg, struct f_owner_ex *) > + > +#define __fcntl_type_check_const_flock(arg) \ > + (__fcntl_types_compatible (arg, const struct flock *) \ > + || __fcntl_types_compatible (arg, struct flock *)) > + > +#define __fcntl_type_check_flock(arg) \ > + __fcntl_types_compatible (arg, struct flock *) > + > +#ifdef __USE_LARGEFILE64 > + > +# define __fcntl_type_check_const_flock64(arg) \ > + (__fcntl_types_compatible (arg, const struct flock64 *) \ > + || __fcntl_types_compatible (arg, struct flock64 *)) > + > +# define __fcntl_type_check_flock64(arg) \ > + __fcntl_types_compatible (arg, struct flock64 *) > + > +#else > + > +# define __fcntl_type_check_const_flock64(arg) 0 > +# define __fcntl_type_check_flock64(arg) 0 > + > +#endif /* __USE_LARGEFILE64 */ > + > +#define __fcntl_type_check_const_flock_or_64(arg) \ > + (__fcntl_type_check_const_flock (arg) \ > + || __fcntl_type_check_const_flock64 (arg)) > + > +#define __fcntl_type_check_flock_or_64(arg) \ > + (__fcntl_type_check_flock (arg) || __fcntl_type_check_flock64 (arg)) > + > +#define __fcntl_type_check(cmd, arg, is_fcntl64) \ > + (__fcntl_is_int (cmd) ? __fcntl_type_check_int (arg) : \ > + __fcntl_is_const_uint64_t_ptr (cmd) \ > + ? __fcntl_type_check_const_uint64_t_ptr (arg) : \ > + __fcntl_is_uint64_t_ptr (cmd) ? __fcntl_type_check_uint64_t_ptr (arg) : \ > + __fcntl_is_const_fowner_ex (cmd) \ > + ? __fcntl_type_check_const_fowner_ex (arg) : \ > + __fcntl_is_fowner_ex (cmd) ? __fcntl_type_check_fowner_ex (arg) : \ > + __fcntl_is_const_flock_or_64 (cmd) \ > + ? __fcntl_type_check_const_flock_or_64 (arg) : \ > + __fcntl_is_flock_or_64 (cmd) \ > + ? __fcntl_type_check_flock_or_64 (arg) : \ > + __fcntl_is_const_flock (cmd, is_fcntl64) \ > + ? __fcntl_type_check_const_flock (arg) : \ > + __fcntl_is_flock (cmd, is_fcntl64) ? __fcntl_type_check_flock (arg) : \ > + __fcntl_is_const_flock64 (cmd, is_fcntl64) \ > + ? __fcntl_type_check_const_flock64 (arg) : \ > + __fcntl_is_flock64 (cmd, is_fcntl64) ? __fcntl_type_check_flock64 (arg) : \ > + 1) > + > + > +__errordecl (__fcntl_missing_arg, > + "fcntl with with this command needs 3 arguments"); > + > +__fortify_function int > +__fcntl_2_inline (int __fd, int __cmd) > +{ > + if (!__builtin_constant_p (__cmd)) > + return __fcntl_alias (__fd, __cmd); > + > + if (__fcntl_requires_arg (__cmd)) > + __fcntl_missing_arg (); > + > + return __fcntl_alias (__fd, __cmd); > +} > + > + > + > +#define fcntl(fd, cmd, ...) \ > + (__VA_OPT__ (0 ?) __fcntl_2_inline (fd, cmd) \ > + __VA_OPT__ (: \ > + !__builtin_constant_p (cmd) ? __fcntl_alias (fd, cmd, __VA_ARGS__) \ > + : __fcntl_type_check (cmd, __VA_ARGS__, 0) \ > + ? __fcntl_alias (fd, cmd, __VA_ARGS__) \ > + : __fcntl_warn (fd, cmd, __VA_ARGS__))) > + > +#ifdef __USE_LARGEFILE64 > + > +#define fcntl64(fd, cmd, ...) \ > + (__VA_OPT__ (0 ?) __fcntl_2_inline (fd, cmd) \ > + __VA_OPT__ (: \ > + !__builtin_constant_p (cmd) ? __fcntl64_alias (fd, cmd, __VA_ARGS__) \ > + : __fcntl_type_check (cmd, __VA_ARGS__, 1) \ > + ? __fcntl64_alias (fd, cmd, __VA_ARGS__) \ > + : __fcntl64_warn (fd, cmd, __VA_ARGS__))) > + > + > +#endif > + > +__glibc_warn_system_headers_end > diff --git a/io/fcntl.h b/io/fcntl.h > index dd620c08..61738f42 100644 > --- a/io/fcntl.h > +++ b/io/fcntl.h > @@ -337,11 +337,17 @@ extern int posix_fallocate64 (int __fd, off64_t __offset, off64_t __len); > > > /* Define some inlines helping to catch common problems. */ > + Spurious new line? > #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function \ > && defined __va_arg_pack_len > # include > #endif > > +#if __USE_FORTIFY_LEVEL > 0 && defined (__fortify_function) \ > + && (__GNUC_PREREQ (8, 1) || __glibc_clang_prereq (12, 0)) > +# include > +#endif > + > __END_DECLS > > #endif /* fcntl.h */ > diff --git a/manual/maint.texi b/manual/maint.texi > index 89da704f..11509def 100644 > --- a/manual/maint.texi > +++ b/manual/maint.texi > @@ -200,7 +200,7 @@ functions but may also include checks for validity of other inputs to > the functions. > > When the @code{_FORTIFY_SOURCE} macro is defined, it enables code that > -validates inputs passed to some functions in @theglibc to determine if > +validates inputs passed to some functions in @theglibc{} to determine if > they are safe. If the compiler is unable to determine that the inputs > to the function call are safe, the call may be replaced by a call to its > hardened variant that does additional safety checks at runtime. Some > @@ -221,7 +221,8 @@ returned by the @code{__builtin_object_size} compiler builtin function. > If the function returns @code{(size_t) -1}, the function call is left > untouched. Additionally, this level also enables validation of flags to > the @code{open}, @code{open64}, @code{openat} and @code{openat64} > -functions. > +functions, as well as validation of the presence and the type of the > +third argument to the @code{fcntl} and @code{fcntl64} functions. > > @item @math{2}: This behaves like @math{1}, with the addition of some > checks that may trap code that is conforming but unsafe, e.g. accepting > @@ -267,6 +268,10 @@ The following functions and macros are fortified in @theglibc{}: > > @item @code{explicit_bzero} > > +@item @code{fcntl} > + > +@item @code{fcntl64} > + > @item @code{FD_SET} > > @item @code{FD_CLR} > diff --git a/posix/test-errno.c b/posix/test-errno.c > index 3685fd15..e73978aa 100644 > --- a/posix/test-errno.c > +++ b/posix/test-errno.c > @@ -122,7 +122,9 @@ do_test (void) > fails |= test_wrp (EBADF, dup2, -1, -1); > fails |= test_wrp (EBADF, fchdir, -1); > fails |= test_wrp (EBADF, fchmod, -1, 0); > - fails |= test_wrp (EBADF, fcntl, -1, 0); > + /* Parenthesized to avoid the fortification macro, which finds a different > + error here at compile time. */ > + fails |= test_wrp (EBADF, (fcntl), -1, 0); > fails |= test_wrp (EBADF, fstatfs, -1, &sfs); > fails |= test_wrp (EBADF, fsync, -1); > fails |= test_wrp (EBADF, ftruncate, -1, 0); > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index be801e3b..ddbf441b 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -246,6 +246,10 @@ ifeq ($(have-GLIBC_2.27)$(build-shared),yesyes) > tests += \ > tst-ofdlocks-compat \ > # tests > + > +routines_no_fortify += \ > + tst-ofdlocks-compat \ > + # routines_no_fortify > endif > > tests-internal += \