I audited gnulib's uses of fstatat and found one fishy one that doesn't
use AT_NO_AUTOMOUNT, namely, in fts.c where the follow-symlink branch
uses 'stat' whereas the no-follow-symlink branch uses fstatat without
AT_NO_AUTOMOUNT. I installed the first patch to cause it be consistent
in using AT_NO_AUTOMOUNT, which is also consistent with what glibc does
(though this doesn't necessarily mean it's right - perhaps fts should
have a new flag to control automounts, depending on what the user wants).
The second attached patch deprecates statat and lstatat, due to the
confusion already mentioned.
I haven't audited Gnulib's uses of 'stat' and 'lstat'.
From 44f347ce4009cd0100d0e6562939a032b16d6db1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 9 Mar 2022 11:54:13 -0800
Subject: [PATCH 1/2] fts: be consistent about AT_NO_AUTOMOUNT
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lib/fts.c (fts_stat): Use fstatat with AT_NO_AUTOMOUNT
consistently, instead of sometimes using stat (which implies
AT_NO_AUTOMOUNT) and sometimes using fstatat without AT_NO_AUTOMOUNT.
Remove a goto while we’re at it.
---
ChangeLog | 8 ++++++++
lib/fts.c | 34 +++++++++++++++++-----------------
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e3f0ed216c..58873a1762 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2022-03-09 Paul Eggert <egg...@cs.ucla.edu>
+
+ fts: be consistent about AT_NO_AUTOMOUNT
+ * lib/fts.c (fts_stat): Use fstatat with AT_NO_AUTOMOUNT
+ consistently, instead of sometimes using stat (which implies
+ AT_NO_AUTOMOUNT) and sometimes using fstatat without AT_NO_AUTOMOUNT.
+ Remove a goto while we’re at it.
+
2022-03-07 Pádraig Brady <p...@draigbrady.com>
fcntl-h: add AT_NO_AUTOMOUNT
diff --git a/lib/fts.c b/lib/fts.c
index 706c56c597..a1a7c09fdb 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1766,7 +1766,8 @@ fts_stat(FTS *sp, register FTSENT *p, bool follow)
{
struct stat *sbp = p->fts_statp;
- if (p->fts_level == FTS_ROOTLEVEL && ISSET(FTS_COMFOLLOW))
+ if (ISSET (FTS_LOGICAL)
+ || (ISSET (FTS_COMFOLLOW) && p->fts_level == FTS_ROOTLEVEL))
follow = true;
/*
@@ -1774,22 +1775,21 @@ fts_stat(FTS *sp, register FTSENT *p, bool follow)
* a stat(2). If that fails, check for a non-existent symlink. If
* fail, set the errno from the stat call.
*/
- if (ISSET(FTS_LOGICAL) || follow) {
- if (stat(p->fts_accpath, sbp)) {
- if (errno == ENOENT
- && lstat(p->fts_accpath, sbp) == 0) {
- __set_errno (0);
- return (FTS_SLNONE);
- }
- p->fts_errno = errno;
- goto err;
- }
- } else if (fstatat(sp->fts_cwd_fd, p->fts_accpath, sbp,
- AT_SYMLINK_NOFOLLOW)) {
- p->fts_errno = errno;
-err: memset(sbp, 0, sizeof(struct stat));
- return (FTS_NS);
- }
+ int flags = (follow ? 0 : AT_SYMLINK_NOFOLLOW) | AT_NO_AUTOMOUNT;
+ if (fstatat (sp->fts_cwd_fd, p->fts_accpath, sbp, flags) < 0)
+ {
+ if (follow && errno == ENOENT
+ && 0 <= fstatat (sp->fts_cwd_fd, p->fts_accpath, sbp,
+ AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT))
+ {
+ __set_errno (0);
+ return FTS_SLNONE;
+ }
+
+ p->fts_errno = errno;
+ memset (sbp, 0, sizeof *sbp);
+ return FTS_NS;
+ }
if (S_ISDIR(sbp->st_mode)) {
if (ISDOT(p->fts_name)) {
--
2.35.1
From eea9688d521634c58efa81130e509f647bbd9ff9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 9 Mar 2022 13:54:53 -0800
Subject: [PATCH 2/2] statat: now obsolete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lib/openat.h (statat, lstatat): Now deprecated.
All uses removed, and replaced with fstatat.
* modules/statat: Mark as obsolete, because it’s confusing:
it’s not clear whether it should use AT_NO_AUTOMOUNT,
which is implied by stat and by lstat, but not by fstatat.
* tests/test-statat.c: Disable deprecated-declarations warnings.
---
ChangeLog | 8 ++++++++
NEWS | 3 +++
lib/fchownat.c | 2 +-
lib/openat.h | 2 ++
lib/renameatu.c | 15 ++++++++-------
lib/unlinkat.c | 5 +++--
modules/fchownat | 1 -
modules/renameatu | 1 -
modules/statat | 7 +++++++
modules/unlinkat | 1 -
tests/test-statat.c | 4 ++++
11 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 58873a1762..294f6286f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
2022-03-09 Paul Eggert <egg...@cs.ucla.edu>
+ statat: now obsolete
+ * lib/openat.h (statat, lstatat): Now deprecated.
+ All uses removed, and replaced with fstatat.
+ * modules/statat: Mark as obsolete, because it’s confusing:
+ it’s not clear whether it should use AT_NO_AUTOMOUNT,
+ which is implied by stat and by lstat, but not by fstatat.
+ * tests/test-statat.c: Disable deprecated-declarations warnings.
+
fts: be consistent about AT_NO_AUTOMOUNT
* lib/fts.c (fts_stat): Use fstatat with AT_NO_AUTOMOUNT
consistently, instead of sometimes using stat (which implies
diff --git a/NEWS b/NEWS
index c13a115ada..8f90d8e958 100644
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,9 @@ User visible incompatible changes
Date Modules Changes
+2022-03-09 statat This module is deprecated. Use fstatat instead,
+ to specify whether you want AT_NO_AUTOMOUNT.
+
2022-01-05 stack This module now uses idx_t instead of size_t
for indexes and counts.
diff --git a/lib/fchownat.c b/lib/fchownat.c
index c0d97db98b..ff368d356d 100644
--- a/lib/fchownat.c
+++ b/lib/fchownat.c
@@ -103,7 +103,7 @@ rpl_fchownat (int fd, char const *file, uid_t owner, gid_t group, int flag)
struct stat st;
if (len && file[len - 1] == '/')
{
- if (statat (fd, file, &st))
+ if (fstatat (fd, file, &st, 0))
return -1;
if (flag == AT_SYMLINK_NOFOLLOW)
return fchownat (fd, file, owner, group, 0);
diff --git a/lib/openat.h b/lib/openat.h
index 5c8ff90b80..56919ef8dc 100644
--- a/lib/openat.h
+++ b/lib/openat.h
@@ -98,12 +98,14 @@ lchmodat (int fd, char const *file, mode_t mode)
# define STATAT_INLINE _GL_INLINE
# endif
+_GL_ATTRIBUTE_DEPRECATED
STATAT_INLINE int
statat (int fd, char const *name, struct stat *st)
{
return fstatat (fd, name, st, 0);
}
+_GL_ATTRIBUTE_DEPRECATED
STATAT_INLINE int
lstatat (int fd, char const *name, struct stat *st)
{
diff --git a/lib/renameatu.c b/lib/renameatu.c
index 0eb33ab6fd..b4e317d6e5 100644
--- a/lib/renameatu.c
+++ b/lib/renameatu.c
@@ -133,12 +133,13 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
break;
case RENAME_NOREPLACE:
- /* This has a race between the call to lstatat and the calls to
- renameat below. This lstatat is needed even if RENAME_EXCL
+ /* This has a race between the call to fstatat and the calls to
+ renameat below. This fstatat is needed even if RENAME_EXCL
is defined, because RENAME_EXCL is buggy on macOS 11.2:
renameatx_np (fd, "X", fd, "X", RENAME_EXCL) incorrectly
succeeds when X exists. */
- if (lstatat (fd2, dst, &dst_st) == 0 || errno == EOVERFLOW)
+ if (fstatat (fd2, dst, &dst_st, AT_SYMLINK_NOFOLLOW) == 0
+ || errno == EOVERFLOW)
return errno_fail (EEXIST);
if (errno != ENOENT)
return -1;
@@ -164,14 +165,14 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
the source does not exist, or if the destination cannot be turned
into a directory, give up now. Otherwise, strip trailing slashes
before calling rename. */
- if (lstatat (fd1, src, &src_st))
+ if (fstatat (fd1, src, &src_st, AT_SYMLINK_NOFOLLOW))
return -1;
if (dst_found_nonexistent)
{
if (!S_ISDIR (src_st.st_mode))
return errno_fail (ENOENT);
}
- else if (lstatat (fd2, dst, &dst_st))
+ else if (fstatat (fd2, dst, &dst_st, AT_SYMLINK_NOFOLLOW))
{
if (errno != ENOENT || !S_ISDIR (src_st.st_mode))
return -1;
@@ -196,7 +197,7 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
goto out;
}
strip_trailing_slashes (src_temp);
- if (lstatat (fd1, src_temp, &src_st))
+ if (fstatat (fd1, src_temp, &src_st, AT_SYMLINK_NOFOLLOW))
{
rename_errno = errno;
goto out;
@@ -213,7 +214,7 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
goto out;
}
strip_trailing_slashes (dst_temp);
- if (lstatat (fd2, dst_temp, &dst_st))
+ if (fstatat (fd2, dst_temp, &dst_st, AT_SYMLINK_NOFOLLOW))
{
if (errno != ENOENT)
{
diff --git a/lib/unlinkat.c b/lib/unlinkat.c
index eae60074d4..c9ff3ab2f1 100644
--- a/lib/unlinkat.c
+++ b/lib/unlinkat.c
@@ -58,7 +58,7 @@ rpl_unlinkat (int fd, char const *name, int flag)
rule of letting unlink("link-to-dir/") attempt to unlink a
directory. */
struct stat st;
- result = lstatat (fd, name, &st);
+ result = fstatat (fd, name, &st, AT_SYMLINK_NOFOLLOW);
if (result == 0 || errno == EOVERFLOW)
{
/* Trailing NUL will overwrite the trailing slash. */
@@ -71,7 +71,8 @@ rpl_unlinkat (int fd, char const *name, int flag)
memcpy (short_name, name, len);
while (len && ISSLASH (short_name[len - 1]))
short_name[--len] = '\0';
- if (len && (lstatat (fd, short_name, &st) || S_ISLNK (st.st_mode)))
+ if (len && (fstatat (fd, short_name, &st, AT_SYMLINK_NOFOLLOW)
+ || S_ISLNK (st.st_mode)))
{
free (short_name);
errno = EPERM;
diff --git a/modules/fchownat b/modules/fchownat
index ea2f2e678c..49c4b18096 100644
--- a/modules/fchownat
+++ b/modules/fchownat
@@ -19,7 +19,6 @@ lchown [test $HAVE_FCHOWNAT = 0 || test $REPLACE_FCHOWNAT = 1]
openat-die [test $HAVE_FCHOWNAT = 0 || test $REPLACE_FCHOWNAT = 1]
openat-h [test $HAVE_FCHOWNAT = 0 || test $REPLACE_FCHOWNAT = 1]
save-cwd [test $HAVE_FCHOWNAT = 0 || test $REPLACE_FCHOWNAT = 1]
-statat [test $REPLACE_FCHOWNAT = 1]
configure.ac:
gl_FUNC_FCHOWNAT
diff --git a/modules/renameatu b/modules/renameatu
index 5ce8e47587..e0c1ce3176 100644
--- a/modules/renameatu
+++ b/modules/renameatu
@@ -13,7 +13,6 @@ extensions
fcntl-h
filenamecat-lgpl [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1]
openat-h [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1]
-statat [test $REPLACE_RENAMEAT = 1]
stdbool [test $REPLACE_RENAMEAT = 1]
at-internal [test $HAVE_RENAMEAT = 0]
filename [test $HAVE_RENAMEAT = 0]
diff --git a/modules/statat b/modules/statat
index 074d342bd8..bf274e054d 100644
--- a/modules/statat
+++ b/modules/statat
@@ -1,6 +1,13 @@
Description:
statat() and lstatat() functions: Return info about a file at a directory.
+Status:
+obsolete
+
+Notice:
+This module is obsolete. Please use fstatat instead, and decide
+whether you want AT_NO_AUTOMOUNT in each fstatat call.
+
Files:
lib/statat.c
diff --git a/modules/unlinkat b/modules/unlinkat
index 0efc1cc1b2..ed11c1e971 100644
--- a/modules/unlinkat
+++ b/modules/unlinkat
@@ -13,7 +13,6 @@ extensions
fcntl-h [test $HAVE_UNLINKAT = 0 || test $REPLACE_UNLINKAT = 1]
openat-h [test $HAVE_UNLINKAT = 0 || test $REPLACE_UNLINKAT = 1]
sys_stat [test $HAVE_UNLINKAT = 0 || test $REPLACE_UNLINKAT = 1]
-statat [test $REPLACE_UNLINKAT = 1]
at-internal [test $HAVE_UNLINKAT = 0]
errno [test $HAVE_UNLINKAT = 0]
fchdir [test $HAVE_UNLINKAT = 0]
diff --git a/tests/test-statat.c b/tests/test-statat.c
index a573604e1c..f46c3f58b5 100644
--- a/tests/test-statat.c
+++ b/tests/test-statat.c
@@ -18,6 +18,10 @@
#include "openat.h"
+#if 4 < __GNUC__ + (3 <= __GNUC_MINOR__)
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
+
#include "signature.h"
SIGNATURE_CHECK (statat, int, (int, char const *, struct stat *));
SIGNATURE_CHECK (lstatat, int, (int, char const *, struct stat *));
--
2.35.1
