[ptxdist] [PATCH] unfs3: fix attribute setting for symlinks

[ptxdist] [PATCH] unfs3: fix attribute setting for symlinks

[Uwe Kleine-König]

unfs makes use of utime() which is unsuitable to set last access and
modification times for symlinks. So make use of the O_PATH flag to
open() and use the futimens() function.

As a side effect this is less racy and improves resolution from seconds
to nanoseconds.

Note this will break builds of unfs on Windows. The futimens() function
is specified in POSIX.1-2008, so this shouldn't be a problem for other
platforms.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

note I didn't try to contact upstream.

Best regards
Uwe

 ...5-attr-use-futimens-instead-of-utime.patch | 102 ++++++++++++++++++
 ...-O_PATH-to-set-attributes-of-symlink.patch |  81 ++++++++++++++
 patches/unfs3-0.9.22/series                   |   4 +-
 3 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 patches/unfs3-0.9.22/0005-attr-use-futimens-instead-of-utime.patch
 create mode 100644 patches/unfs3-0.9.22/0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch

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
new file mode 100644
index 000000000000..401142fb1b16
--- /dev/null
+++ b/patches/unfs3-0.9.22/0005-attr-use-futimens-instead-of-utime.patch
@@ -0,0 +1,102 @@
+From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <uwe@kleine-koenig.org>
+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
new file mode 100644
index 000000000000..798927a520e6
--- /dev/null
+++ b/patches/unfs3-0.9.22/0006-attr-make-use-of-O_PATH-to-set-attributes-of-symlink.patch
@@ -0,0 +1,81 @@
+From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <uwe@kleine-koenig.org>
+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 3c8683ef546a..deda41f14aef 100644
--- a/patches/unfs3-0.9.22/series
+++ b/patches/unfs3-0.9.22/series
@@ -4,4 +4,6 @@
 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
-# b3936a9a6ca0c9fd1c164603ca1d2064  - git-ptx-patches magic
+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
-- 
2.25.0


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] unfs3: fix attribute setting for symlinks

[Uwe Kleine-König]

On Tue, Feb 18, 2020 at 03:57:20PM +0100, Uwe Kleine-König wrote:
> unfs makes use of utime() which is unsuitable to set last access and
> modification times for symlinks. So make use of the O_PATH flag to
> open() and use the futimens() function.
> 
> As a side effect this is less racy and improves resolution from seconds
> to nanoseconds.
> 
> Note this will break builds of unfs on Windows. The futimens() function
> is specified in POSIX.1-2008, so this shouldn't be a problem for other
> platforms.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Here is something fishy. After deploying this patch in a local BSP I see
some errors that were not there before.

So please hold up merging until I found the time to look into these
issues please.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] unfs3: fix attribute setting for symlinks

[Michael Olbrich]

On Mon, Feb 24, 2020 at 12:00:51PM +0100, Uwe Kleine-König wrote:
> On Tue, Feb 18, 2020 at 03:57:20PM +0100, Uwe Kleine-König wrote:
> > unfs makes use of utime() which is unsuitable to set last access and
> > modification times for symlinks. So make use of the O_PATH flag to
> > open() and use the futimens() function.
> > 
> > As a side effect this is less racy and improves resolution from seconds
> > to nanoseconds.
> > 
> > Note this will break builds of unfs on Windows. The futimens() function
> > is specified in POSIX.1-2008, so this shouldn't be a problem for other
> > platforms.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Here is something fishy. After deploying this patch in a local BSP I see
> some errors that were not there before.
> 
> So please hold up merging until I found the time to look into these
> issues please.

It's already in master.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de