From: Paul Eggert Date: Mon, 22 Sep 2025 23:41:41 +0000 (-0700) Subject: fchownat: improve on test failure fix X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f657c55c682a09006a6278adfca2f0ed146049fe;p=thirdparty%2Fgnulib.git fchownat: improve on test failure fix * lib/fchownat.c (rpl_fchownat): Clear the flag only if the trailing slash check needs to be made. Do this before checking for the nofollow bug, to avoid the need for the fork+chdir+lchown dance in that case. Move the empty-filename check earlier, so that its file[0] check can more easily be combined with the trailing slash check. * m4/chown.m4, m4/fchownat.m4, modules/fchownat: Revert most recent change. --- diff --git a/ChangeLog b/ChangeLog index e845a3c76e..04bdb80fee 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2025-09-22 Paul Eggert + + fchownat: improve on test failure fix + * lib/fchownat.c (rpl_fchownat): + Clear the flag only if the trailing slash check needs to be made. + Do this before checking for the nofollow bug, to + avoid the need for the fork+chdir+lchown dance in that case. + Move the empty-filename check earlier, so that its file[0] check + can more easily be combined with the trailing slash check. + * m4/chown.m4, m4/fchownat.m4, modules/fchownat: + Revert most recent change. + 2025-09-22 Bruno Haible fchownat: Fix test failure on OpenBSD and Cygwin 2.9 (regr. 2025-09-20). diff --git a/lib/fchownat.c b/lib/fchownat.c index a5e8bb8f47..1c5a210d9a 100644 --- a/lib/fchownat.c +++ b/lib/fchownat.c @@ -113,17 +113,22 @@ rpl_fchownat (int fd, char const *file, uid_t owner, gid_t group, int flag) or CHOWN_MODIFIES_SYMLINK, as no known fchownat implementations have these bugs. */ -# if FCHOWNAT_NOFOLLOW_BUG - if (flag == AT_SYMLINK_NOFOLLOW) - return local_lchownat (fd, file, owner, group); -# endif - if (FCHOWNAT_EMPTY_FILENAME_BUG && file[0] == '\0') { errno = ENOENT; return -1; } + bool trailing_slash_check = (CHOWN_TRAILING_SLASH_BUG + && file[0] && file[strlen (file) - 1] == '/'); + if (trailing_slash_check) + flag &= ~AT_SYMLINK_NOFOLLOW; + +# if FCHOWNAT_NOFOLLOW_BUG + if (flag == AT_SYMLINK_NOFOLLOW) + return local_lchownat (fd, file, owner, group); +# endif + struct stat st; gid_t no_gid = -1; uid_t no_uid = -1; @@ -131,9 +136,7 @@ rpl_fchownat (int fd, char const *file, uid_t owner, gid_t group, int flag) bool uid_noop = owner == no_uid; bool change_time_check = CHOWN_CHANGE_TIME_BUG && !(gid_noop & uid_noop); - if (change_time_check - || (CHOWN_TRAILING_SLASH_BUG - && file[0] && file[strlen (file) - 1] == '/')) + if (change_time_check | trailing_slash_check) { int r = fstatat (fd, file, &st, flag); @@ -141,8 +144,6 @@ rpl_fchownat (int fd, char const *file, uid_t owner, gid_t group, int flag) trailing slash check needs. */ if (r < 0 && (change_time_check || errno != EOVERFLOW)) return r; - - flag &= ~AT_SYMLINK_NOFOLLOW; } int result = fchownat (fd, file, owner, group, flag); diff --git a/m4/chown.m4 b/m4/chown.m4 index 3b377f2989..f899f3b680 100644 --- a/m4/chown.m4 +++ b/m4/chown.m4 @@ -1,5 +1,5 @@ # chown.m4 -# serial 40 +# serial 39 dnl Copyright (C) 1997-2001, 2003-2005, 2007, 2009-2025 Free Software dnl Foundation, Inc. dnl This file is free software; the Free Software Foundation @@ -130,7 +130,39 @@ AC_DEFUN_ONCE([gl_FUNC_CHOWN], ;; esac - gl_FUNC_CHOWN_CTIME + dnl OpenBSD fails to update ctime if ownership does not change. + AC_CACHE_CHECK([whether chown updates ctime per POSIX], + [gl_cv_func_chown_ctime_works], + [dnl This test is tricky as it depends on timing and file timestamp + dnl resolution, and there were false positives when configuring with + dnl Linux fakeroot. Since the problem occurs only on OpenBSD and Cygwin, + dnl test only on these platforms. + AS_CASE([$host_os], + [openbsd* | cygwin*], + [AC_RUN_IFELSE([AC_LANG_PROGRAM([[ +#include +#include +#include +#include +#include +]GL_MDA_DEFINES], + [[struct stat st1, st2; + if (close (creat ("conftest.file", 0600))) return 1; + if (stat ("conftest.file", &st1)) return 2; + sleep (1); + if (chown ("conftest.file", st1.st_uid, st1.st_gid)) return 3; + if (stat ("conftest.file", &st2)) return 4; + if (st2.st_ctime <= st1.st_ctime) return 5; + ]])], + [gl_cv_func_chown_ctime_works=yes], + [gl_cv_func_chown_ctime_works=no], + [# Obey --enable-cross-guesses. + gl_cv_func_chown_ctime_works="$gl_cross_guess_normal" + ]) + rm -f conftest.file + ], + [gl_cv_func_chown_ctime_works=yes]) + ]) case "$gl_cv_func_chown_ctime_works" in *yes) ;; *) @@ -189,47 +221,3 @@ AC_DEFUN_ONCE([gl_FUNC_CHOWN_FOLLOWS_SYMLINK], ;; esac ]) - -AC_DEFUN_ONCE([gl_FUNC_CHOWN_CTIME], -[ - AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles - AC_CHECK_FUNCS_ONCE([chown]) - - dnl mingw lacks chown altogether. - if test $ac_cv_func_chown != no; then - dnl OpenBSD and Cygwin 2.9.0 fail to update ctime if ownership does not - dnl change. - AC_CACHE_CHECK([whether chown updates ctime per POSIX], - [gl_cv_func_chown_ctime_works], - [dnl This test is tricky as it depends on timing and file timestamp - dnl resolution, and there were false positives when configuring with - dnl Linux fakeroot. Since the problem occurs only on OpenBSD and Cygwin, - dnl test only on these platforms. - AS_CASE([$host_os], - [openbsd* | cygwin*], - [AC_RUN_IFELSE([AC_LANG_PROGRAM([[ -#include -#include -#include -#include -#include -]GL_MDA_DEFINES], - [[struct stat st1, st2; - if (close (creat ("conftest.file", 0600))) return 1; - if (stat ("conftest.file", &st1)) return 2; - sleep (1); - if (chown ("conftest.file", st1.st_uid, st1.st_gid)) return 3; - if (stat ("conftest.file", &st2)) return 4; - if (st2.st_ctime <= st1.st_ctime) return 5; - ]])], - [gl_cv_func_chown_ctime_works=yes], - [gl_cv_func_chown_ctime_works=no], - [# Obey --enable-cross-guesses. - gl_cv_func_chown_ctime_works="$gl_cross_guess_normal" - ]) - rm -f conftest.file - ], - [gl_cv_func_chown_ctime_works=yes]) - ]) - fi -]) diff --git a/m4/fchownat.m4 b/m4/fchownat.m4 index bad9b58360..300a254225 100644 --- a/m4/fchownat.m4 +++ b/m4/fchownat.m4 @@ -1,5 +1,5 @@ # fchownat.m4 -# serial 9 +# serial 8 dnl Copyright (C) 2004-2025 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -46,18 +46,16 @@ AC_DEFUN([gl_FUNC_FCHOWNAT_DEREF_BUG], AC_CACHE_CHECK([whether fchownat works with AT_SYMLINK_NOFOLLOW], [gl_cv_func_fchownat_nofollow_works], - [gl_FUNC_CHOWN_CTIME - case "$gl_cv_func_chown_ctime_works" in - *yes) - gl_dangle=conftest.dangle - # Remove any remnants of a previous test. - rm -f $gl_dangle - # Arrange for deletion of the temporary file this test creates. - ac_clean_files="$ac_clean_files $gl_dangle" - ln -s conftest.no-such $gl_dangle - AC_RUN_IFELSE( - [AC_LANG_SOURCE( - [[ + [ + gl_dangle=conftest.dangle + # Remove any remnants of a previous test. + rm -f $gl_dangle + # Arrange for deletion of the temporary file this test creates. + ac_clean_files="$ac_clean_files $gl_dangle" + ln -s conftest.no-such $gl_dangle + AC_RUN_IFELSE( + [AC_LANG_SOURCE( + [[ #include #include /* Android 4.3 declares fchownat() in instead. */ @@ -72,27 +70,12 @@ main () AT_SYMLINK_NOFOLLOW) != 0 && errno == ENOENT); } - ]])], - [gl_cv_func_fchownat_nofollow_works=yes], - [gl_cv_func_fchownat_nofollow_works=no], - [gl_cv_func_fchownat_nofollow_works="$gl_cross_guess_normal"]) - ;; - *) - dnl On OpenBSD and Cygwin 2.9.0, the test above would produce - dnl gl_cv_func_fchownat_nofollow_works=yes, and this would then - dnl lead to an fchownat test failure: - dnl test-lchown.h:185: assertion 'st1.st_gid == st2.st_gid' failed - dnl Since testing for this bug directly is only possible on machines - dnl where the current user is in at least two groups, we use - dnl gl_FUNC_CHOWN_CTIME as a substitute test. - gl_cv_func_fchownat_nofollow_works="guessing no" - ;; - esac - ]) - case "$gl_cv_func_fchownat_nofollow_works" in - *yes) $2 ;; - *) $1 ;; - esac + ]])], + [gl_cv_func_fchownat_nofollow_works=yes], + [gl_cv_func_fchownat_nofollow_works=no], + [gl_cv_func_fchownat_nofollow_works="$gl_cross_guess_normal"]) + ]) + AS_IF([test "$gl_cv_func_fchownat_nofollow_works" != yes], [$1], [$2]) ]) # gl_FUNC_FCHOWNAT_EMPTY_FILENAME_BUG([ACTION-IF-BUGGY[, ACTION-IF-NOT_BUGGY]]) diff --git a/modules/fchownat b/modules/fchownat index 61e1d215a1..31e722c89c 100644 --- a/modules/fchownat +++ b/modules/fchownat @@ -5,7 +5,6 @@ Files: lib/fchownat.c lib/at-func.c m4/fchownat.m4 -m4/chown.m4 Depends-on: unistd-h