unfs3: fix attribute settings, second try

The last change to unfs3 repaired some things for symlinks but in return
broke stuff for regular files.

With this change the previous attempt is replaced by something better
tested now.

Instead of letting unfs open the file to adapt, use fchmodat() et al,
which are racy if the underlaying filesystem changes, but on the nfs
side it is fine.

Fixes: 4e21b490eb28 ("unfs3: fix attribute setting for symlinks")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
[mol: simplify patch]
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>

4 files changed, 118 insertions, 186 deletions

diff --git a/patches/unfs3-0.9.22/0005-attr-handle-symlinks-correctly.patch b/patches/unfs3-0.9.22/0005-attr-handle-symlinks-correctly.patch
new file mode 100644
index 000000000..31428deab
--- /dev/null
+++ b/patches/unfs3-0.9.22/0005-attr-handle-symlinks-correctly.patch
@@ -0,0 +1,116 @@
+From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
+Date: Thu, 27 Feb 2020 14:04:18 +0100
+Subject: [PATCH] attr: handle symlinks correctly
+
+Make sure to change the properties of the symlink and not the symlink
+target. So use utimensat(), fchownat() and fchmodat() with
+AT_SYMLINK_NOFOLLOW to set the relevant attributes.
+
+This change breaks compilation on Windows.
+
+Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
+---
+ attr.c | 57 ++++++++++++++++++++++++++++++---------------------------
+ 1 file changed, 30 insertions(+), 27 deletions(-)
+
+diff --git a/attr.c b/attr.c
+index 2653ed0a16bb..2a13441179a0 100644
+--- a/attr.c
++++ b/attr.c
+@@ -267,38 +267,41 @@ post_op_attr get_post_cached(struct svc_req * req)
+ 
+ /*
+  * setting of time, races with local filesystem
+- *
+- * there is no futimes() function in POSIX or Linux
+  */
+-static nfsstat3 set_time(const char *path, backend_statstruct buf, sattr3 new)
++static nfsstat3 set_time(const char *path, sattr3 new)
+ {
+-    time_t new_atime, new_mtime;
+-    struct utimbuf utim;
+-    int res;
+-
+     /* set atime and mtime */
+     if (new.atime.set_it != DONT_CHANGE || new.mtime.set_it != DONT_CHANGE) {
+ 
++	/* atime in t[0], mtime in t[1] */
++	struct timespec t[2];
++	int res;
++
+ 	/* compute atime to set */
+-	if (new.atime.set_it == SET_TO_SERVER_TIME)
+-	    new_atime = time(NULL);
+-	else if (new.atime.set_it == SET_TO_CLIENT_TIME)
+-	    new_atime = new.atime.set_atime_u.atime.seconds;
+-	else			       /* DONT_CHANGE */
+-	    new_atime = buf.st_atime;
++	if (new.atime.set_it == SET_TO_SERVER_TIME) {
++	    t[0].tv_sec = UTIME_NOW;
++	    t[0].tv_nsec = UTIME_NOW;
++	} else if (new.atime.set_it == SET_TO_CLIENT_TIME) {
++	    t[0].tv_sec = new.atime.set_atime_u.atime.seconds;
++	    t[0].tv_nsec = new.atime.set_atime_u.atime.nseconds;
++	} else {
++	    t[0].tv_sec = UTIME_OMIT;
++	    t[0].tv_nsec = UTIME_OMIT;
++	}
+ 
+ 	/* compute mtime to set */
+-	if (new.mtime.set_it == SET_TO_SERVER_TIME)
+-	    new_mtime = time(NULL);
+-	else if (new.mtime.set_it == SET_TO_CLIENT_TIME)
+-	    new_mtime = new.mtime.set_mtime_u.mtime.seconds;
+-	else			       /* DONT_CHANGE */
+-	    new_mtime = buf.st_mtime;
+-
+-	utim.actime = new_atime;
+-	utim.modtime = new_mtime;
++	if (new.mtime.set_it == SET_TO_SERVER_TIME) {
++	    t[1].tv_sec = UTIME_NOW;
++	    t[1].tv_nsec = UTIME_NOW;
++	} else if (new.mtime.set_it == SET_TO_CLIENT_TIME) {
++	    t[1].tv_sec = new.mtime.set_mtime_u.mtime.seconds;
++	    t[1].tv_nsec = new.mtime.set_mtime_u.mtime.nseconds;
++	} else {
++	    t[1].tv_sec = UTIME_OMIT;
++	    t[1].tv_nsec = UTIME_OMIT;
++	}
+ 
+-	res = backend_utime(path, &utim);
++	res = utimensat(AT_FDCWD, path, t, AT_SYMLINK_NOFOLLOW);
+ 	if (res == -1)
+ 	    return setattr_err();
+     }
+@@ -346,19 +349,19 @@ static nfsstat3 set_attr_unsafe(const char *path, nfs_fh3 nfh, sattr3 new)
+ 	else
+ 	    new_gid = -1;
+ 
+-	res = backend_lchown(path, new_uid, new_gid);
++	res = fchownat(AT_FDCWD, path, new_uid, new_gid, AT_SYMLINK_NOFOLLOW);
+ 	if (res == -1)
+ 	    return setattr_err();
+     }
+ 
+     /* set mode */
+     if (new.mode.set_it == TRUE) {
+-	res = backend_chmod(path, new.mode.set_mode3_u.mode);
++	res = fchmodat(AT_FDCWD, path, new.mode.set_mode3_u.mode, 0);
+ 	if (res == -1)
+ 	    return setattr_err();
+     }
+ 
+-    return set_time(path, buf, new);
++    return set_time(path, new);
+ }
+ 
+ /*
+@@ -461,7 +464,7 @@ nfsstat3 set_attr(const char *path, nfs_fh3 nfh, sattr3 new)
+     }
+ 
+     /* finally, set times */
+-    return set_time(path, buf, new);
++    return set_time(path, new);
+ }
+ 
+ /*
diff --git a/patches/unfs3-0.9.22/0005-attr-use-futimens-instead-of-utime.patch b/patches/unfs3-0.9.22/0005-attr-use-futimens-instead-of-utime.patch
deleted file mode 100644
index 36f69e44b..000000000
--- a/patches/unfs3-0.9.22/0005-attr-use-futimens-instead-of-utime.patch
+++ /dev/null
@@ -1,102 +0,0 @@
-From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
-Date: Tue, 18 Feb 2020 15:20:12 +0100
-Subject: [PATCH] attr: use futimens() instead of utime()
-
-The former has two relevant advantages: It works on file descriptors
-instead of a path name and it has nano second resolution (instead of only
-seconds).
-
-Note, I don't know about Windows which is probably broken by this commit
-as I assume it doesn't support futimens().
----
- attr.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
- 1 file changed, 48 insertions(+), 5 deletions(-)
-
-diff --git a/attr.c b/attr.c
-index 2653ed0a16bb..08c9d24cb446 100644
---- a/attr.c
-+++ b/attr.c
-@@ -265,12 +265,51 @@ post_op_attr get_post_cached(struct svc_req * req)
-     return get_post_buf(st_cache, req);
- }
- 
-+static nfsstat3 set_time(int fd, sattr3 new)
-+{
-+    if (new.atime.set_it != DONT_CHANGE || new.mtime.set_it != DONT_CHANGE) {
-+
-+	/* atime in t[0], mtime in t[1] */
-+	struct timespec t[2];
-+	int res;
-+
-+	/* compute atime to set */
-+	if (new.atime.set_it == SET_TO_SERVER_TIME) {
-+	    t[0].tv_sec = UTIME_NOW;
-+	    t[0].tv_nsec = UTIME_NOW;
-+	} else if (new.atime.set_it == SET_TO_CLIENT_TIME) {
-+	    t[0].tv_sec = new.atime.set_atime_u.atime.seconds;
-+	    t[0].tv_nsec = new.atime.set_atime_u.atime.nseconds;
-+	} else { /* DONT_CHANGE */
-+	    t[0].tv_sec = UTIME_OMIT;
-+	    t[0].tv_nsec = UTIME_OMIT;
-+	}
-+
-+	/* compute mtime to set */
-+	if (new.mtime.set_it == SET_TO_SERVER_TIME) {
-+	    t[1].tv_sec = UTIME_NOW;
-+	    t[1].tv_nsec = UTIME_NOW;
-+	} else if (new.mtime.set_it == SET_TO_CLIENT_TIME) {
-+	    t[1].tv_sec = new.mtime.set_mtime_u.mtime.seconds;
-+	    t[1].tv_nsec = new.mtime.set_mtime_u.mtime.nseconds;
-+	} else { /* DONT_CHANGE */
-+	    t[1].tv_sec = UTIME_OMIT;
-+	    t[1].tv_nsec = UTIME_OMIT;
-+	}
-+
-+	res = futimens(fd, &t);
-+	if (res == -1)
-+	    return setattr_err();
-+    }
-+    return NFS3_OK;
-+}
-+
- /*
-  * setting of time, races with local filesystem
-  *
-  * there is no futimes() function in POSIX or Linux
-  */
--static nfsstat3 set_time(const char *path, backend_statstruct buf, sattr3 new)
-+static nfsstat3 set_time_path(const char *path, backend_statstruct buf, sattr3 new)
- {
-     time_t new_atime, new_mtime;
-     struct utimbuf utim;
-@@ -358,7 +397,7 @@ static nfsstat3 set_attr_unsafe(const char *path, nfs_fh3 nfh, sattr3 new)
- 	    return setattr_err();
-     }
- 
--    return set_time(path, buf, new);
-+    return set_time_path(path, buf, new);
- }
- 
- /*
-@@ -454,14 +493,18 @@ nfsstat3 set_attr(const char *path, nfs_fh3 nfh, sattr3 new)
- 	}
-     }
- 
-+    /* finally, set times */
-+    res = set_time(fd, new);
-+    if (res != NFS3_OK) {
-+	backend_close(fd);
-+	return res;
-+    }
-+
-     res = backend_close(fd);
-     if (res == -1) {
- 	/* error on close probably means attributes didn't make it */
- 	return NFS3ERR_IO;
-     }
--
--    /* finally, set times */
--    return set_time(path, buf, new);
- }
- 
- /*
diff --git a/patches/unfs3-0.9.22/0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch b/patches/unfs3-0.9.22/0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch
deleted file mode 100644
index 7e38c549d..000000000
--- a/patches/unfs3-0.9.22/0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch
+++ /dev/null
@@ -1,81 +0,0 @@
-From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
-Date: Tue, 18 Feb 2020 15:23:52 +0100
-Subject: [PATCH] attr: make use of O_PATH to set attributes of symlinks and
- devices
-
-This fixes setting atime/mtime for symlinks which before this patch changed
-the timestamps for the symlink's target (and resulted in an error if the
-symlink is dangling).
----
- attr.c | 24 ++++++++++++++++++++++--
- 1 file changed, 22 insertions(+), 2 deletions(-)
-
-diff --git a/attr.c b/attr.c
-index 08c9d24cb446..ae2a759e9c86 100644
---- a/attr.c
-+++ b/attr.c
-@@ -304,6 +304,7 @@ static nfsstat3 set_time(int fd, sattr3 new)
-     return NFS3_OK;
- }
- 
-+#ifndef O_PATH
- /*
-  * setting of time, races with local filesystem
-  *
-@@ -399,6 +400,7 @@ static nfsstat3 set_attr_unsafe(const char *path, nfs_fh3 nfh, sattr3 new)
- 
-     return set_time_path(path, buf, new);
- }
-+#endif /* #ifndef O_PATH */
- 
- /*
-  * set attributes of an object
-@@ -410,13 +412,29 @@ nfsstat3 set_attr(const char *path, nfs_fh3 nfh, sattr3 new)
-     uid_t new_uid;
-     gid_t new_gid;
-     backend_statstruct buf;
-+#ifdef O_PATH
-+    int flags = O_PATH | O_NOFOLLOW;
-+#endif
- 
-     res = backend_lstat(path, &buf);
-     if (res != 0)
- 	return NFS3ERR_STALE;
- 
-+#ifdef O_PATH
-+
-+    if (new.size.set_it == TRUE) {
-+	if (!S_ISREG(buf.st_mode))
-+	    return NFS3ERR_NOTSUPP;
-+	flags = O_WRONLY | O_NONBLOCK;
-+    }
-+
-+    fd = backend_open(path, flags);
-+    if (fd < 0)
-+	return NFS3ERR_STALE;
-+
-+#else
-     /* 
--     * don't open(2) device nodes, it could trigger
-+     * don't open(2) device nodes without O_PATH, it could trigger
-      * module loading on the server
-      */
-     if (S_ISBLK(buf.st_mode) || S_ISCHR(buf.st_mode))
-@@ -424,7 +442,7 @@ nfsstat3 set_attr(const char *path, nfs_fh3 nfh, sattr3 new)
- 
- #ifdef S_ISLNK
-     /* 
--     * opening a symlink would open the underlying file,
-+     * opening a symlink without O_PATH would open the underlying file,
-      * don't try to do that
-      */
-     if (S_ISLNK(buf.st_mode))
-@@ -441,6 +459,8 @@ nfsstat3 set_attr(const char *path, nfs_fh3 nfh, sattr3 new)
-     if (fd == -1)
- 	return set_attr_unsafe(path, nfh, new);
- 
-+#endif /* #ifdef O_PATH / else */
-+
-     res = backend_fstat(fd, &buf);
-     if (res == -1) {
- 	backend_close(fd);
diff --git a/patches/unfs3-0.9.22/series b/patches/unfs3-0.9.22/series
index deda41f14..3257be13d 100644
--- a/patches/unfs3-0.9.22/series
+++ b/patches/unfs3-0.9.22/series
@@ -4,6 +4,5 @@
 0002-fix-unfs3-build-with-recent-versions-of-flex.patch
 0003-64-bit-fixes-don-t-assume-that-long-is-the-same-size.patch
 0004-use-libtirpc-if-found.patch
-0005-attr-use-futimens-instead-of-utime.patch
-0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch
-# 0534e0228eabedcabb5e164a284ad3f5  - git-ptx-patches magic
+0005-attr-handle-symlinks-correctly.patch
+# b2dd04f9dcebab8f3f969f165662a798  - git-ptx-patches magic