| Message ID | 20230528172013.73111-4-bugaevc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fcntl fortification | expand |
On 28/05/23 14:20, 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 (). > > Recently, Hector Martin <marcan@marcan.st> has identified an issue in > libwebrtc where fcntl (fd, F_DUPFD_CLOEXEC) was invoked without a third > argument [0]. With the patch, the bug would have been detected at > compile time, assuming libwebrtc is built with FORTIFY_SOURCE. Hopefully > this will help detecting similar bugs in existing software, and prevent > more instances of similar bugs from being introduced in the future. > > [0]: https://social.treehouse.systems/@marcan/110355001391961285 > > Add tests for fcntl () and fcntl64 () calls still compiling and > functioning correctly in various configurations, and mention the new > fortification in the manual. > > The abilists have been modified with 'make update-abi-all'. Some comments below. > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > debug/tst-fortify.c | 148 +++++++ > include/fcntl.h | 1 + > io/Makefile | 1 + > io/Versions | 3 + > io/bits/fcntl2.h | 397 ++++++++++++++++++ > io/fcntl_2.c | 33 ++ > manual/maint.texi | 19 +- > sysdeps/mach/hurd/i386/libc.abilist | 1 + > sysdeps/mach/hurd/x86_64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arc/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 + > sysdeps/unix/sysv/linux/csky/libc.abilist | 1 + > sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 + > sysdeps/unix/sysv/linux/i386/libc.abilist | 1 + > sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 + > .../sysv/linux/loongarch/lp64/libc.abilist | 1 + > .../sysv/linux/m68k/coldfire/libc.abilist | 1 + > .../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 + > .../sysv/linux/microblaze/be/libc.abilist | 1 + > .../sysv/linux/microblaze/le/libc.abilist | 1 + > .../sysv/linux/mips/mips32/fpu/libc.abilist | 1 + > .../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 + > .../sysv/linux/mips/mips64/n32/libc.abilist | 1 + > .../sysv/linux/mips/mips64/n64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 + > sysdeps/unix/sysv/linux/or1k/libc.abilist | 1 + > .../linux/powerpc/powerpc32/fpu/libc.abilist | 1 + > .../powerpc/powerpc32/nofpu/libc.abilist | 1 + > .../linux/powerpc/powerpc64/be/libc.abilist | 1 + > .../linux/powerpc/powerpc64/le/libc.abilist | 1 + > .../unix/sysv/linux/riscv/rv32/libc.abilist | 1 + > .../unix/sysv/linux/riscv/rv64/libc.abilist | 1 + > .../unix/sysv/linux/s390/s390-32/libc.abilist | 1 + > .../unix/sysv/linux/s390/s390-64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 + > sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 + > .../sysv/linux/sparc/sparc32/libc.abilist | 1 + > .../sysv/linux/sparc/sparc64/libc.abilist | 1 + > .../unix/sysv/linux/x86_64/64/libc.abilist | 1 + > .../unix/sysv/linux/x86_64/x32/libc.abilist | 1 + > 43 files changed, 632 insertions(+), 6 deletions(-) > create mode 100644 io/fcntl_2.c > > diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c > index 7850a4e5..03264154 100644 > --- a/debug/tst-fortify.c > +++ b/debug/tst-fortify.c > @@ -36,6 +36,8 @@ > #include <sys/select.h> > #include <sys/socket.h> > #include <sys/un.h> > +#include <support/support.h> > +#include <support/xunistd.h> > > #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 int __attribute_noinline__ > +hide_constant (int value) > +{ > + volatile int v = value; > + return v; > +} > + Interesting construct, but I wonder if 'volatile' is really required here. Afaiu what really prevents compiler to return fortify compiler warning here is the __attribute_noinline__. > volatile int chk_fail_ok; > volatile int ret; > jmp_buf chk_fail_buf; > @@ -1763,6 +1774,143 @@ 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); > + > +#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); > + > + struct flock flock; Maybe move the flock struct to within the block for where ofd_locks_supported is true? > + int ofd_locks_supported = support_fcntl_support_ofd_locks (lockfd1); > + > + if (ofd_locks_supported) > + { > + 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) > + { > + 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 __USE_FORTIFY_LEVEL >= 1 > + CHK_FAIL_START > + fcntl (STDIN_FILENO, hide_constant (F_SETFD)); > + CHK_FAIL_END > +#endif > + > +#if defined (__USE_LARGEFILE64) || defined (__USE_TIME_BITS64) There is no need to replicate the tests for the LFS support, this is already done by tst-fortify-cc-lfs-X tests. > + /* 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); > + > +#ifdef F_OFD_GETLK > + 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 > + > +# if __USE_FORTIFY_LEVEL >= 1 > + CHK_FAIL_START > + fcntl64 (STDIN_FILENO, hide_constant (F_SETFD)); > + CHK_FAIL_END > +# endif > #endif > > return ret; > diff --git a/include/fcntl.h b/include/fcntl.h > index be435047..cb86c5e7 100644 > --- a/include/fcntl.h > +++ b/include/fcntl.h > @@ -32,6 +32,7 @@ extern int __open64_2 (const char *__path, int __oflag); > extern int __openat_2 (int __fd, const char *__path, int __oflag); > extern int __openat64_2 (int __fd, const char *__path, int __oflag); > > +extern int __fcntl_2 (int __fd, int __cmd); > > #if IS_IN (rtld) > # include <dl-fcntl.h> > diff --git a/io/Makefile b/io/Makefile > index 6b58728e..1773a0f2 100644 > --- a/io/Makefile > +++ b/io/Makefile > @@ -71,6 +71,7 @@ routines := \ > fchownat \ > fcntl \ > fcntl64 \ > + fcntl_2 \ > file_change_detection \ > flock \ > fstat \ > diff --git a/io/Versions b/io/Versions > index 4e195408..0e77a287 100644 > --- a/io/Versions > +++ b/io/Versions > @@ -140,6 +140,9 @@ libc { > GLIBC_2.34 { > closefrom; > } > + GLIBC_2.38 { > + __fcntl_2; > + } > GLIBC_PRIVATE { > __libc_fcntl64; > __fcntl_nocancel; Ok. > diff --git a/io/bits/fcntl2.h b/io/bits/fcntl2.h > index bdb48fa8..e8dd3dea 100644 > --- a/io/bits/fcntl2.h > +++ b/io/bits/fcntl2.h > @@ -170,3 +170,400 @@ openat64 (int __fd, const char *__path, int __oflag, ...) > } > # endif > #endif > + > +extern int __fcntl_2 (int __fd, int __cmd); > + > +#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: > + case F_DUPFD_CLOEXEC: > + 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: > + case F_ADD_SEALS: > + case F_GET_RW_HINT: > + case F_SET_RW_HINT: > + 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: > + case F_DUPFD_CLOEXEC: > + 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: > +#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 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 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 > + > +template<typename, typename> > +struct __fcntl_types_compatible_helper This makes the C++ tests fail with GCC 13: $ x86_64-glibc-linux-gnu-g++ [...] tst-fortify-cc-default-1-def.cc -D_FORTIFY_SOURCE=1 -Wno-format -Wno-deprecated-declarations -Wno-error [...] [...] In file included from ../include/bits/fcntl2.h:1, from ../io/fcntl.h:342, from ../include/fcntl.h:2, from ./tst-fortify.c:25, from /home/azanella/Projects/glibc/build/x86_64-linux-gnu/debug/tst-fortify-cc-default-1-def.cc:3: ../include/bits/../../io/bits/fcntl2.h:450:1: error: template with C linkage 450 | template<typename, typename> | ^~~~~~~~ In file included from ../include/sys/cdefs.h:10, from ../include/features.h:503, from ./tst-fortify.c:20: ../misc/sys/cdefs.h:140:25: note: ‘extern "C"’ linkage started here 140 | # define __BEGIN_DECLS extern "C" { | ^~~~~~~~~~ ../io/fcntl.h:28:1: note: in expansion of macro ‘__BEGIN_DECLS’ 28 | __BEGIN_DECLS | ^~~~~~~~~~~~~ ../include/bits/../../io/bits/fcntl2.h:460:1: error: template with C linkage 460 | template<typename __T> | ^~~~~~~~ ../misc/sys/cdefs.h:140:25: note: ‘extern "C"’ linkage started here 140 | # define __BEGIN_DECLS extern "C" { | ^~~~~~~~~~ ../io/fcntl.h:28:1: note: in expansion of macro ‘__BEGIN_DECLS’ 28 | __BEGIN_DECLS | ^~~~~~~~~~~~~ In file included from ../misc/sys/select.h:30, from ../include/sys/select.h:2, from ../posix/sys/types.h:179, from ../include/sys/types.h:1, from ../stdlib/stdlib.h:514, from ../include/stdlib.h:16, from ./tst-fortify.c:31: [...] You will will need to include bits/fcntl2.h after the __END_DECLS, and add __BEGIN_DECLS/__END_DECLS on fcntl2.h. Something like this on top this patch: diff --git a/io/bits/fcntl2.h b/io/bits/fcntl2.h index e8dd3dea28..95323af2c2 100644 --- a/io/bits/fcntl2.h +++ b/io/bits/fcntl2.h @@ -20,6 +20,8 @@ # error "Never include <bits/fcntl2.h> directly; use <fcntl.h> instead." #endif +__BEGIN_DECLS + /* Check that calls to open and openat with O_CREAT or O_TMPFILE set have an appropriate third/fourth parameter. */ #ifndef __USE_FILE_OFFSET64 @@ -440,6 +442,8 @@ __fcntl_is_flock64 (int __cmd, int __is_fcntl64) } } +__END_DECLS + #ifndef __cplusplus # define __fcntl_types_compatible(arg, type) \ @@ -527,6 +531,7 @@ struct __fcntl_types_compatible_helper<__T, __T> __fcntl_is_flock64 (cmd, is_fcntl64) ? __fcntl_type_check_flock64 (arg) : \ 1) +__BEGIN_DECLS __errordecl (__fcntl_missing_arg, "fcntl with with this command needs 3 arguments"); @@ -567,3 +572,5 @@ __glibc_warn_system_headers_begin #endif __glibc_warn_system_headers_end + +__END_DECLS diff --git a/io/fcntl.h b/io/fcntl.h index dd620c086f..f6b6618424 100644 --- a/io/fcntl.h +++ b/io/fcntl.h @@ -335,6 +335,7 @@ extern int posix_fallocate64 (int __fd, off64_t __offset, off64_t __len); # endif #endif +__END_DECLS /* Define some inlines helping to catch common problems. */ #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function \ @@ -342,6 +343,4 @@ extern int posix_fallocate64 (int __fd, off64_t __offset, off64_t __len); # include <bits/fcntl2.h> #endif -__END_DECLS - #endif /* fcntl.h */ > +{ > + __always_inline static int > + __compatible () > + { > + return 0; > + } > +}; > + > +template<typename __T> > +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 () > + > +#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(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 (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_2 (__fd, __cmd); > + > + if (__fcntl_requires_arg (__cmd)) > + __fcntl_missing_arg (); > + > + return __fcntl_alias (__fd, __cmd); > +} > + > +__glibc_warn_system_headers_begin > + > +#define fcntl(fd, cmd, ...) \ > + (__VA_OPT__ (0 ?) __fcntl_2_inline (fd, cmd) \ I think we will need to enable fcntl fortify only for gcc 8 or higher, since __VA_OPT__ is not support on gcc 7. > + __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_2.c b/io/fcntl_2.c > new file mode 100644 > index 00000000..8db79c2a > --- /dev/null > +++ b/io/fcntl_2.c > @@ -0,0 +1,33 @@ > +/* _FORTIFY_SOURCE wrapper for fcntl. > + Copyright (C) 2013-2023 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 > + <https://www.gnu.org/licenses/>. */ > + > +/* Make sure to get __fcntl_requires_arg from bits/fcntl2.h */ > +#undef _FORTIFY_SOURCE > +#define _FORTIFY_SOURCE 1 > + > +#include <fcntl.h> > +#include <stdio.h> > + > +int > +__fcntl_2 (int fd, int cmd) > +{ > + if (__fcntl_requires_arg (cmd)) > + __fortify_fail ("invalid fcntl call: this command requires an argument"); > + > + return __libc_fcntl64 (fd, cmd); > +} > diff --git a/manual/maint.texi b/manual/maint.texi > index a8441e20..74647e40 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 > @@ -243,10 +244,11 @@ depending on the architecture, one may also see fortified variants have > the @code{_chkieee128} suffix or the @code{__nldbl___} prefix to their > names. > > -Another exception is the @code{open} family of functions, where their > -fortified replacements have the @code{__} prefix and a @code{_2} suffix. > -The @code{FD_SET}, @code{FD_CLR} and @code{FD_ISSET} macros use the > -@code{__fdelt_chk} function on fortification. > +Another exception is the @code{open} and @code{fcntl} families of > +functions, where their fortified 2-argument version replacements have the > +@code{__} prefix and a @code{_2} suffix. The @code{FD_SET}, @code{FD_CLR} > +and @code{FD_ISSET} macros use the @code{__fdelt_chk} function on > +fortification. > > The following functions and macros are fortified in @theglibc{}: > @c Generated using the following command: > @@ -255,6 +257,7 @@ The following functions and macros are fortified in @theglibc{}: > @c sort -u | grep ^__ | > @c grep -v -e ieee128 -e __nldbl -e align_cpy -e "fdelt_warn" | > @c sed 's/__fdelt_chk/@item @code{FD_SET}\n\n@item @code{FD_CLR}\n\n@item @code{FD_ISSET}\n/' | > +@c sed 's/__fcntl_2/@item @code{fcntl}\n\n@item @code{fcntl64}\n/' | > @c sed 's/__\(.*\)_\(chk\|2\)/@item @code{\1}\n/' > > @itemize @bullet > @@ -267,6 +270,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/sysdeps/mach/hurd/i386/libc.abilist b/sysdeps/mach/hurd/i386/libc.abilist > index 6925222f..5397289b 100644 > --- a/sysdeps/mach/hurd/i386/libc.abilist > +++ b/sysdeps/mach/hurd/i386/libc.abilist > @@ -2294,6 +2294,7 @@ GLIBC_2.36 arc4random_buf F > GLIBC_2.36 arc4random_uniform F > GLIBC_2.36 c8rtomb F > GLIBC_2.36 mbrtoc8 F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/mach/hurd/x86_64/libc.abilist b/sysdeps/mach/hurd/x86_64/libc.abilist > index a0be5c1a..6ed14d1f 100644 > --- a/sysdeps/mach/hurd/x86_64/libc.abilist > +++ b/sysdeps/mach/hurd/x86_64/libc.abilist > @@ -194,6 +194,7 @@ GLIBC_2.38 __errno_location F > GLIBC_2.38 __explicit_bzero_chk F > GLIBC_2.38 __fbufsize F > GLIBC_2.38 __fcntl F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __fdelt_chk F > GLIBC_2.38 __fdelt_warn F > GLIBC_2.38 __fentry__ F > diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist > index 0e2d9c30..6a3e3e7c 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist > @@ -2633,6 +2633,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist > index f1bec197..4cbc9346 100644 > --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist > +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist > @@ -2730,6 +2730,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/arc/libc.abilist b/sysdeps/unix/sysv/linux/arc/libc.abilist > index aa874b88..01fa7a34 100644 > --- a/sysdeps/unix/sysv/linux/arc/libc.abilist > +++ b/sysdeps/unix/sysv/linux/arc/libc.abilist > @@ -2394,6 +2394,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist > index afbd57da..2d7f5db4 100644 > --- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist > +++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist > @@ -514,6 +514,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist > index e7364cd3..85eea0e5 100644 > --- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist > +++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist > @@ -511,6 +511,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist > index 913fa592..0bc57ae1 100644 > --- a/sysdeps/unix/sysv/linux/csky/libc.abilist > +++ b/sysdeps/unix/sysv/linux/csky/libc.abilist > @@ -2670,6 +2670,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist > index 43af3a98..4f2e8b5d 100644 > --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist > +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist > @@ -2619,6 +2619,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist > index af72f8fa..8b8b8789 100644 > --- a/sysdeps/unix/sysv/linux/i386/libc.abilist > +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist > @@ -2803,6 +2803,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist > index 48cbb0fa..22f7f159 100644 > --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist > @@ -2568,6 +2568,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist b/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist > index c15884bb..acb9a793 100644 > --- a/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist > @@ -2154,6 +2154,7 @@ GLIBC_2.36 wprintf F > GLIBC_2.36 write F > GLIBC_2.36 writev F > GLIBC_2.36 wscanf F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > index 3738db81..af6762d1 100644 > --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist > @@ -515,6 +515,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > index ed136277..19ac2156 100644 > --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist > @@ -2746,6 +2746,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist > index 83577386..6d7e9d95 100644 > --- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist > +++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist > @@ -2719,6 +2719,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist > index 58c5da58..08459ff7 100644 > --- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist > +++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist > @@ -2716,6 +2716,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > index d3741945..3b9caff0 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist > @@ -2711,6 +2711,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > index 5319fdc2..2eb10c81 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist > @@ -2709,6 +2709,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > index 1743ea6e..7f13cd56 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist > @@ -2717,6 +2717,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > index 9b1f53c6..adea92bf 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist > @@ -2619,6 +2619,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist > index ae1c6ca1..2816e0b3 100644 > --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist > +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist > @@ -2758,6 +2758,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/or1k/libc.abilist b/sysdeps/unix/sysv/linux/or1k/libc.abilist > index a7c572c9..93674747 100644 > --- a/sysdeps/unix/sysv/linux/or1k/libc.abilist > +++ b/sysdeps/unix/sysv/linux/or1k/libc.abilist > @@ -2140,6 +2140,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > index 074fa031..68aa8601 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist > @@ -2773,6 +2773,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > index dfcb4bd2..bc2f2b8d 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist > @@ -2806,6 +2806,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist > index 63bbccf3..cdc982a3 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist > @@ -2527,6 +2527,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist > index ab85fd61..66733662 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist > @@ -2829,6 +2829,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fscanfieee128 F > GLIBC_2.38 __isoc23_fwscanf F > diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist > index b716f5c7..6cf51998 100644 > --- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist > @@ -2396,6 +2396,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist > index 774e777b..84f920f2 100644 > --- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist > @@ -2596,6 +2596,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > index 8625135c..c80822de 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist > @@ -2771,6 +2771,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > index d00c7eb2..ac344828 100644 > --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist > @@ -2564,6 +2564,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist > index b6303724..c064e000 100644 > --- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist > @@ -2626,6 +2626,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist > index d8005561..a185e3cb 100644 > --- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist > @@ -2623,6 +2623,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > index 5be55c11..ad5aa083 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist > @@ -2766,6 +2766,7 @@ GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > GLIBC_2.37 __ppoll64_chk F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > index 475fdaae..27fffe9d 100644 > --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist > @@ -2591,6 +2591,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > index 6cfb928b..3c46f003 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist > @@ -2542,6 +2542,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > index c7350971..5a4fa94c 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist > @@ -2648,6 +2648,7 @@ GLIBC_2.36 pidfd_open F > GLIBC_2.36 pidfd_send_signal F > GLIBC_2.36 process_madvise F > GLIBC_2.36 process_mrelease F > +GLIBC_2.38 __fcntl_2 F > GLIBC_2.38 __isoc23_fscanf F > GLIBC_2.38 __isoc23_fwscanf F > GLIBC_2.38 __isoc23_scanf F
Hello, On Mon, May 29, 2023 at 7:54 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > Some comments below. thank you for the review :) > > +/* Return VALUE, but do it in a way that the compiler cannot > > + see that it's a compile-time constant. */ > > +static int __attribute_noinline__ > > +hide_constant (int value) > > +{ > > + volatile int v = value; > > + return v; > > +} > > + > > Interesting construct, but I wonder if 'volatile' is really required here. > Afaiu what really prevents compiler to return fortify compiler warning here > is the __attribute_noinline__. It's not really required. Apparently even an 'inline' function call is enough to to disarm __builtin_constant_p [0]; I just threw both volatile and noinline in for good measure. [0] https://godbolt.org/z/v9569KGTT > > + struct flock flock; > > Maybe move the flock struct to within the block for where ofd_locks_supported > is true? It's used in the following if (ofd_locks_supported) block -- the one checking for non-compile-time-const cmd -- too. > > +#if defined (__USE_LARGEFILE64) || defined (__USE_TIME_BITS64) > > There is no need to replicate the tests for the LFS support, this is already > done by tst-fortify-cc-lfs-X tests. Hm, I don't think I understand your point, could you please expand? The -lfs- tests are the ones that enable -D _FILE_OFFSET_BITS=64, right? Here, I'm testing that if __USE_LARGEFILE64 is enabled (i.e. -D _LARGEFILE64_SOURCE), which means fcntl64 () is available as a separate function, the fcntl64 fortification also behaves as expected. It's not enough to just test that fcntl () works as expected in 64-bit mode, we need to test that fcntl64 () also works as expected. These are separate functions (perhaps aliases of each other, but separate fortified macros for sure) that may or may not both work with 64-bit types. __USE_LARGEFILE64 and __USE_FILE_OFFSET64 are orthogonal in my understanding, aren't they? __USE_LARGEFILE64 means that xxxx64 APIs are declared as a separate set of APIs that explicitly work with 64-bit types; __USE_FILE_OFFSET64 means that the regular APIs (xxxx, not xxxx64) are declared to already use 64-bit. You can have neither enabled, or one of them enabled, or both. No? > > +template<typename, typename> > > +struct __fcntl_types_compatible_helper > > This makes the C++ tests fail with GCC 13: > > [...] > ../include/bits/../../io/bits/fcntl2.h:460:1: error: template with C linkage > 460 | template<typename __T> > | ^~~~~~~~ > ../misc/sys/cdefs.h:140:25: note: ‘extern "C"’ linkage started here > 140 | # define __BEGIN_DECLS extern "C" { Interesting. Any idea why this wasn't failing in my testing? > You will will need to include bits/fcntl2.h after the __END_DECLS, and add > __BEGIN_DECLS/__END_DECLS on fcntl2.h. Something like this on top this > patch Right, makes sense, thank you. > I think we will need to enable fcntl fortify only for gcc 8 or higher, since > __VA_OPT__ is not support on gcc 7. So maybe it is a good idea to move this all into fcntl3.h like it was done in Florian's patch? Then I'd include that after __END_DECLS and only if GCC >= 8. Sergey
On 29/05/23 14:31, Sergey Bugaev wrote: > Hello, > > On Mon, May 29, 2023 at 7:54 PM Adhemerval Zanella Netto > <adhemerval.zanella@linaro.org> wrote: >> Some comments below. > > thank you for the review :) > >>> +/* Return VALUE, but do it in a way that the compiler cannot >>> + see that it's a compile-time constant. */ >>> +static int __attribute_noinline__ >>> +hide_constant (int value) >>> +{ >>> + volatile int v = value; >>> + return v; >>> +} >>> + >> >> Interesting construct, but I wonder if 'volatile' is really required here. >> Afaiu what really prevents compiler to return fortify compiler warning here >> is the __attribute_noinline__. > > It's not really required. Apparently even an 'inline' function call is > enough to to disarm __builtin_constant_p [0]; I just threw both > volatile and noinline in for good measure.> > [0] https://godbolt.org/z/v9569KGTT It is because afaiu 'volatile' does not really match the semantics of what you are trying to do here. Maybe if you really need the compile to force lvalue evaluation, it should use something similar to math_opt_barrier. > >>> + struct flock flock; >> >> Maybe move the flock struct to within the block for where ofd_locks_supported >> is true? > > It's used in the following if (ofd_locks_supported) block -- the one > checking for non-compile-time-const cmd -- too. Yes, but the idea is to avoid the stack allocated variable to escape its specific usage (Florian is the one that constantly nag about it). > >>> +#if defined (__USE_LARGEFILE64) || defined (__USE_TIME_BITS64) >> >> There is no need to replicate the tests for the LFS support, this is already >> done by tst-fortify-cc-lfs-X tests. > > Hm, I don't think I understand your point, could you please expand? Nevermind, you do need to check for __USE_LARGEFILE64 symbols. But you can omit the '#if defined", they are always defined. > > The -lfs- tests are the ones that enable -D _FILE_OFFSET_BITS=64, > right? Here, I'm testing that if __USE_LARGEFILE64 is enabled (i.e. -D > _LARGEFILE64_SOURCE), which means fcntl64 () is available as a > separate function, the fcntl64 fortification also behaves as expected. > It's not enough to just test that fcntl () works as expected in 64-bit > mode, we need to test that fcntl64 () also works as expected. These > are separate functions (perhaps aliases of each other, but separate > fortified macros for sure) that may or may not both work with 64-bit > types. And debugging a bit, it seems that I found another issue. Building tst-fortify-c-lfs-1-def.c the post-processed output (-E -dD) for fcntl call is: res = (0 ? __fcntl_2_inline (0, 2) : !__builtin_constant_p (2) ? __fcntl_alias (0, 2, 0) : (__fcntl_is_int (2) ? __builtin_types_compatible_p (__typeof (0), int) : __fcntl_is_const_uint64_t_ptr (2) ? (__builtin_types_compatible_p (__typeof (0), const __uint64_t *) || __builtin_types_compatible_p (__typeof (0), __uint64_t *)) : __fcntl_is_uint64_t_ptr (2) ? __builtin_types_compatible_p (__typeof (0), __uint64_t *) : __fcntl_is_const_fowner_ex (2) ? (__builtin_types_compatible_p (__typeof (0), const struct f_owner_ex *) || __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *)) : __fcntl_is_fowner_ex (2) ? __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *) : __fcntl_is_const_flock (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock *) || __builtin_types_compatible_p (__typeof (0), struct flock *)) : __fcntl_is_flock (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock *) : __fcntl_is_const_flock64 (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock64 *) || __builtin_types_compatible_p (__typeof (0), struct flock64 *)) : __fcntl_is_flock64 (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock64 *) : 1) ? __fcntl_alias (0, 2, 0) : __fcntl_warn (0, 2, 0)); Which is the non-LFS version. For non macro calls, pread for instance, we fix this by using the __glibc_fortify version: # ifndef __USE_FILE_OFFSET64 __fortify_function __wur ssize_t pread (int __fd, void *__buf, size_t __nbytes, __off_t __offset) { return __glibc_fortify (pread, __nbytes, sizeof (char), __glibc_objsize0 (__buf), __fd, __buf, __nbytes, __offset); } # else __fortify_function __wur ssize_t pread (int __fd, void *__buf, size_t __nbytes, __off64_t __offset) { return __glibc_fortify (pread64, __nbytes, sizeof (char), __glibc_objsize0 (__buf), __fd, __buf, __nbytes, __offset); } # endif So I think this patch should do the same: #ifndef __USE_FILE_OFFSET64 # 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__))) #else # define fcntl(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__, 0) \ ? __fcntl64_alias (fd, cmd, __VA_ARGS__) \ : __fcntl64_warn (fd, cmd, __VA_ARGS__))) #endif #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 > > __USE_LARGEFILE64 and __USE_FILE_OFFSET64 are orthogonal in my > understanding, aren't they? __USE_LARGEFILE64 means that xxxx64 APIs > are declared as a separate set of APIs that explicitly work with > 64-bit types; __USE_FILE_OFFSET64 means that the regular APIs (xxxx, > not xxxx64) are declared to already use 64-bit. You can have neither > enabled, or one of them enabled, or both. No? > >>> +template<typename, typename> >>> +struct __fcntl_types_compatible_helper >> >> This makes the C++ tests fail with GCC 13: >> >> [...] >> ../include/bits/../../io/bits/fcntl2.h:460:1: error: template with C linkage >> 460 | template<typename __T> >> | ^~~~~~~~ >> ../misc/sys/cdefs.h:140:25: note: ‘extern "C"’ linkage started here >> 140 | # define __BEGIN_DECLS extern "C" { > > Interesting. Any idea why this wasn't failing in my testing? My guess is because template with extern "C" are not really supported in all configurations. > >> You will will need to include bits/fcntl2.h after the __END_DECLS, and add >> __BEGIN_DECLS/__END_DECLS on fcntl2.h. Something like this on top this >> patch > > Right, makes sense, thank you. > >> I think we will need to enable fcntl fortify only for gcc 8 or higher, since >> __VA_OPT__ is not support on gcc 7. > > So maybe it is a good idea to move this all into fcntl3.h like it was > done in Florian's patch? Then I'd include that after __END_DECLS and > only if GCC >= 8. Yes, it would be better indeed.
On Mon, May 29, 2023 at 9:09 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > > It's not really required. Apparently even an 'inline' function call is > > enough to to disarm __builtin_constant_p [0]; I just threw both > > volatile and noinline in for good measure.> > > [0] https://godbolt.org/z/v9569KGTT > > It is because afaiu 'volatile' does not really match the semantics of what you > are trying to do here. Maybe if you really need the compile to force lvalue > evaluation, it should use something similar to math_opt_barrier. So, an asm block that does nothing, but is said to read and modify the value? Sounds better indeed, and reminds me of how std::hint::black_box is implemented. > Nevermind, you do need to check for __USE_LARGEFILE64 symbols. But > you can omit the '#if defined", they are always defined. Uhh that's unfortunate actually. I hoped the test would check the non-__USE_LARGEFILE64 configuration as well, to catch some __USE_LARGEFILE64-only definition being used unconditionally, for example. > And debugging a bit, it seems that I found another issue. Building > tst-fortify-c-lfs-1-def.c the post-processed output (-E -dD) for fcntl call is: > > res = (0 ? __fcntl_2_inline (0, 2) : !__builtin_constant_p (2) ? __fcntl_alias (0, 2, 0) : (__fcntl_is_int (2) ? __builtin_types_compatible_p (__typeof (0), int) : __fcntl_is_const_uint64_t_ptr (2) ? (__builtin_types_compatible_p (__typeof (0), const __uint64_t *) || __builtin_types_compatible_p (__typeof (0), __uint64_t *)) : __fcntl_is_uint64_t_ptr (2) ? __builtin_types_compatible_p (__typeof (0), __uint64_t *) : __fcntl_is_const_fowner_ex (2) ? (__builtin_types_compatible_p (__typeof (0), const struct f_owner_ex *) || __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *)) : __fcntl_is_fowner_ex (2) ? __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *) : __fcntl_is_const_flock (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock *) || __builtin_types_compatible_p (__typeof (0), struct flock *)) : __fcntl_is_flock (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock *) : __fcntl_is_const_flock64 (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock64 *) || __builtin_types_compatible_p (__typeof (0), struct flock64 *)) : __fcntl_is_flock64 (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock64 *) : 1) ? __fcntl_alias (0, 2, 0) : __fcntl_warn (0, 2, 0)); > > Which is the non-LFS version. Hmm, I don't see the issue? And the test passes, doesn't it? :) (If it passes *despite* the 32-vs-64 confusion, that is a problem in itself, that'd mean it's not checking what it's supposed to be checking.) The fcntl macro will always expand to the same thing (modulo C vs C++ type check differences). But then __fcntl_alias and __fcntl_warn both reference the same symbol that fcntl would reference had it not been for the fortification -- which is going to be the 'fcntl64' symbol with __USE_FILE_OFFSET64. > So I think this patch should do the same: > > #ifndef __USE_FILE_OFFSET64 > # 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__))) > #else > # define fcntl(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__, 0) \ > ? __fcntl64_alias (fd, cmd, __VA_ARGS__) \ > : __fcntl64_warn (fd, cmd, __VA_ARGS__))) > #endif What's the point? -- __fcntl_alias and __fcntl64_alias are supposed to both reference the same thing (fcntl64 or __fcntl_time64) with __USE_FILE_OFFSET64. Or more generally: do the same thing as the non-fortified definition does. Hope this makes sense. Sergey
On 29/05/23 16:57, Sergey Bugaev wrote: > On Mon, May 29, 2023 at 9:09 PM Adhemerval Zanella Netto > <adhemerval.zanella@linaro.org> wrote: >>> It's not really required. Apparently even an 'inline' function call is >>> enough to to disarm __builtin_constant_p [0]; I just threw both >>> volatile and noinline in for good measure.> >>> [0] https://godbolt.org/z/v9569KGTT >> >> It is because afaiu 'volatile' does not really match the semantics of what you >> are trying to do here. Maybe if you really need the compile to force lvalue >> evaluation, it should use something similar to math_opt_barrier. > > So, an asm block that does nothing, but is said to read and modify the > value? Sounds better indeed, and reminds me of how > std::hint::black_box is implemented. Now that you brought Rust black_box, we already something similar on benchtests: DO_NOT_OPTIMIZE_OUT. > >> Nevermind, you do need to check for __USE_LARGEFILE64 symbols. But >> you can omit the '#if defined", they are always defined. > > Uhh that's unfortunate actually. I hoped the test would check the > non-__USE_LARGEFILE64 configuration as well, to catch some > __USE_LARGEFILE64-only definition being used unconditionally, for > example. > The LFS names are not considered a namespace pollution, so I think that's why it always provided (just check tst-fortify.c LFS name usage, like pread64). >> And debugging a bit, it seems that I found another issue. Building >> tst-fortify-c-lfs-1-def.c the post-processed output (-E -dD) for fcntl call is: >> >> res = (0 ? __fcntl_2_inline (0, 2) : !__builtin_constant_p (2) ? __fcntl_alias (0, 2, 0) : (__fcntl_is_int (2) ? __builtin_types_compatible_p (__typeof (0), int) : __fcntl_is_const_uint64_t_ptr (2) ? (__builtin_types_compatible_p (__typeof (0), const __uint64_t *) || __builtin_types_compatible_p (__typeof (0), __uint64_t *)) : __fcntl_is_uint64_t_ptr (2) ? __builtin_types_compatible_p (__typeof (0), __uint64_t *) : __fcntl_is_const_fowner_ex (2) ? (__builtin_types_compatible_p (__typeof (0), const struct f_owner_ex *) || __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *)) : __fcntl_is_fowner_ex (2) ? __builtin_types_compatible_p (__typeof (0), struct f_owner_ex *) : __fcntl_is_const_flock (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock *) || __builtin_types_compatible_p (__typeof (0), struct flock *)) : __fcntl_is_flock (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock *) : __fcntl_is_const_flock64 (2, 0) ? (__builtin_types_compatible_p (__typeof (0), const struct flock64 *) || __builtin_types_compatible_p (__typeof (0), struct flock64 *)) : __fcntl_is_flock64 (2, 0) ? __builtin_types_compatible_p (__typeof (0), struct flock64 *) : 1) ? __fcntl_alias (0, 2, 0) : __fcntl_warn (0, 2, 0)); >> >> Which is the non-LFS version. > > Hmm, I don't see the issue? And the test passes, doesn't it? :) > (If it passes *despite* the 32-vs-64 confusion, that is a problem in > itself, that'd mean it's not checking what it's supposed to be > checking.) > > The fcntl macro will always expand to the same thing (modulo C vs C++ > type check differences). But then __fcntl_alias and __fcntl_warn both > reference the same symbol that fcntl would reference had it not been > for the fortification -- which is going to be the 'fcntl64' symbol > with __USE_FILE_OFFSET64. > >> So I think this patch should do the same: >> >> #ifndef __USE_FILE_OFFSET64 >> # 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__))) >> #else >> # define fcntl(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__, 0) \ >> ? __fcntl64_alias (fd, cmd, __VA_ARGS__) \ >> : __fcntl64_warn (fd, cmd, __VA_ARGS__))) >> #endif > > What's the point? -- __fcntl_alias and __fcntl64_alias are supposed to > both reference the same thing (fcntl64 or __fcntl_time64) with > __USE_FILE_OFFSET64. Or more generally: do the same thing as the > non-fortified definition does. > > Hope this makes sense. Yeah, I forgot __fcntl_alias is in fact ... an alias. So in fact there is no issue here.
On Mon, May 29, 2023 at 11:14 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > Now that you brought Rust black_box, we already something similar on > benchtests: DO_NOT_OPTIMIZE_OUT. I see, thanks -- but that also seems different in that it doesn't mark the value as the output of the asm block, so the compiler could see that it's returned unmodified. So it wouldn't be suitable here. Rust's black_box is / was [0] instead implemented as llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); i.e. it marks the mutable reference to the value as inline asm's input, not the value itself. [0]: nowadays it's a compiler intrinsic that still gets codegenned to a similar asm block when using the LLVM codegen backend > The LFS names are not considered a namespace pollution, so I think that's > why it always provided (just check tst-fortify.c LFS name usage, like > pread64). They may be always provided when building tst-fortify.c, but they're certainly not always provided when building user code (not even provided by default): $ gcc use-fcntl64.c -Werror use-fcntl64.c: In function ‘main’: use-fcntl64.c:6:3: error: implicit declaration of function ‘fcntl64’; did you mean ‘fcntl’? [-Werror=implicit-function-declaration] 6 | fcntl64 (0, F_GETFD); | ^~~~~~~ | fcntl cc1: all warnings being treated as errors $ gcc use-fcntl64.c -D _LARGEFILE64_SOURCE -Werror (builds successfully) So this has to be supported, and ideally tested too. Sergey
On 29/05/23 17:49, Sergey Bugaev wrote: > On Mon, May 29, 2023 at 11:14 PM Adhemerval Zanella Netto > <adhemerval.zanella@linaro.org> wrote: >> Now that you brought Rust black_box, we already something similar on >> benchtests: DO_NOT_OPTIMIZE_OUT. > > I see, thanks -- but that also seems different in that it doesn't mark > the value as the output of the asm block, so the compiler could see > that it's returned unmodified. So it wouldn't be suitable here. It is returned unmodified, but the asm acts a compiler barrier which gcc documentation also declares as 'strong memory barrier' [1] (which I think was written before C11 memory semantic). [1] https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html > > Rust's black_box is / was [0] instead implemented as > > llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); What the 'volatile' constraint does for the llvm_asm? Is is to mimic a 'asm volatile' or is something else? > > i.e. it marks the mutable reference to the value as inline asm's > input, not the value itself. > > [0]: nowadays it's a compiler intrinsic that still gets codegenned to > a similar asm block when using the LLVM codegen backend Yeah, without a compiler intrinsic with proper semantic we need to rely on hacks such this asm barrier to prevent code optimization. > >> The LFS names are not considered a namespace pollution, so I think that's >> why it always provided (just check tst-fortify.c LFS name usage, like >> pread64). > > They may be always provided when building tst-fortify.c, but they're > certainly not always provided when building user code (not even > provided by default): > > $ gcc use-fcntl64.c -Werror > use-fcntl64.c: In function ‘main’: > use-fcntl64.c:6:3: error: implicit declaration of function ‘fcntl64’; > did you mean ‘fcntl’? [-Werror=implicit-function-declaration] > 6 | fcntl64 (0, F_GETFD); > | ^~~~~~~ > | fcntl > cc1: all warnings being treated as errors > $ gcc use-fcntl64.c -D _LARGEFILE64_SOURCE -Werror > (builds successfully) > > So this has to be supported, and ideally tested too. It is exported because all tests are actually built with _GNU_SOURCE (done by include/libc-symbols.h), so the test check is superfluous. It also leaks implementation details, such as internal defines. Usually to check for internal implementation we use test-internal (which are built statically). But if you really want to check for _LARGEFILE64_SOURCE, you will need to add *another* fortify test that undef _GNU_SOURCE (like stdlib/tst-strtol-binary-c11.c for instance).
On Tue, May 30, 2023 at 12:09 AM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > It is returned unmodified, but the asm acts a compiler barrier which gcc > documentation also declares as 'strong memory barrier' [1] (which I think was > written before C11 memory semantic). > > [1] https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html But that one is talking about load/store reordering, not constant propagation. Here's what I'm talking about: [0] (and an actual Rust version at [1]). [0]: https://godbolt.org/z/qaMa7EavY [1]: https://godbolt.org/z/c4dhaKbqe Would you like me to add something similar to Rust's black_box glibc-wide in a header (if so, what would be a good name?), or should I just do it locally in tst-fortify.c where I need it? > > Rust's black_box is / was [0] instead implemented as > > > > llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); > > What the 'volatile' constraint does for the llvm_asm? Is is to mimic a > 'asm volatile' or is something else? Yes, I believe it was the same thing as 'asm volatile'. This is because llvm_asm!() was not a special *syntax* like inline asm is in GCC, but a magic macro, so 'volatile' has to go inside the macro. But also note that llvm_asm!() has been deprecated and removed [2] (it was always an unstable feature, never intended to be stabilized); it's been replaced by the new asm!() macro that has a different mini-syntax (more like the Rust formatting macros, less like GCC/LLVM inline assembly). [2]: https://github.com/rust-lang/rust/pull/92816 With the asm!() macro the same would rather look like this: asm!( "/* pretend to use {0} */", in(reg) &mut dummy, options(nostack, preserves_flags) ) The default around 'volatile' (and other flags) has been flipped, now you'd need to specify 'options(pure)' to get the previous non-volatile behavior. > It is exported because all tests are actually built with _GNU_SOURCE (done > by include/libc-symbols.h), so the test check is superfluous. It also > leaks implementation details, such as internal defines. Usually to check > for internal implementation we use test-internal (which are built > statically). > > But if you really want to check for _LARGEFILE64_SOURCE, you will need to > add *another* fortify test that undef _GNU_SOURCE (like stdlib/tst-strtol-binary-c11.c > for instance). I thought the src-chk-nongnu = \#undef _GNU_SOURCE part handled undoing the #define _GNU_SOURCE (from include/libc-symbols.h) for the -nongnu- tests? I also think I might have actually gotten errors here about missing fcntl64 and the like before I added the ifdef check, but I might be misremembering this one. Sergey
* Adhemerval Zanella Netto: > On 29/05/23 14:31, Sergey Bugaev wrote: >> Hello, >> >> On Mon, May 29, 2023 at 7:54 PM Adhemerval Zanella Netto >> <adhemerval.zanella@linaro.org> wrote: >>> Some comments below. >> >> thank you for the review :) >> >>>> +/* Return VALUE, but do it in a way that the compiler cannot >>>> + see that it's a compile-time constant. */ >>>> +static int __attribute_noinline__ >>>> +hide_constant (int value) >>>> +{ >>>> + volatile int v = value; >>>> + return v; >>>> +} >>>> + >>> >>> Interesting construct, but I wonder if 'volatile' is really required here. >>> Afaiu what really prevents compiler to return fortify compiler warning here >>> is the __attribute_noinline__. >> >> It's not really required. Apparently even an 'inline' function call is >> enough to to disarm __builtin_constant_p [0]; I just threw both >> volatile and noinline in for good measure.> >> [0] https://godbolt.org/z/v9569KGTT > > It is because afaiu 'volatile' does not really match the semantics of what you > are trying to do here. The noinline attribute does exactly what it says: it prevents inlining, but it does not prevent inter-procedural analysis. So if a function returns a constrant, the compiler will still use that constant elsewhere. Newer GCC versions support the noipa attribute. If you drop the static, you can use the weak attribute for compatibility with pretty much all GCC versions. But I think using volatile (without noinline for clarity) is fine here. >>>> +template<typename, typename> >>>> +struct __fcntl_types_compatible_helper >>> >>> This makes the C++ tests fail with GCC 13: >>> >>> [...] >>> ../include/bits/../../io/bits/fcntl2.h:460:1: error: template with C linkage >>> 460 | template<typename __T> >>> | ^~~~~~~~ >>> ../misc/sys/cdefs.h:140:25: note: ‘extern "C"’ linkage started here >>> 140 | # define __BEGIN_DECLS extern "C" { >> >> Interesting. Any idea why this wasn't failing in my testing? > > My guess is because template with extern "C" are not really supported in all > configurations. It's possible to use extern "C++" around the template. >> So maybe it is a good idea to move this all into fcntl3.h like it was >> done in Florian's patch? Then I'd include that after __END_DECLS and >> only if GCC >= 8. > > Yes, it would be better indeed. Even then, we may still have to use extern "C++", to preserve compatibility with C++ applications which include <fcntl.h> in an extern "C" block. See this commit: commit e37208ce86916af9510ffb9ce7b3c187986f07de Author: Florian Weimer <fweimer@redhat.com> Date: Sat Oct 22 17:33:26 2016 +0200 math.h: Wrap C++ bits in extern "C++" It is still common to include system header files in an extern "C" block. This means that exiting <math.h>'s own extern "C" block is not sufficient to get back to C++ mode. Use an extern "C++" wrapper instead. Thanks, Florian
Hello, On Tue, May 30, 2023 at 10:41 AM Florian Weimer <fweimer@redhat.com> wrote: > The noinline attribute does exactly what it says: it prevents inlining, > but it does not prevent inter-procedural analysis. So if a function > returns a constrant, the compiler will still use that constant > elsewhere. Newer GCC versions support the noipa attribute. If you drop > the static, you can use the weak attribute for compatibility with pretty > much all GCC versions. > > But I think using volatile (without noinline for clarity) is fine here. So do I read this right that you're saying I should just do static int hide_constant (int value) { volatile int v = value; return v; } ? After the yesterday's discussion with Adhemerval, I was thinking of doing static inline int hide_constant (int value) { asm ("" : "+rm" (value)); return value; } This does *not* prevent the whole computation from being optimized out if the result is unused (i.e. there's no 'volatile'), but it does prevent the compiler from assuming anything about the return value. > > My guess is because template with extern "C" are not really supported in all > > configurations. > > It's possible to use extern "C++" around the template. I didn't realize it was possible to nest the extern "..." { } blocks, thank you. Sergey
* Sergey Bugaev: > Hello, > > On Tue, May 30, 2023 at 10:41 AM Florian Weimer <fweimer@redhat.com> wrote: >> The noinline attribute does exactly what it says: it prevents inlining, >> but it does not prevent inter-procedural analysis. So if a function >> returns a constrant, the compiler will still use that constant >> elsewhere. Newer GCC versions support the noipa attribute. If you drop >> the static, you can use the weak attribute for compatibility with pretty >> much all GCC versions. >> >> But I think using volatile (without noinline for clarity) is fine here. > > So do I read this right that you're saying I should just do > > static int > hide_constant (int value) > { > volatile int v = value; > return v; > } > > ? After the yesterday's discussion with Adhemerval, I was thinking of doing > > static inline int > hide_constant (int value) > { > asm ("" : "+rm" (value)); > return value; > } > > This does *not* prevent the whole computation from being optimized out > if the result is unused (i.e. there's no 'volatile'), but it does > prevent the compiler from assuming anything about the return value. I think both variants are fine in this case. If Adhemerval prefers the second, please use that. Thanks, Florian
On 29/05/23 18:59, Sergey Bugaev wrote: > On Tue, May 30, 2023 at 12:09 AM Adhemerval Zanella Netto > <adhemerval.zanella@linaro.org> wrote: >> It is returned unmodified, but the asm acts a compiler barrier which gcc >> documentation also declares as 'strong memory barrier' [1] (which I think was >> written before C11 memory semantic). >> >> [1] https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html > > But that one is talking about load/store reordering, not constant > propagation. Here's what I'm talking about: [0] (and an actual Rust > version at [1]). > > [0]: https://godbolt.org/z/qaMa7EavY > [1]: https://godbolt.org/z/c4dhaKbqe > > Would you like me to add something similar to Rust's black_box > glibc-wide in a header (if so, what would be a good name?), or should > I just do it locally in tst-fortify.c where I need it? Ah, I see it now. I am not sure if it would change the benchmarks we have, since the idea is only to prevent compiler optimize away function calls that might have no side-effects. But it does seems a useful addition for internal header, maybe tune the name a bit. > >>> Rust's black_box is / was [0] instead implemented as >>> >>> llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); >> >> What the 'volatile' constraint does for the llvm_asm? Is is to mimic a >> 'asm volatile' or is something else? > > Yes, I believe it was the same thing as 'asm volatile'. This is > because llvm_asm!() was not a special *syntax* like inline asm is in > GCC, but a magic macro, so 'volatile' has to go inside the macro. > > But also note that llvm_asm!() has been deprecated and removed [2] (it > was always an unstable feature, never intended to be stabilized); it's > been replaced by the new asm!() macro that has a different mini-syntax > (more like the Rust formatting macros, less like GCC/LLVM inline > assembly). > > [2]: https://github.com/rust-lang/rust/pull/92816 > > With the asm!() macro the same would rather look like this: > > asm!( > "/* pretend to use {0} */", > in(reg) &mut dummy, > options(nostack, preserves_flags) > ) > > The default around 'volatile' (and other flags) has been flipped, now > you'd need to specify 'options(pure)' to get the previous non-volatile > behavior. Thanks for the explanation. > >> It is exported because all tests are actually built with _GNU_SOURCE (done >> by include/libc-symbols.h), so the test check is superfluous. It also >> leaks implementation details, such as internal defines. Usually to check >> for internal implementation we use test-internal (which are built >> statically). >> >> But if you really want to check for _LARGEFILE64_SOURCE, you will need to >> add *another* fortify test that undef _GNU_SOURCE (like stdlib/tst-strtol-binary-c11.c >> for instance). > > I thought the > > src-chk-nongnu = \#undef _GNU_SOURCE > > part handled undoing the #define _GNU_SOURCE (from > include/libc-symbols.h) for the -nongnu- tests? But at same time we always use -D_LARGEFILE64_SOURCE=1 for nognu: CFLAGS-tst-fortify-$(1)-nongnu-$(2)-$(3).$(1) += -D_LARGEFILE64_SOURCE=1 We have some overlap on current way to organize tst-fortify, but I think it should cover everything: 1. tst-fortify-c-default: uses _GNU_SOURCE, provides both LFS and non-LFS, no redirections. 2. tst-fortify-c-lfs: uses _GNU_SOURCE and _FILE_OFFSET_BITS=64, provides both LFS and non-LFS, redirects non-LFS calls to LFS. 3. tst-fortify-c-nongnu: undef _GNU_SOURCE, defines _LARGEFILE64_SOURCE, provides both LFS and non-LFS, no redirections. I guess you might add another configuration to undef _GNU_SOURCE and define _LARGEFILE64_SOURCE; but I don't think it would increase coverage. > > I also think I might have actually gotten errors here about missing > fcntl64 and the like before I added the ifdef check, but I might be > misremembering this one. > > Sergey
On 30/05/23 06:50, Florian Weimer wrote: > * Sergey Bugaev: > >> Hello, >> >> On Tue, May 30, 2023 at 10:41 AM Florian Weimer <fweimer@redhat.com> wrote: >>> The noinline attribute does exactly what it says: it prevents inlining, >>> but it does not prevent inter-procedural analysis. So if a function >>> returns a constrant, the compiler will still use that constant >>> elsewhere. Newer GCC versions support the noipa attribute. If you drop >>> the static, you can use the weak attribute for compatibility with pretty >>> much all GCC versions. >>> >>> But I think using volatile (without noinline for clarity) is fine here. >> >> So do I read this right that you're saying I should just do >> >> static int >> hide_constant (int value) >> { >> volatile int v = value; >> return v; >> } >> >> ? After the yesterday's discussion with Adhemerval, I was thinking of doing >> >> static inline int >> hide_constant (int value) >> { >> asm ("" : "+rm" (value)); >> return value; >> } >> >> This does *not* prevent the whole computation from being optimized out >> if the result is unused (i.e. there's no 'volatile'), but it does >> prevent the compiler from assuming anything about the return value. > > I think both variants are fine in this case. If Adhemerval prefers the > second, please use that. Using the second allows us to provide an internal macro that can consolidate the DO_NOT_OPTIMIZE on benchtests as well.
diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c index 7850a4e5..03264154 100644 --- a/debug/tst-fortify.c +++ b/debug/tst-fortify.c @@ -36,6 +36,8 @@ #include <sys/select.h> #include <sys/socket.h> #include <sys/un.h> +#include <support/support.h> +#include <support/xunistd.h> #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 int __attribute_noinline__ +hide_constant (int value) +{ + volatile int v = value; + return v; +} + volatile int chk_fail_ok; volatile int ret; jmp_buf chk_fail_buf; @@ -1763,6 +1774,143 @@ 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); + +#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); + + struct flock flock; + int ofd_locks_supported = support_fcntl_support_ofd_locks (lockfd1); + + if (ofd_locks_supported) + { + 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) + { + 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 __USE_FORTIFY_LEVEL >= 1 + CHK_FAIL_START + fcntl (STDIN_FILENO, hide_constant (F_SETFD)); + CHK_FAIL_END +#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); + +#ifdef F_OFD_GETLK + 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 + +# if __USE_FORTIFY_LEVEL >= 1 + CHK_FAIL_START + fcntl64 (STDIN_FILENO, hide_constant (F_SETFD)); + CHK_FAIL_END +# endif #endif return ret; diff --git a/include/fcntl.h b/include/fcntl.h index be435047..cb86c5e7 100644 --- a/include/fcntl.h +++ b/include/fcntl.h @@ -32,6 +32,7 @@ extern int __open64_2 (const char *__path, int __oflag); extern int __openat_2 (int __fd, const char *__path, int __oflag); extern int __openat64_2 (int __fd, const char *__path, int __oflag); +extern int __fcntl_2 (int __fd, int __cmd); #if IS_IN (rtld) # include <dl-fcntl.h> diff --git a/io/Makefile b/io/Makefile index 6b58728e..1773a0f2 100644 --- a/io/Makefile +++ b/io/Makefile @@ -71,6 +71,7 @@ routines := \ fchownat \ fcntl \ fcntl64 \ + fcntl_2 \ file_change_detection \ flock \ fstat \ diff --git a/io/Versions b/io/Versions index 4e195408..0e77a287 100644 --- a/io/Versions +++ b/io/Versions @@ -140,6 +140,9 @@ libc { GLIBC_2.34 { closefrom; } + GLIBC_2.38 { + __fcntl_2; + } GLIBC_PRIVATE { __libc_fcntl64; __fcntl_nocancel; diff --git a/io/bits/fcntl2.h b/io/bits/fcntl2.h index bdb48fa8..e8dd3dea 100644 --- a/io/bits/fcntl2.h +++ b/io/bits/fcntl2.h @@ -170,3 +170,400 @@ openat64 (int __fd, const char *__path, int __oflag, ...) } # endif #endif + +extern int __fcntl_2 (int __fd, int __cmd); + +#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: + case F_DUPFD_CLOEXEC: + 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: + case F_ADD_SEALS: + case F_GET_RW_HINT: + case F_SET_RW_HINT: + 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: + case F_DUPFD_CLOEXEC: + 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: +#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 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 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 + +template<typename, typename> +struct __fcntl_types_compatible_helper +{ + __always_inline static int + __compatible () + { + return 0; + } +}; + +template<typename __T> +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 () + +#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(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 (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_2 (__fd, __cmd); + + if (__fcntl_requires_arg (__cmd)) + __fcntl_missing_arg (); + + return __fcntl_alias (__fd, __cmd); +} + +__glibc_warn_system_headers_begin + +#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_2.c b/io/fcntl_2.c new file mode 100644 index 00000000..8db79c2a --- /dev/null +++ b/io/fcntl_2.c @@ -0,0 +1,33 @@ +/* _FORTIFY_SOURCE wrapper for fcntl. + Copyright (C) 2013-2023 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 + <https://www.gnu.org/licenses/>. */ + +/* Make sure to get __fcntl_requires_arg from bits/fcntl2.h */ +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 1 + +#include <fcntl.h> +#include <stdio.h> + +int +__fcntl_2 (int fd, int cmd) +{ + if (__fcntl_requires_arg (cmd)) + __fortify_fail ("invalid fcntl call: this command requires an argument"); + + return __libc_fcntl64 (fd, cmd); +} diff --git a/manual/maint.texi b/manual/maint.texi index a8441e20..74647e40 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 @@ -243,10 +244,11 @@ depending on the architecture, one may also see fortified variants have the @code{_chkieee128} suffix or the @code{__nldbl___} prefix to their names. -Another exception is the @code{open} family of functions, where their -fortified replacements have the @code{__} prefix and a @code{_2} suffix. -The @code{FD_SET}, @code{FD_CLR} and @code{FD_ISSET} macros use the -@code{__fdelt_chk} function on fortification. +Another exception is the @code{open} and @code{fcntl} families of +functions, where their fortified 2-argument version replacements have the +@code{__} prefix and a @code{_2} suffix. The @code{FD_SET}, @code{FD_CLR} +and @code{FD_ISSET} macros use the @code{__fdelt_chk} function on +fortification. The following functions and macros are fortified in @theglibc{}: @c Generated using the following command: @@ -255,6 +257,7 @@ The following functions and macros are fortified in @theglibc{}: @c sort -u | grep ^__ | @c grep -v -e ieee128 -e __nldbl -e align_cpy -e "fdelt_warn" | @c sed 's/__fdelt_chk/@item @code{FD_SET}\n\n@item @code{FD_CLR}\n\n@item @code{FD_ISSET}\n/' | +@c sed 's/__fcntl_2/@item @code{fcntl}\n\n@item @code{fcntl64}\n/' | @c sed 's/__\(.*\)_\(chk\|2\)/@item @code{\1}\n/' @itemize @bullet @@ -267,6 +270,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/sysdeps/mach/hurd/i386/libc.abilist b/sysdeps/mach/hurd/i386/libc.abilist index 6925222f..5397289b 100644 --- a/sysdeps/mach/hurd/i386/libc.abilist +++ b/sysdeps/mach/hurd/i386/libc.abilist @@ -2294,6 +2294,7 @@ GLIBC_2.36 arc4random_buf F GLIBC_2.36 arc4random_uniform F GLIBC_2.36 c8rtomb F GLIBC_2.36 mbrtoc8 F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/mach/hurd/x86_64/libc.abilist b/sysdeps/mach/hurd/x86_64/libc.abilist index a0be5c1a..6ed14d1f 100644 --- a/sysdeps/mach/hurd/x86_64/libc.abilist +++ b/sysdeps/mach/hurd/x86_64/libc.abilist @@ -194,6 +194,7 @@ GLIBC_2.38 __errno_location F GLIBC_2.38 __explicit_bzero_chk F GLIBC_2.38 __fbufsize F GLIBC_2.38 __fcntl F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __fdelt_chk F GLIBC_2.38 __fdelt_warn F GLIBC_2.38 __fentry__ F diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist index 0e2d9c30..6a3e3e7c 100644 --- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist +++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist @@ -2633,6 +2633,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist index f1bec197..4cbc9346 100644 --- a/sysdeps/unix/sysv/linux/alpha/libc.abilist +++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist @@ -2730,6 +2730,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/arc/libc.abilist b/sysdeps/unix/sysv/linux/arc/libc.abilist index aa874b88..01fa7a34 100644 --- a/sysdeps/unix/sysv/linux/arc/libc.abilist +++ b/sysdeps/unix/sysv/linux/arc/libc.abilist @@ -2394,6 +2394,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist index afbd57da..2d7f5db4 100644 --- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist +++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist @@ -514,6 +514,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist index e7364cd3..85eea0e5 100644 --- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist +++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist @@ -511,6 +511,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist index 913fa592..0bc57ae1 100644 --- a/sysdeps/unix/sysv/linux/csky/libc.abilist +++ b/sysdeps/unix/sysv/linux/csky/libc.abilist @@ -2670,6 +2670,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist index 43af3a98..4f2e8b5d 100644 --- a/sysdeps/unix/sysv/linux/hppa/libc.abilist +++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist @@ -2619,6 +2619,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist index af72f8fa..8b8b8789 100644 --- a/sysdeps/unix/sysv/linux/i386/libc.abilist +++ b/sysdeps/unix/sysv/linux/i386/libc.abilist @@ -2803,6 +2803,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist index 48cbb0fa..22f7f159 100644 --- a/sysdeps/unix/sysv/linux/ia64/libc.abilist +++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist @@ -2568,6 +2568,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist b/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist index c15884bb..acb9a793 100644 --- a/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist +++ b/sysdeps/unix/sysv/linux/loongarch/lp64/libc.abilist @@ -2154,6 +2154,7 @@ GLIBC_2.36 wprintf F GLIBC_2.36 write F GLIBC_2.36 writev F GLIBC_2.36 wscanf F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist index 3738db81..af6762d1 100644 --- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist @@ -515,6 +515,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist index ed136277..19ac2156 100644 --- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist @@ -2746,6 +2746,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist index 83577386..6d7e9d95 100644 --- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist +++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist @@ -2719,6 +2719,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist index 58c5da58..08459ff7 100644 --- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist +++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist @@ -2716,6 +2716,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist index d3741945..3b9caff0 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist @@ -2711,6 +2711,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist index 5319fdc2..2eb10c81 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist @@ -2709,6 +2709,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist index 1743ea6e..7f13cd56 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist @@ -2717,6 +2717,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist index 9b1f53c6..adea92bf 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist @@ -2619,6 +2619,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist index ae1c6ca1..2816e0b3 100644 --- a/sysdeps/unix/sysv/linux/nios2/libc.abilist +++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist @@ -2758,6 +2758,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/or1k/libc.abilist b/sysdeps/unix/sysv/linux/or1k/libc.abilist index a7c572c9..93674747 100644 --- a/sysdeps/unix/sysv/linux/or1k/libc.abilist +++ b/sysdeps/unix/sysv/linux/or1k/libc.abilist @@ -2140,6 +2140,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist index 074fa031..68aa8601 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist @@ -2773,6 +2773,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist index dfcb4bd2..bc2f2b8d 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist @@ -2806,6 +2806,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist index 63bbccf3..cdc982a3 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist @@ -2527,6 +2527,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist index ab85fd61..66733662 100644 --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist @@ -2829,6 +2829,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fscanfieee128 F GLIBC_2.38 __isoc23_fwscanf F diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist index b716f5c7..6cf51998 100644 --- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist +++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist @@ -2396,6 +2396,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist index 774e777b..84f920f2 100644 --- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist +++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist @@ -2596,6 +2596,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist index 8625135c..c80822de 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist +++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist @@ -2771,6 +2771,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist index d00c7eb2..ac344828 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist +++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist @@ -2564,6 +2564,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist index b6303724..c064e000 100644 --- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist +++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist @@ -2626,6 +2626,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist index d8005561..a185e3cb 100644 --- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist +++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist @@ -2623,6 +2623,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist index 5be55c11..ad5aa083 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist @@ -2766,6 +2766,7 @@ GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F GLIBC_2.37 __ppoll64_chk F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist index 475fdaae..27fffe9d 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist @@ -2591,6 +2591,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist index 6cfb928b..3c46f003 100644 --- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist @@ -2542,6 +2542,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist index c7350971..5a4fa94c 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist @@ -2648,6 +2648,7 @@ GLIBC_2.36 pidfd_open F GLIBC_2.36 pidfd_send_signal F GLIBC_2.36 process_madvise F GLIBC_2.36 process_mrelease F +GLIBC_2.38 __fcntl_2 F GLIBC_2.38 __isoc23_fscanf F GLIBC_2.38 __isoc23_fwscanf F GLIBC_2.38 __isoc23_scanf F
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 (). Recently, Hector Martin <marcan@marcan.st> has identified an issue in libwebrtc where fcntl (fd, F_DUPFD_CLOEXEC) was invoked without a third argument [0]. With the patch, the bug would have been detected at compile time, assuming libwebrtc is built with FORTIFY_SOURCE. Hopefully this will help detecting similar bugs in existing software, and prevent more instances of similar bugs from being introduced in the future. [0]: https://social.treehouse.systems/@marcan/110355001391961285 Add tests for fcntl () and fcntl64 () calls still compiling and functioning correctly in various configurations, and mention the new fortification in the manual. The abilists have been modified with 'make update-abi-all'. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- debug/tst-fortify.c | 148 +++++++ include/fcntl.h | 1 + io/Makefile | 1 + io/Versions | 3 + io/bits/fcntl2.h | 397 ++++++++++++++++++ io/fcntl_2.c | 33 ++ manual/maint.texi | 19 +- sysdeps/mach/hurd/i386/libc.abilist | 1 + sysdeps/mach/hurd/x86_64/libc.abilist | 1 + sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 + sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 + sysdeps/unix/sysv/linux/arc/libc.abilist | 1 + sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 + sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 + sysdeps/unix/sysv/linux/csky/libc.abilist | 1 + sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 + sysdeps/unix/sysv/linux/i386/libc.abilist | 1 + sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 + .../sysv/linux/loongarch/lp64/libc.abilist | 1 + .../sysv/linux/m68k/coldfire/libc.abilist | 1 + .../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 + .../sysv/linux/microblaze/be/libc.abilist | 1 + .../sysv/linux/microblaze/le/libc.abilist | 1 + .../sysv/linux/mips/mips32/fpu/libc.abilist | 1 + .../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 + .../sysv/linux/mips/mips64/n32/libc.abilist | 1 + .../sysv/linux/mips/mips64/n64/libc.abilist | 1 + sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 + sysdeps/unix/sysv/linux/or1k/libc.abilist | 1 + .../linux/powerpc/powerpc32/fpu/libc.abilist | 1 + .../powerpc/powerpc32/nofpu/libc.abilist | 1 + .../linux/powerpc/powerpc64/be/libc.abilist | 1 + .../linux/powerpc/powerpc64/le/libc.abilist | 1 + .../unix/sysv/linux/riscv/rv32/libc.abilist | 1 + .../unix/sysv/linux/riscv/rv64/libc.abilist | 1 + .../unix/sysv/linux/s390/s390-32/libc.abilist | 1 + .../unix/sysv/linux/s390/s390-64/libc.abilist | 1 + sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 + sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 + .../sysv/linux/sparc/sparc32/libc.abilist | 1 + .../sysv/linux/sparc/sparc64/libc.abilist | 1 + .../unix/sysv/linux/x86_64/64/libc.abilist | 1 + .../unix/sysv/linux/x86_64/x32/libc.abilist | 1 + 43 files changed, 632 insertions(+), 6 deletions(-) create mode 100644 io/fcntl_2.c