On 07/03/2022 17:11, Paul Eggert wrote:
On 3/7/22 07:02, Pádraig Brady wrote:
When looking into https://bugs.gnu.org/54286
which discussed statx + AT_NO_AUTOMOUNT,
I was wondering about the fstatat changes in
https://github.com/coreutils/coreutils/commit/571f63f50
and whether some of those should also specify AT_NO_AUTOMOUNT?
I'm a bit lost, as I see no mention of fstatat in that commit.
That commit replaced some stat() calls with fstatat().
I was not aware of AT_NO_AUTOMOUNT until now, and am surprised about it.
POSIX says that stat(f,s) is supposed to be equivalent to
fstatat(AT_FDCWD,f,s,0). But if I understand things correctly, this
isn't true on GNU/Linux: you must pass AT_NO_AUTOMOUNT instead of 0. I
assume that this departure from POSIX was intentional, but even so it's
pretty confusing. Could you explain why AT_NO_AUTOMOUNT wasn't made the
default for fstatat?
Yes this is surprising.
I'm only going by the fstatat man page,
but I also see correlation of the issue in for e.g.:
https://bugs.python.org/issue35677
I've yet to dig into the original kernel discussions and implementation.
I suppose we first need to audit all of Gnulib's uses of fstatat and
statx to see whether they need AT_NO_AUTOMOUNT added. Once we're done
with that we can turn to Coreutils.
Yes I expect we'll have different treatments in different places.
Attached is an adjustment for ln in coreutils at least.
For example, what about the calls to fstatat in gnulib/lib/openat.h? Are
statat and lstatat intended as convenient aliases for fstatat, or as
directory-fd extensions to stat and lstat? If the former, they should be
left alone; if the latter, they need AT_NO_AUTOMOUNT. Or perhaps it
would be better to eliminate them and replace them with calls to
fstatat, since we'll need to decide on a call-by-call basis anyway.
I see unlinkat() uses lstatat(),
and in that context it would be better to specify AT_NO_AUTOMOUNT
I think since we're operating on the dir not the file itself.
Also agreed that eliminating these aliases
would avoid at least this particular ambiguity.
Another example: what about the fstatat call in gnulib/lib/fchmodat.c? I
suppose we should not add AT_NO_AUTOMOUNT there, under the theory that
GNU/Linux faccessat does not do the equivalent of AT_NO_AUTOMOUNT. Is
that right?
Yes the flag is not mentioned wrt that call,
I've yet to audit kernel code.
cheers,
Pádraig
From 1f5aa31f4787f4c585d663b56606ac6e5b068e4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 7 Mar 2022 17:18:49 +0000
Subject: [PATCH] ln: avoid triggering automounts
* src/ln.c (do_link): Always pass AT_NO_AUTOMOUNT to fstatat().
NEWS: Mention the fix.
---
NEWS | 8 ++++----
src/ln.c | 6 ++++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/NEWS b/NEWS
index 8e95be755..f57ca004b 100644
--- a/NEWS
+++ b/NEWS
@@ -34,10 +34,10 @@ GNU coreutils NEWS -*- outline -*-
and the documentation has been clarified for unusual cases.
[bug introduced in coreutils-7.0]
- ls and stat no longer try to automount files, reverting to the behavior
- before the statx() call was introduced. Only `stat --cached=never`
- will continue to automount files.
- [bug introduced in coreutils-8.32]
+ ln, ls, and stat no longer try to automount files, reverting to the behavior
+ before the statx() and fstatat() calls were introduced.
+ Only `stat --cached=never` will continue to automount files.
+ [bug introduced in ln in coreutils-8.31, and ls and stat in coreutils-8.32]
On macOS, 'mv A B' no longer fails with "Operation not supported"
when A and B are in the same tmpfs file system.
diff --git a/src/ln.c b/src/ln.c
index bb4695853..6b9a1ec76 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -192,7 +192,8 @@ do_link (char const *source, int destdir_fd, char const *dest_base,
diagnostics. */
if ((link_errno || dest_set) && !symbolic_link)
{
- source_status = fstatat (AT_FDCWD, source, &source_stats, nofollow_flag);
+ source_status = fstatat (AT_FDCWD, source, &source_stats,
+ nofollow_flag | AT_NO_AUTOMOUNT);
if (source_status != 0)
{
error (0, errno, _("failed to access %s"), quoteaf (source));
@@ -217,7 +218,8 @@ do_link (char const *source, int destdir_fd, char const *dest_base,
if (force)
{
struct stat dest_stats;
- if (fstatat (destdir_fd, dest_base, &dest_stats, AT_SYMLINK_NOFOLLOW)
+ if (fstatat (destdir_fd, dest_base, &dest_stats,
+ AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)
!= 0)
{
if (errno != ENOENT)
--
2.26.2
