I installed the attached, which follows your suggestion of reverting the
*.m4 and modules/* changes, and which fixes fchmodat.c in what should be
a better way. Tested on OpenBSD 7.7 but only on a platform where I have
just one groupid.From f657c55c682a09006a6278adfca2f0ed146049fe Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Mon, 22 Sep 2025 16:41:41 -0700
Subject: [PATCH] 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.
---
ChangeLog | 12 ++++++++
lib/fchownat.c | 21 +++++++------
m4/chown.m4 | 80 ++++++++++++++++++++----------------------------
m4/fchownat.m4 | 51 ++++++++++--------------------
modules/fchownat | 1 -
5 files changed, 74 insertions(+), 91 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e845a3c76e..04bdb80fee 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2025-09-22 Paul Eggert <[email protected]>
+
+ 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 <[email protected]>
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 <unistd.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+]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 <unistd.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <sys/stat.h>
-]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 <fcntl.h>
#include <unistd.h>
/* Android 4.3 declares fchownat() in <sys/stat.h> 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
--
2.48.1
