[Top][All Lists]
[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
- Re: [Bug-tar] [PATCH] two patches for --atime-preserve races and other problems, Paul Eggert, 2010/09/16
- Re: [Bug-tar] [PATCH] two patches for --atime-preserve races and other problems, Eric Blake, 2010/09/16
- Re: [Bug-tar] [PATCH] two patches for --atime-preserve races and other problems, Paul Eggert, 2010/09/16
- Re: [PATCH] fdutimensat: add an atflag parameter, Paul Eggert, 2010/09/17
- Re: [PATCH] fdutimensat: add an atflag parameter, Eric Blake, 2010/09/17
- Re: [PATCH] fdutimensat: add an atflag parameter, Eric Blake, 2010/09/17
- Re: [PATCH] fdutimensat: add an atflag parameter, Paul Eggert, 2010/09/17
- Re: [PATCH] fdutimensat: add an atflag parameter, Eric Blake, 2010/09/17
- Re: [PATCH] fdutimensat: add an atflag parameter, Paul Eggert, 2010/09/17
- [PATCH] fdutimens, fdutimensat: update signature, again, Eric Blake, 2010/09/17