bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] fdutimensat: add an atflag parameter


From: Eric Blake
Subject: Re: [PATCH] fdutimensat: add an atflag parameter
Date: Fri, 17 Sep 2010 08:29:11 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100907 Fedora/3.1.3-1.fc13 Mnenhy/0.8.3 Thunderbird/3.1.3

On 09/16/2010 05:45 PM, Paul Eggert wrote:
  {
    int result = 1;
+  if (atflag&  ~AT_SYMLINK_NOFOLLOW)
+    {
+      errno = EINVAL;
+      return -1;
+    }
Thanks, but unfortunately this doesn't look quite right for GNU tar's purposes.
Tar specifies a nonnegative FD along with a flag equal to AT_SYMLINK_NOFOLLOW.
The idea is that if the underlying system doesn't support futimens
for some reason, tar can still fall back on utimensat, and know
that utimensat won't follow the symlink.

Ah, so your idea is that when fd is valid, but the fallback has to be used, you intentionally pass AT_SYMLINK_NOFOLLOW so that if you lost a race and someone has injected a symlink, such that "file" no longer matches "fd", then we end up changing the time of just the symlink (or more likely dying with ENOSYS, as pretty much all platforms that can support changing the timestamp of symlinks already support changing timestamps of an fd), which is safer than than following the symlink and changing the timestamp of a completely wrong file.

Makes sense to me.

I don't know whether there is any actual system where utimensat works
but futimens doesn't, but I'm pretty sure there is such a problem with
chmod/chown/etc. and I wouldn't be surprised if the problem also existed
with futimens.

If you like, I can install the obvious patch.

Nah, I just did this (ignore atflag if fd is valid, and let utimensat rather than fdutimensat validate atflag if fd was invalid):

diff --git i/ChangeLog w/ChangeLog
index 328f8da..49603e5 100644
--- i/ChangeLog
+++ w/ChangeLog
@@ -1,3 +1,11 @@
+2010-09-17  Eric Blake  <address@hidden>
+
+       fdutimensat: drop atflag validation
+       * lib/fdutimensat.c (fdutimensat): Allow AT_SYMLINK_NOFOLLOW even
+       with valid fd, to close a race scenario where futimens is
+       unsupported and FILE was replaced by a symlink.
+       Suggested by Paul Eggert.
+
 2010-09-16  Eric Blake  <address@hidden>

        fdutimensat: add an atflag parameter
diff --git i/lib/fdutimensat.c w/lib/fdutimensat.c
index 77f893b..0219a78 100644
--- i/lib/fdutimensat.c
+++ w/lib/fdutimensat.c
@@ -35,8 +35,8 @@
    use just futimes (or equivalent) instead of utimes (or equivalent),
    and fail if on an old system without futimes (or equivalent).
    If TIMESPEC is null, set the time stamps to the current time.
-   ATFLAG must be 0 if FD is non-negative; otherwise it may be
-   AT_SYMLINK_NOFOLLOW to operate on FILE as a symlink.
+   ATFLAG is passed to utimensat if FD is negative or futimens was
+   unsupported, which can allow operation on FILE as a symlink.
    Return 0 on success, -1 (setting errno) on failure.  */

 int
@@ -44,20 +44,8 @@ fdutimensat (int dir, char const *file, int fd, struct timespec const ts[2],
              int atflag)
 {
   int result = 1;
-  if (atflag & ~AT_SYMLINK_NOFOLLOW)
-    {
-      errno = EINVAL;
-      return -1;
-    }
   if (0 <= fd)
-    {
-      if (atflag)
-        {
-          errno = EINVAL;
-          return -1;
-        }
-      result = futimens (fd, ts);
-    }
+    result = futimens (fd, ts);
   if (file && (fd < 0 || (result == -1 && errno == ENOSYS)))
     result = utimensat (dir, file, ts, atflag);
   if (result == 1)

--
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org



reply via email to

[Prev in Thread] Current Thread [Next in Thread]