linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] Allow setting file birth time with utimensat()
@ 2019-02-14 10:00 Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 1/6] fs: add btime to struct iattr Omar Sandoval
                   ` (11 more replies)
  0 siblings, 12 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

Hi,

Since statx was added in 4.11, userspace has had an interface for
reading btime (file creation time), but no way to set it. This RFC patch
series adds support for changing btime with utimensat(). Patch 1 adds
the VFS infrastructure, patch 2 adds the support to utimensat() with a
new flag, and the rest of the patches add filesystem support; I excluded
CIFS for now because I don't have a CIFS setup to test it on.

Updating btime is useful for at least a couple of use cases:

- Backup/restore programs (my motivation for this feature is btrfs send)
- File servers which interoperate with operating systems that allow
  updating file creation time, including Mac OS [1] and Windows [2]

I've also included a man page patch, xfs_io support, and an xfstest.

Thoughts on the implementation or the interface?

Thanks!

1: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/setattrlist.2.html
2: https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-setfiletime

Omar Sandoval (6):
  fs: add btime to struct iattr
  fs: add AT_UTIME_BTIME for utimensat()
  Btrfs: add support for setting btime
  ext4: add support for setting btime
  f2fs: add support for setting btime
  xfs: add support for setting btime

 fs/attr.c                      |  6 +++
 fs/btrfs/inode.c               |  2 +
 fs/btrfs/super.c               |  4 +-
 fs/ext4/inode.c                | 15 +++++-
 fs/ext4/super.c                |  2 +-
 fs/f2fs/file.c                 | 19 ++++++--
 fs/f2fs/super.c                |  2 +-
 fs/utimes.c                    | 86 +++++++++++++++++++++-------------
 fs/xfs/libxfs/xfs_format.h     |  2 +-
 fs/xfs/libxfs/xfs_log_format.h |  2 +-
 fs/xfs/xfs_iops.c              | 11 ++++-
 fs/xfs/xfs_super.c             |  2 +-
 include/linux/fs.h             |  4 ++
 include/uapi/linux/fcntl.h     |  2 +
 14 files changed, 111 insertions(+), 48 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [RFC PATCH 1/6] fs: add btime to struct iattr
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 2/6] fs: add AT_UTIME_BTIME for utimensat() Omar Sandoval
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

Add btime as an attribute that can be updated through notify_change() if
the filesystem supports it, which is indicated by FS_HAS_BTIME in
file_system_type->fs_flags.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/attr.c          | 6 ++++++
 include/linux/fs.h | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/fs/attr.c b/fs/attr.c
index d22e8187477f..4831ef479951 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -233,6 +233,10 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
 
 	WARN_ON_ONCE(!inode_is_locked(inode));
 
+	if ((ia_valid & ATTR_BTIME) &&
+	    !(inode->i_sb->s_type->fs_flags & FS_HAS_BTIME))
+		return -EOPNOTSUPP;
+
 	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
 		if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
 			return -EPERM;
@@ -267,6 +271,8 @@ int notify_change(struct dentry * dentry, struct iattr * attr, struct inode **de
 		attr->ia_atime = now;
 	if (!(ia_valid & ATTR_MTIME_SET))
 		attr->ia_mtime = now;
+	if (!(ia_valid & ATTR_BTIME_SET))
+		attr->ia_btime = now;
 	if (ia_valid & ATTR_KILL_PRIV) {
 		error = security_inode_need_killpriv(dentry);
 		if (error < 0)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 29d8e2cfed0e..d66040f7740b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -191,6 +191,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define ATTR_OPEN	(1 << 15) /* Truncating from open(O_TRUNC) */
 #define ATTR_TIMES_SET	(1 << 16)
 #define ATTR_TOUCH	(1 << 17)
+#define ATTR_BTIME	(1 << 18)
+#define ATTR_BTIME_SET	(1 << 19)
 
 /*
  * Whiteout is represented by a char device.  The following constants define the
@@ -217,6 +219,7 @@ struct iattr {
 	struct timespec64 ia_atime;
 	struct timespec64 ia_mtime;
 	struct timespec64 ia_ctime;
+	struct timespec64 ia_btime;
 
 	/*
 	 * Not an attribute, but an auxiliary info for filesystems wanting to
@@ -2172,6 +2175,7 @@ struct file_system_type {
 #define FS_BINARY_MOUNTDATA	2
 #define FS_HAS_SUBTYPE		4
 #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
+#define FS_HAS_BTIME		16	/* Supports getting/setting btime. */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH 2/6] fs: add AT_UTIME_BTIME for utimensat()
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 1/6] fs: add btime to struct iattr Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 3/6] Btrfs: add support for setting btime Omar Sandoval
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

Now that we have a btime iattr, use it to allow updating btime from
utimensat(). We do so by adding a new AT_UTIME_BTIME flag. Iff this flag
is given, the btime is set to times[2] (unless times is NULL, in which
case the current time is used).

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/utimes.c                | 86 +++++++++++++++++++++++---------------
 include/uapi/linux/fcntl.h |  2 +
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/fs/utimes.c b/fs/utimes.c
index bdcf2daf39c1..cb9fe77e5f91 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -16,7 +16,20 @@ static bool nsec_valid(long nsec)
 	return nsec >= 0 && nsec <= 999999999;
 }
 
-static int utimes_common(const struct path *path, struct timespec64 *times)
+static void init_time_attr(struct iattr *newattrs, struct timespec64 *time_attr,
+			   struct timespec64 time, unsigned int attr,
+			   unsigned int attr_set)
+{
+	if (time.tv_nsec == UTIME_OMIT) {
+		newattrs->ia_valid &= ~attr;
+	} else {
+		*time_attr = time;
+		newattrs->ia_valid |= attr_set;
+	}
+}
+
+static int utimes_common(const struct path *path, struct timespec64 *times,
+			 bool btime)
 {
 	int error;
 	struct iattr newattrs;
@@ -28,25 +41,21 @@ static int utimes_common(const struct path *path, struct timespec64 *times)
 		goto out;
 
 	if (times && times[0].tv_nsec == UTIME_NOW &&
-		     times[1].tv_nsec == UTIME_NOW)
+	    times[1].tv_nsec == UTIME_NOW &&
+	    (!btime || times[2].tv_nsec == UTIME_NOW))
 		times = NULL;
 
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
+	if (btime)
+		newattrs.ia_valid |= ATTR_BTIME;
 	if (times) {
-		if (times[0].tv_nsec == UTIME_OMIT)
-			newattrs.ia_valid &= ~ATTR_ATIME;
-		else if (times[0].tv_nsec != UTIME_NOW) {
-			newattrs.ia_atime.tv_sec = times[0].tv_sec;
-			newattrs.ia_atime.tv_nsec = times[0].tv_nsec;
-			newattrs.ia_valid |= ATTR_ATIME_SET;
-		}
-
-		if (times[1].tv_nsec == UTIME_OMIT)
-			newattrs.ia_valid &= ~ATTR_MTIME;
-		else if (times[1].tv_nsec != UTIME_NOW) {
-			newattrs.ia_mtime.tv_sec = times[1].tv_sec;
-			newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
-			newattrs.ia_valid |= ATTR_MTIME_SET;
+		init_time_attr(&newattrs, &newattrs.ia_atime, times[0],
+			       ATTR_ATIME, ATTR_ATIME_SET);
+		init_time_attr(&newattrs, &newattrs.ia_mtime, times[1],
+			       ATTR_MTIME, ATTR_MTIME_SET);
+		if (btime) {
+			init_time_attr(&newattrs, &newattrs.ia_btime, times[2],
+				       ATTR_BTIME, ATTR_BTIME_SET);
 		}
 		/*
 		 * Tell setattr_prepare(), that this is an explicit time
@@ -90,14 +99,16 @@ static int utimes_common(const struct path *path, struct timespec64 *times)
 long do_utimes(int dfd, const char __user *filename, struct timespec64 *times,
 	       int flags)
 {
+	bool btime = flags & AT_UTIME_BTIME;
 	int error = -EINVAL;
 
 	if (times && (!nsec_valid(times[0].tv_nsec) ||
-		      !nsec_valid(times[1].tv_nsec))) {
+		      !nsec_valid(times[1].tv_nsec) ||
+		      (btime && !nsec_valid(times[2].tv_nsec)))) {
 		goto out;
 	}
 
-	if (flags & ~AT_SYMLINK_NOFOLLOW)
+	if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_UTIME_BTIME))
 		goto out;
 
 	if (filename == NULL && dfd != AT_FDCWD) {
@@ -111,7 +122,7 @@ long do_utimes(int dfd, const char __user *filename, struct timespec64 *times,
 		if (!f.file)
 			goto out;
 
-		error = utimes_common(&f.file->f_path, times);
+		error = utimes_common(&f.file->f_path, times, btime);
 		fdput(f);
 	} else {
 		struct path path;
@@ -124,7 +135,7 @@ long do_utimes(int dfd, const char __user *filename, struct timespec64 *times,
 		if (error)
 			goto out;
 
-		error = utimes_common(&path, times);
+		error = utimes_common(&path, times, btime);
 		path_put(&path);
 		if (retry_estale(error, lookup_flags)) {
 			lookup_flags |= LOOKUP_REVAL;
@@ -139,16 +150,20 @@ long do_utimes(int dfd, const char __user *filename, struct timespec64 *times,
 SYSCALL_DEFINE4(utimensat, int, dfd, const char __user *, filename,
 		struct __kernel_timespec __user *, utimes, int, flags)
 {
-	struct timespec64 tstimes[2];
+	struct timespec64 tstimes[3];
 
 	if (utimes) {
-		if ((get_timespec64(&tstimes[0], &utimes[0]) ||
-			get_timespec64(&tstimes[1], &utimes[1])))
-			return -EFAULT;
-
+		int i, n = (flags & AT_UTIME_BTIME) ? 3 : 2;
+		bool all_omit = true;
+
+		for (i = 0; i < n; i++) {
+			if (get_timespec64(&tstimes[i], &utimes[i]))
+				return -EFAULT;
+			if (tstimes[i].tv_nsec != UTIME_OMIT)
+				all_omit = false;
+		}
 		/* Nothing to do, we must not even check the path.  */
-		if (tstimes[0].tv_nsec == UTIME_OMIT &&
-		    tstimes[1].tv_nsec == UTIME_OMIT)
+		if (all_omit)
 			return 0;
 	}
 
@@ -242,14 +257,19 @@ COMPAT_SYSCALL_DEFINE2(utime, const char __user *, filename,
 
 COMPAT_SYSCALL_DEFINE4(utimensat, unsigned int, dfd, const char __user *, filename, struct old_timespec32 __user *, t, int, flags)
 {
-	struct timespec64 tv[2];
+	struct timespec64 tv[3];
 
 	if  (t) {
-		if (get_old_timespec32(&tv[0], &t[0]) ||
-		    get_old_timespec32(&tv[1], &t[1]))
-			return -EFAULT;
-
-		if (tv[0].tv_nsec == UTIME_OMIT && tv[1].tv_nsec == UTIME_OMIT)
+		int i, n = (flags & AT_UTIME_BTIME) ? 3 : 2;
+		bool all_omit = true;
+
+		for (i = 0; i < n; i++) {
+			if (get_old_timespec32(&tv[i], &t[i]))
+				return -EFAULT;
+			if (tv[i].tv_nsec != UTIME_OMIT)
+				all_omit = false;
+		}
+		if (all_omit)
 			return 0;
 	}
 	return do_utimes(dfd, filename, t ? tv : NULL, flags);
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 6448cdd9a350..fc5b02439697 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -90,5 +90,7 @@
 #define AT_STATX_FORCE_SYNC	0x2000	/* - Force the attributes to be sync'd with the server */
 #define AT_STATX_DONT_SYNC	0x4000	/* - Don't sync attributes with the server */
 
+#define AT_UTIME_BTIME		0x8000	/* Also update file creation time */
+
 
 #endif /* _UAPI_LINUX_FCNTL_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH 3/6] Btrfs: add support for setting btime
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 1/6] fs: add btime to struct iattr Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 2/6] fs: add AT_UTIME_BTIME for utimensat() Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 4/6] ext4: " Omar Sandoval
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

The Btrfs inode format has always included btime (under the name otime),
so setting it is trivial.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/inode.c | 2 ++
 fs/btrfs/super.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c349667c761..49ad777d8057 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5153,6 +5153,8 @@ static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if (attr->ia_valid) {
 		setattr_copy(inode, attr);
+		if (attr->ia_valid & ATTR_BTIME)
+			BTRFS_I(inode)->i_otime = attr->ia_btime;
 		inode_inc_iversion(inode);
 		err = btrfs_dirty_inode(inode);
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 0a3f122dd61f..2af368cad2aa 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2140,7 +2140,7 @@ static struct file_system_type btrfs_fs_type = {
 	.name		= "btrfs",
 	.mount		= btrfs_mount,
 	.kill_sb	= btrfs_kill_super,
-	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
+	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_HAS_BTIME,
 };
 
 static struct file_system_type btrfs_root_fs_type = {
@@ -2148,7 +2148,7 @@ static struct file_system_type btrfs_root_fs_type = {
 	.name		= "btrfs",
 	.mount		= btrfs_mount_root,
 	.kill_sb	= btrfs_kill_super,
-	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
+	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_HAS_BTIME,
 };
 
 MODULE_ALIAS_FS("btrfs");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH 4/6] ext4: add support for setting btime
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (2 preceding siblings ...)
  2019-02-14 10:00 ` [RFC PATCH 3/6] Btrfs: add support for setting btime Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 5/6] f2fs: " Omar Sandoval
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

The ext4 inode format includes btime (under the name crtime) if the
inode is large enough (256 bytes).

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/ext4/inode.c | 15 +++++++++++++--
 fs/ext4/super.c |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 34d7e0703cc6..7f4f83ef539d 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5489,6 +5489,13 @@ static void ext4_wait_for_tail_page_commit(struct inode *inode)
 	}
 }
 
+static bool ext4_inode_has_crtime(struct inode *inode)
+{
+	struct ext4_inode *raw_inode;
+
+	return EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), i_crtime);
+}
+
 /*
  * ext4_setattr()
  *
@@ -5520,6 +5527,9 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
 	int orphan = 0;
 	const unsigned int ia_valid = attr->ia_valid;
 
+	if ((ia_valid & ATTR_BTIME) && !ext4_inode_has_crtime(inode))
+		return -EOPNOTSUPP;
+
 	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
 		return -EIO;
 
@@ -5671,6 +5681,8 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if (!error) {
 		setattr_copy(inode, attr);
+		if (ia_valid & ATTR_BTIME)
+			EXT4_I(inode)->i_crtime = attr->ia_btime;
 		mark_inode_dirty(inode);
 	}
 
@@ -5695,11 +5707,10 @@ int ext4_getattr(const struct path *path, struct kstat *stat,
 		 u32 request_mask, unsigned int query_flags)
 {
 	struct inode *inode = d_inode(path->dentry);
-	struct ext4_inode *raw_inode;
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	unsigned int flags;
 
-	if (EXT4_FITS_IN_INODE(raw_inode, ei, i_crtime)) {
+	if (ext4_inode_has_crtime(inode)) {
 		stat->result_mask |= STATX_BTIME;
 		stat->btime.tv_sec = ei->i_crtime.tv_sec;
 		stat->btime.tv_nsec = ei->i_crtime.tv_nsec;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index fb12d3c17c1b..cdd3975de130 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5976,7 +5976,7 @@ static struct file_system_type ext4_fs_type = {
 	.name		= "ext4",
 	.mount		= ext4_mount,
 	.kill_sb	= kill_block_super,
-	.fs_flags	= FS_REQUIRES_DEV,
+	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_BTIME,
 };
 MODULE_ALIAS_FS("ext4");
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH 5/6] f2fs: add support for setting btime
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (3 preceding siblings ...)
  2019-02-14 10:00 ` [RFC PATCH 4/6] ext4: " Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [RFC PATCH 6/6] xfs: " Omar Sandoval
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

The f2fs inode format includes btime (under the name crtime) if the
feature was enabled at mkfs time.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/f2fs/file.c  | 19 +++++++++++++++----
 fs/f2fs/super.c |  2 +-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index bba56b39dcc5..6064d3e42987 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -690,17 +690,23 @@ int f2fs_truncate(struct inode *inode)
 	return 0;
 }
 
+static bool f2fs_inode_has_crtime(struct inode *inode)
+{
+	struct f2fs_inode *ri;
+
+	return (f2fs_has_extra_attr(inode) &&
+		f2fs_sb_has_inode_crtime(F2FS_I_SB(inode)) &&
+		F2FS_FITS_IN_INODE(ri, F2FS_I(inode)->i_extra_isize, i_crtime));
+}
+
 int f2fs_getattr(const struct path *path, struct kstat *stat,
 		 u32 request_mask, unsigned int query_flags)
 {
 	struct inode *inode = d_inode(path->dentry);
 	struct f2fs_inode_info *fi = F2FS_I(inode);
-	struct f2fs_inode *ri;
 	unsigned int flags;
 
-	if (f2fs_has_extra_attr(inode) &&
-			f2fs_sb_has_inode_crtime(F2FS_I_SB(inode)) &&
-			F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_crtime)) {
+	if (f2fs_inode_has_crtime(inode)) {
 		stat->result_mask |= STATX_BTIME;
 		stat->btime.tv_sec = fi->i_crtime.tv_sec;
 		stat->btime.tv_nsec = fi->i_crtime.tv_nsec;
@@ -770,6 +776,9 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 	int err;
 	bool size_changed = false;
 
+	if ((attr->ia_valid & ATTR_BTIME) && !f2fs_inode_has_crtime(inode))
+		return -EOPNOTSUPP;
+
 	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
 		return -EIO;
 
@@ -848,6 +857,8 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
 	}
 
 	__setattr_copy(inode, attr);
+	if (attr->ia_valid & ATTR_BTIME)
+		F2FS_I(inode)->i_crtime = attr->ia_btime;
 
 	if (attr->ia_valid & ATTR_MODE) {
 		err = posix_acl_chmod(inode, f2fs_get_inode_mode(inode));
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index c46a1d4318d4..e32070c8cb27 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3485,7 +3485,7 @@ static struct file_system_type f2fs_fs_type = {
 	.name		= "f2fs",
 	.mount		= f2fs_mount,
 	.kill_sb	= kill_f2fs_super,
-	.fs_flags	= FS_REQUIRES_DEV,
+	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_BTIME,
 };
 MODULE_ALIAS_FS("f2fs");
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH 6/6] xfs: add support for setting btime
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (4 preceding siblings ...)
  2019-02-14 10:00 ` [RFC PATCH 5/6] f2fs: " Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [PATCH] generic: add a test for AT_UTIME_BTIME Omar Sandoval
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

The XFS inode format includes btime (under the name crtime) in inode
version 3. Also update the comments in libxfs to reflect that di_crtime
can now change.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/xfs/libxfs/xfs_format.h     |  2 +-
 fs/xfs/libxfs/xfs_log_format.h |  2 +-
 fs/xfs/xfs_iops.c              | 11 +++++++++--
 fs/xfs/xfs_super.c             |  2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 9bb3c48843ec..ff6cb860ae77 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -884,9 +884,9 @@ typedef struct xfs_dinode {
 	__be64		di_flags2;	/* more random flags */
 	__be32		di_cowextsize;	/* basic cow extent size for file */
 	__u8		di_pad2[12];	/* more padding for future expansion */
+	xfs_timestamp_t	di_crtime;	/* time created */
 
 	/* fields only written to during inode creation */
-	xfs_timestamp_t	di_crtime;	/* time created */
 	__be64		di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index e5f97c69b320..5cb35b3db870 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -415,9 +415,9 @@ struct xfs_log_dinode {
 	uint64_t	di_flags2;	/* more random flags */
 	uint32_t	di_cowextsize;	/* basic cow extent size for file */
 	uint8_t		di_pad2[12];	/* more padding for future expansion */
+	xfs_ictimestamp_t di_crtime;	/* time created */
 
 	/* fields only written to during inode creation */
-	xfs_ictimestamp_t di_crtime;	/* time created */
 	xfs_ino_t	di_ino;		/* inode number */
 	uuid_t		di_uuid;	/* UUID of the filesystem */
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f48ffd7a8d3e..20570f61ffeb 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -582,6 +582,10 @@ xfs_setattr_time(
 		inode->i_ctime = iattr->ia_ctime;
 	if (iattr->ia_valid & ATTR_MTIME)
 		inode->i_mtime = iattr->ia_mtime;
+	if (iattr->ia_valid & ATTR_BTIME) {
+		ip->i_d.di_crtime.t_sec = iattr->ia_btime.tv_sec;
+		ip->i_d.di_crtime.t_nsec = iattr->ia_btime.tv_nsec;
+	}
 }
 
 static int
@@ -1025,11 +1029,14 @@ xfs_vn_setattr(
 	struct dentry		*dentry,
 	struct iattr		*iattr)
 {
+	struct inode		*inode = d_inode(dentry);
+	struct xfs_inode	*ip = XFS_I(inode);
 	int			error;
 
+	if ((iattr->ia_valid & ATTR_BTIME) && ip->i_d.di_version != 3)
+		return -EOPNOTSUPP;
+
 	if (iattr->ia_valid & ATTR_SIZE) {
-		struct inode		*inode = d_inode(dentry);
-		struct xfs_inode	*ip = XFS_I(inode);
 		uint			iolock;
 
 		xfs_ilock(ip, XFS_MMAPLOCK_EXCL);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index c9097cb0b955..54df86bc634b 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1860,7 +1860,7 @@ static struct file_system_type xfs_fs_type = {
 	.name			= "xfs",
 	.mount			= xfs_fs_mount,
 	.kill_sb		= kill_block_super,
-	.fs_flags		= FS_REQUIRES_DEV,
+	.fs_flags		= FS_REQUIRES_DEV | FS_HAS_BTIME,
 };
 MODULE_ALIAS_FS("xfs");
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] generic: add a test for AT_UTIME_BTIME
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (5 preceding siblings ...)
  2019-02-14 10:00 ` [RFC PATCH 6/6] xfs: " Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [PATCH] utimensat2: document AT_UTIME_BTIME Omar Sandoval
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

Test that on filesystems supporting btime, the timestamp is updated as
expected.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 common/rc             | 14 ++++++++++
 tests/generic/526     | 64 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/526.out | 14 ++++++++++
 tests/generic/group   |  1 +
 4 files changed, 93 insertions(+)
 create mode 100755 tests/generic/526
 create mode 100644 tests/generic/526.out

diff --git a/common/rc b/common/rc
index b8ed1776..d2f43a21 100644
--- a/common/rc
+++ b/common/rc
@@ -3843,6 +3843,20 @@ _require_btime()
 	rm -f $TEST_DIR/test_creation_time
 }
 
+_require_utime_btime()
+{
+	_require_btime
+	_require_xfs_io_command utimes
+	testio=`$XFS_IO_PROG -f -c "utimes 0 0 0 0 0 0" "$TEST_DIR/$$.xfs_io" 2>&1`
+	echo $testio | grep -q "expected 4 arguments" && \
+		_notrun "xfs_io utimes btime support is missing"
+	echo $testio | grep -q "Operation not supported" && \
+		_notrun "updating inode creation time not supported by this filesystem"
+	echo $testio | grep -q "Invalid" && \
+		_notrun "updating inode creation time not supported by this kernel"
+	rm -f $testfile > /dev/null 2>&1
+}
+
 _require_scratch_btime()
 {
 	_require_scratch
diff --git a/tests/generic/526 b/tests/generic/526
new file mode 100755
index 00000000..c2e47a27
--- /dev/null
+++ b/tests/generic/526
@@ -0,0 +1,64 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2019 Omar Sandoval.  All Rights Reserved.
+#
+# FS QA Test 526
+#
+# Test basic AT_UTIME_BTIME functionality.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	rm -f "$TEST_DIR/$$"
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+_supported_fs generic
+_supported_os Linux
+_require_test
+_require_utime_btime
+
+SEC=683153828
+UTIME_NOW=$(((1 << 30) - 1))
+UTIME_OMIT=$(((1 << 30) - 2))
+
+touch "$TEST_DIR/$$"
+
+echo "Set btime"
+$XFS_IO_PROG -c "utimes 0 $UTIME_OMIT 0 $UTIME_OMIT $SEC 0" "$TEST_DIR/$$"
+$XFS_IO_PROG -c 'statx -r' "$TEST_DIR/$$" | grep btime
+
+echo "Set atime and mtime"
+$XFS_IO_PROG -c "utimes 0 $UTIME_NOW 0 $UTIME_NOW" "$TEST_DIR/$$"
+$XFS_IO_PROG -c 'statx -r' "$TEST_DIR/$$" | grep btime
+
+echo "Omit btime"
+$XFS_IO_PROG -c "utimes 0 $UTIME_NOW 0 $UTIME_NOW 0 $UTIME_OMIT" "$TEST_DIR/$$"
+$XFS_IO_PROG -c 'statx -r' "$TEST_DIR/$$" | grep btime
+
+echo "Cycle mount"
+_test_cycle_mount
+$XFS_IO_PROG -c 'statx -r' "$TEST_DIR/$$" | grep btime
+
+echo "Set btime to now"
+$XFS_IO_PROG -c "utimes 0 $UTIME_OMIT 0 $UTIME_OMIT 0 $UTIME_NOW" "$TEST_DIR/$$"
+# The timestamp changed, so this shouldn't output anything.
+$XFS_IO_PROG -c 'statx -r' "$TEST_DIR/$$" | grep -w "$SEC"
+
+status=0
+exit
diff --git a/tests/generic/526.out b/tests/generic/526.out
new file mode 100644
index 00000000..f9ec18e9
--- /dev/null
+++ b/tests/generic/526.out
@@ -0,0 +1,14 @@
+QA output created by 526
+Set btime
+stat.btime.tv_sec = 683153828
+stat.btime.tv_nsec = 0
+Set atime and mtime
+stat.btime.tv_sec = 683153828
+stat.btime.tv_nsec = 0
+Omit btime
+stat.btime.tv_sec = 683153828
+stat.btime.tv_nsec = 0
+Cycle mount
+stat.btime.tv_sec = 683153828
+stat.btime.tv_nsec = 0
+Set btime to now
diff --git a/tests/generic/group b/tests/generic/group
index cfd003d3..2e0203fc 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -528,3 +528,4 @@
 523 auto quick attr
 524 auto quick
 525 auto quick rw
+526 auto quick
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] utimensat2: document AT_UTIME_BTIME
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (6 preceding siblings ...)
  2019-02-14 10:00 ` [PATCH] generic: add a test for AT_UTIME_BTIME Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 10:00 ` [PATCH] xfs_io: add AT_UTIME_BTIME support Omar Sandoval
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 man2/utimensat.2 | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/man2/utimensat.2 b/man2/utimensat.2
index d61b43e96..3b7c62181 100644
--- a/man2/utimensat.2
+++ b/man2/utimensat.2
@@ -211,7 +211,7 @@ is ignored.
 .PP
 The
 .I flags
-field is a bit mask that may be 0, or include the following constant,
+field is a bit mask that may be 0, or include the following constants,
 defined in
 .IR <fcntl.h> :
 .TP
@@ -220,6 +220,36 @@ If
 .I pathname
 specifies a symbolic link, then update the timestamps of the link,
 rather than the file to which it refers.
+.TP
+.B AT_UTIME_BTIME
+Also update the file's "creation time" (\fIbtime\fP). If
+.I times
+is not NULL, then its length must be at least 3, and
+.IR times [2]
+specifies the new creation time. The
+.I tv_nsec
+field may be
+.BR UTIME_NOW
+or
+.BR UTIME_OMIT ;
+this has the same meaning as for the first two timestamps.
+If
+.I times
+is NULL, then the creation time is set to the current time.
+.IP
+Not all filesystems store the file creation time; if a filesystem does not, an
+error is returned. At least the following filesystems store the creation time:
+.RS
+.IP * 3
+Btrfs
+.IP *
+ext4, if the filesystem was created with an inode size of at least 256
+.IP *
+F2FS, if the filesystem was created with the inode_crtime feature
+.IP *
+XFS, if the filesystem was created with the v5 format (i.e., with CRCs enabled)
+.HP
+Support for this flag was added in Linux 5.2.
 .SH RETURN VALUE
 On success,
 .BR utimensat ()
@@ -359,6 +389,12 @@ or, one of the prefix components of
 .I pathname
 is not a directory.
 .TP
+.B EOPNOTSUPP
+.I flags
+contains
+.BR AT_UTIME_BTIME
+but the filesystem does not store the file creation time.
+.TP
 .B EPERM
 The caller attempted to change one or both timestamps to a value
 other than the current time,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH] xfs_io: add AT_UTIME_BTIME support
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (7 preceding siblings ...)
  2019-02-14 10:00 ` [PATCH] utimensat2: document AT_UTIME_BTIME Omar Sandoval
@ 2019-02-14 10:00 ` Omar Sandoval
  2019-02-14 22:06 ` [RFC PATCH 0/6] Allow setting file birth time with utimensat() Dave Chinner
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 10:00 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

From: Omar Sandoval <osandov@fb.com>

In order to test updating btime, make the utimes command optionally take
the btime timestamp.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 io/utimes.c       | 41 +++++++++++++++++++++++++++++++----------
 man/man8/xfs_io.8 |  5 +++--
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/io/utimes.c b/io/utimes.c
index 40117472..72ce68f3 100644
--- a/io/utimes.c
+++ b/io/utimes.c
@@ -4,11 +4,18 @@
  * All Rights Reserved.
  */
 
+#include <fcntl.h>
+#include <sys/syscall.h>
+
 #include "command.h"
 #include "input.h"
 #include "init.h"
 #include "io.h"
 
+#ifndef AT_UTIME_BTIME
+#define AT_UTIME_BTIME 0x8000
+#endif
+
 static cmdinfo_t utimes_cmd;
 
 static void
@@ -16,9 +23,10 @@ utimes_help(void)
 {
 	printf(_(
 "\n"
-" Update file atime and mtime of the current file with nansecond precision.\n"
+" Update file atime, mtime, and optionally btime of the current file with\n"
+" nansecond precision.\n"
 "\n"
-" Usage: utimes atime_sec atime_nsec mtime_sec mtime_nsec.\n"
+" Usage: utimes atime_sec atime_nsec mtime_sec mtime_nsec [btime_sec btime_nsec]\n"
 " *_sec: Seconds elapsed since 1970-01-01 00:00:00 UTC.\n"
 " *_nsec: Nanoseconds since the corresponding *_sec.\n"
 "\n"));
@@ -29,9 +37,13 @@ utimes_f(
 	int		argc,
 	char		**argv)
 {
-	struct timespec t[2];
+	struct timespec t[3];
+	int flags = 0;
 	int result;
 
+	if (argc == 6)
+		return command_usage(&utimes_cmd);
+
 	/* Get the timestamps */
 	result = timespec_from_string(argv[1], argv[2], &t[0]);
 	if (result) {
@@ -43,13 +55,22 @@ utimes_f(
 		fprintf(stderr, "Bad value for mtime\n");
 		return 0;
 	}
-
-	/* Call futimens to update time. */
-	if (futimens(file->fd, t)) {
-		perror("futimens");
-		return 0;
+	if (argc == 7) {
+		result = timespec_from_string(argv[5], argv[6], &t[2]);
+		if (result) {
+			fprintf(stderr, "Bad value for btime\n");
+			return 0;
+		}
+		flags |= AT_UTIME_BTIME;
 	}
 
+	/*
+	 * Use syscall() because the glibc wrapper for utimensat() disallows a
+	 * NULL pathname.
+	 */
+	if (syscall(SYS_utimensat, file->fd, NULL, t, flags))
+		perror("utimensat");
+
 	return 0;
 }
 
@@ -59,9 +80,9 @@ utimes_init(void)
 	utimes_cmd.name = "utimes";
 	utimes_cmd.cfunc = utimes_f;
 	utimes_cmd.argmin = 4;
-	utimes_cmd.argmax = 4;
+	utimes_cmd.argmax = 6;
 	utimes_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	utimes_cmd.args = _("atime_sec atime_nsec mtime_sec mtime_nsec");
+	utimes_cmd.args = _("atime_sec atime_nsec mtime_sec mtime_nsec [btime_sec btime_nsec]");
 	utimes_cmd.oneline = _("Update file times of the current file");
 	utimes_cmd.help = utimes_help;
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index fbf50df5..dbf8098b 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -827,8 +827,9 @@ verbose output will be printed.
 .RE
 .PD
 .TP
-.BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec"
-The utimes command changes the atime and mtime of the current file.
+.BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec [btime_sec btime_nsec]"
+The utimes command changes the atime, mtime, and optionally btime of the
+current file.
 sec uses UNIX timestamp notation and is the seconds elapsed since
 1970-01-01 00:00:00 UTC.
 nsec is the nanoseconds since the sec. This value needs to be in
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (8 preceding siblings ...)
  2019-02-14 10:00 ` [PATCH] xfs_io: add AT_UTIME_BTIME support Omar Sandoval
@ 2019-02-14 22:06 ` Dave Chinner
  2019-02-14 23:14   ` Omar Sandoval
  2019-02-17 16:35   ` Boaz Harrosh
  2019-02-15  1:57 ` Hans van Kranenburg
  2019-02-22 15:02 ` David Sterba
  11 siblings, 2 replies; 29+ messages in thread
From: Dave Chinner @ 2019-02-14 22:06 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Hi,
> 
> Since statx was added in 4.11, userspace has had an interface for
> reading btime (file creation time), but no way to set it. This RFC patch
> series adds support for changing btime with utimensat(). Patch 1 adds
> the VFS infrastructure, patch 2 adds the support to utimensat() with a
> new flag, and the rest of the patches add filesystem support; I excluded
> CIFS for now because I don't have a CIFS setup to test it on.
> 
> Updating btime is useful for at least a couple of use cases:
> 
> - Backup/restore programs (my motivation for this feature is btrfs send)
> - File servers which interoperate with operating systems that allow
>   updating file creation time, including Mac OS [1] and Windows [2]

So you're adding an interface that allows users to change the create
time of files without needing any privileges?

Inode create time is forensic metadata in XFS  - information we use
for sequence of event and inode lifetime analysis during examination
of broken filesystem images and systems that have been broken into.
Just because it's exposed to userspace via statx(), it doesn't mean
that it is information that users should be allowed to change. i.e.
allowing users to be able to change the create time on files makes
it completely useless for the purpose it was added to XFS for...

And allowing root to change the create time doesn't really help,
because once you've broken into a system, this makes it really easy
to cover tracks (e.g. we can't find files that were created and
unlinked during the break in window anymore) and lay false
trails....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 22:06 ` [RFC PATCH 0/6] Allow setting file birth time with utimensat() Dave Chinner
@ 2019-02-14 23:14   ` Omar Sandoval
  2019-02-15  0:16     ` Dave Chinner
  2019-02-17 16:35   ` Boaz Harrosh
  1 sibling, 1 reply; 29+ messages in thread
From: Omar Sandoval @ 2019-02-14 23:14 UTC (permalink / raw)
  To: Dave Chinner
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On Fri, Feb 15, 2019 at 09:06:26AM +1100, Dave Chinner wrote:
> On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Hi,
> > 
> > Since statx was added in 4.11, userspace has had an interface for
> > reading btime (file creation time), but no way to set it. This RFC patch
> > series adds support for changing btime with utimensat(). Patch 1 adds
> > the VFS infrastructure, patch 2 adds the support to utimensat() with a
> > new flag, and the rest of the patches add filesystem support; I excluded
> > CIFS for now because I don't have a CIFS setup to test it on.
> > 
> > Updating btime is useful for at least a couple of use cases:
> > 
> > - Backup/restore programs (my motivation for this feature is btrfs send)
> > - File servers which interoperate with operating systems that allow
> >   updating file creation time, including Mac OS [1] and Windows [2]
> 
> So you're adding an interface that allows users to change the create
> time of files without needing any privileges?

I think it'd be reasonable to make this a privileged operation. I didn't
for this initial submission for a couple of reasons:

1. The precedent on Mac OS and Windows is that this isn't a privileged
   operation.
2. I knew there would be different opinions on this either way I went.

> Inode create time is forensic metadata in XFS  - information we use
> for sequence of event and inode lifetime analysis during examination
> of broken filesystem images and systems that have been broken into.
> Just because it's exposed to userspace via statx(), it doesn't mean
> that it is information that users should be allowed to change. i.e.
> allowing users to be able to change the create time on files makes
> it completely useless for the purpose it was added to XFS for...
> 
> And allowing root to change the create time doesn't really help,
> because once you've broken into a system, this makes it really easy
> to cover tracks

If the threat model is that the attacker has root, then they can
overwrite the timestamp on disk anyways, no?

> (e.g. we can't find files that were created and
> unlinked during the break in window anymore) and lay false
> trails....

Fair point, although there's still ctime during the break-in window,
which I assume you'd be looking for anyways since files modified during
the break-in window are also of interest.

I see a few options, none of which are particularly nice:

1. Filesystems like XFS could choose not to support setting btime even
   if they support reading it.
2. XFS could add a second, writeable btime which is used for
   statx/utimes when available (it would fit in di_pad2...).
3. We could add a btime_writable sysctl/mount option/mkfs option.

Thanks!

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 23:14   ` Omar Sandoval
@ 2019-02-15  0:16     ` Dave Chinner
  2019-02-15  6:59       ` Omar Sandoval
  0 siblings, 1 reply; 29+ messages in thread
From: Dave Chinner @ 2019-02-15  0:16 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On Thu, Feb 14, 2019 at 03:14:29PM -0800, Omar Sandoval wrote:
> On Fri, Feb 15, 2019 at 09:06:26AM +1100, Dave Chinner wrote:
> > On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > Hi,
> > > 
> > > Since statx was added in 4.11, userspace has had an interface for
> > > reading btime (file creation time), but no way to set it. This RFC patch
> > > series adds support for changing btime with utimensat(). Patch 1 adds
> > > the VFS infrastructure, patch 2 adds the support to utimensat() with a
> > > new flag, and the rest of the patches add filesystem support; I excluded
> > > CIFS for now because I don't have a CIFS setup to test it on.
> > > 
> > > Updating btime is useful for at least a couple of use cases:
> > > 
> > > - Backup/restore programs (my motivation for this feature is btrfs send)
> > > - File servers which interoperate with operating systems that allow
> > >   updating file creation time, including Mac OS [1] and Windows [2]
> > 
> > So you're adding an interface that allows users to change the create
> > time of files without needing any privileges?
> 
> I think it'd be reasonable to make this a privileged operation. I didn't
> for this initial submission for a couple of reasons:
> 
> 1. The precedent on Mac OS and Windows is that this isn't a privileged
>    operation.

Don't really care about them. Interop file servers that support these
operations on other OSs will need to be storing this info in xattrs
because they have to work on filesystems that don't support btime.

> 2. I knew there would be different opinions on this either way I went.

Yup.

> > Inode create time is forensic metadata in XFS  - information we use
> > for sequence of event and inode lifetime analysis during examination
> > of broken filesystem images and systems that have been broken into.
> > Just because it's exposed to userspace via statx(), it doesn't mean
> > that it is information that users should be allowed to change. i.e.
> > allowing users to be able to change the create time on files makes
> > it completely useless for the purpose it was added to XFS for...
> > 
> > And allowing root to change the create time doesn't really help,
> > because once you've broken into a system, this makes it really easy
> > to cover tracks
> 
> If the threat model is that the attacker has root, then they can
> overwrite the timestamp on disk anyways, no?

Modifying the block devicee under an active filesystem is fraught
with danger, and there's no guarantee it will work if the metadata
being modified is still active in memory. Corrupting the filesystem
is a sure way to get noticed....

> > (e.g. we can't find files that were created and
> > unlinked during the break in window anymore) and lay false
> > trails....
> 
> Fair point, although there's still ctime during the break-in window,

Unless you're smart enough to know how to trigger S_NOCMTIME or
FMODE_NOCMTIME....

> which I assume you'd be looking for anyways since files modified during
> the break-in window are also of interest.

... and then that also can't be guaranteed. :/

> I see a few options, none of which are particularly nice:
> 
> 1. Filesystems like XFS could choose not to support setting btime even
>    if they support reading it.
> 2. XFS could add a second, writeable btime which is used for
>    statx/utimes when available (it would fit in di_pad2...).
> 3. We could add a btime_writable sysctl/mount option/mkfs option.

4. create time remains a read-only field, and btrfs grows its own
special interface to twiddle it in btrfs-recv if it really is
necessary.

I'm still not convinced that even backup/restore should be doing this,
because there's so much other metadata that is unique even on
restored files that it doesn't really make any sense to me to lie
about it being created in the past....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (9 preceding siblings ...)
  2019-02-14 22:06 ` [RFC PATCH 0/6] Allow setting file birth time with utimensat() Dave Chinner
@ 2019-02-15  1:57 ` Hans van Kranenburg
  2019-02-15  5:39   ` Omar Sandoval
  2019-02-22 15:02 ` David Sterba
  11 siblings, 1 reply; 29+ messages in thread
From: Hans van Kranenburg @ 2019-02-15  1:57 UTC (permalink / raw)
  To: Omar Sandoval, linux-fsdevel, Al Viro
  Cc: kernel-team, linux-api, linux-btrfs, linux-ext4,
	linux-f2fs-devel, linux-xfs

Hi,

On 2/14/19 11:00 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Since statx was added in 4.11, userspace has had an interface for
> reading btime (file creation time), but no way to set it. This RFC patch
> series adds support for changing btime with utimensat(). Patch 1 adds
> the VFS infrastructure, patch 2 adds the support to utimensat() with a
> new flag, and the rest of the patches add filesystem support; I excluded
> CIFS for now because I don't have a CIFS setup to test it on.
> 
> Updating btime is useful for at least a couple of use cases:
> 
> - Backup/restore programs (my motivation for this feature is btrfs send)

Can you give an example of such usefulness? What's the thing you run
into that you can't do without having this?

You guys are having a technical discussion about 'implementation or the
interface' in this thread, while I'm wondering what I'm missing as btrfs
send/receive user by not having this. I never needed it in my use cases.

There's two levels of use case hidden in the above line. So, I don't
mean why btrfs send/receive is useful (it is, for quick efficient
replication of changes) but, what's an important usecase for btime on
top of that?

> - File servers which interoperate with operating systems that allow
>   updating file creation time, including Mac OS [1] and Windows [2]
> 
> I've also included a man page patch, xfs_io support, and an xfstest.
> 
> Thoughts on the implementation or the interface?
> 
> Thanks!
> 
> 1: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/setattrlist.2.html
> 2: https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-setfiletime
> 
> Omar Sandoval (6):
>   fs: add btime to struct iattr
>   fs: add AT_UTIME_BTIME for utimensat()
>   Btrfs: add support for setting btime
>   ext4: add support for setting btime
>   f2fs: add support for setting btime
>   xfs: add support for setting btime
> 
>  fs/attr.c                      |  6 +++
>  fs/btrfs/inode.c               |  2 +
>  fs/btrfs/super.c               |  4 +-
>  fs/ext4/inode.c                | 15 +++++-
>  fs/ext4/super.c                |  2 +-
>  fs/f2fs/file.c                 | 19 ++++++--
>  fs/f2fs/super.c                |  2 +-
>  fs/utimes.c                    | 86 +++++++++++++++++++++-------------
>  fs/xfs/libxfs/xfs_format.h     |  2 +-
>  fs/xfs/libxfs/xfs_log_format.h |  2 +-
>  fs/xfs/xfs_iops.c              | 11 ++++-
>  fs/xfs/xfs_super.c             |  2 +-
>  include/linux/fs.h             |  4 ++
>  include/uapi/linux/fcntl.h     |  2 +
>  14 files changed, 111 insertions(+), 48 deletions(-)
> 


-- 
Hans van Kranenburg

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-15  1:57 ` Hans van Kranenburg
@ 2019-02-15  5:39   ` Omar Sandoval
  2019-02-15 18:25     ` Hans van Kranenburg
  0 siblings, 1 reply; 29+ messages in thread
From: Omar Sandoval @ 2019-02-15  5:39 UTC (permalink / raw)
  To: Hans van Kranenburg
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On Fri, Feb 15, 2019 at 01:57:39AM +0000, Hans van Kranenburg wrote:
> Hi,
> 
> On 2/14/19 11:00 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Since statx was added in 4.11, userspace has had an interface for
> > reading btime (file creation time), but no way to set it. This RFC patch
> > series adds support for changing btime with utimensat(). Patch 1 adds
> > the VFS infrastructure, patch 2 adds the support to utimensat() with a
> > new flag, and the rest of the patches add filesystem support; I excluded
> > CIFS for now because I don't have a CIFS setup to test it on.
> > 
> > Updating btime is useful for at least a couple of use cases:
> > 
> > - Backup/restore programs (my motivation for this feature is btrfs send)
> 
> Can you give an example of such usefulness? What's the thing you run
> into that you can't do without having this?

That boils down to what's useful about having the file creation time,
and it's really just another tidbit of information which you may or may
not care about. Maybe you have a document that you've been editing for
awhile, and you want to know when you started working on it. Or, you
want to know when a user created some directory that they keep adding
files to.

If the file creation time is useful to you, then you likely want it
preserved if you have to restore from backups. If I had to restore from
backups yesterday and I'm trying to figure out when I started that
document, I don't care that I restored that file yesterday, I want the
real creation date.

If you've never wondered when a file was created, then I'm sure you
won't care whether btrfs send/receive preserves it :)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-15  0:16     ` Dave Chinner
@ 2019-02-15  6:59       ` Omar Sandoval
  2019-02-15 13:57         ` David Disseldorp
  2019-02-17  1:57         ` Andreas Dilger
  0 siblings, 2 replies; 29+ messages in thread
From: Omar Sandoval @ 2019-02-15  6:59 UTC (permalink / raw)
  To: Dave Chinner
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs, Theodore Ts'o,
	Jaegeuk Kim, Steve French

On Fri, Feb 15, 2019 at 11:16:57AM +1100, Dave Chinner wrote:
> On Thu, Feb 14, 2019 at 03:14:29PM -0800, Omar Sandoval wrote:
> > On Fri, Feb 15, 2019 at 09:06:26AM +1100, Dave Chinner wrote:
> > > On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
> > > > From: Omar Sandoval <osandov@fb.com>
> > > > 
> > > > Hi,
> > > > 
> > > > Since statx was added in 4.11, userspace has had an interface for
> > > > reading btime (file creation time), but no way to set it. This RFC patch
> > > > series adds support for changing btime with utimensat(). Patch 1 adds
> > > > the VFS infrastructure, patch 2 adds the support to utimensat() with a
> > > > new flag, and the rest of the patches add filesystem support; I excluded
> > > > CIFS for now because I don't have a CIFS setup to test it on.
> > > > 
> > > > Updating btime is useful for at least a couple of use cases:
> > > > 
> > > > - Backup/restore programs (my motivation for this feature is btrfs send)
> > > > - File servers which interoperate with operating systems that allow
> > > >   updating file creation time, including Mac OS [1] and Windows [2]
> > > 
> > > So you're adding an interface that allows users to change the create
> > > time of files without needing any privileges?
> > 
> > I think it'd be reasonable to make this a privileged operation. I didn't
> > for this initial submission for a couple of reasons:
> > 
> > 1. The precedent on Mac OS and Windows is that this isn't a privileged
> >    operation.
> 
> Don't really care about them. Interop file servers that support these
> operations on other OSs will need to be storing this info in xattrs
> because they have to work on filesystems that don't support btime.
> 
> > 2. I knew there would be different opinions on this either way I went.
> 
> Yup.
> 
> > > Inode create time is forensic metadata in XFS  - information we use
> > > for sequence of event and inode lifetime analysis during examination
> > > of broken filesystem images and systems that have been broken into.
> > > Just because it's exposed to userspace via statx(), it doesn't mean
> > > that it is information that users should be allowed to change. i.e.
> > > allowing users to be able to change the create time on files makes
> > > it completely useless for the purpose it was added to XFS for...
> > > 
> > > And allowing root to change the create time doesn't really help,
> > > because once you've broken into a system, this makes it really easy
> > > to cover tracks
> > 
> > If the threat model is that the attacker has root, then they can
> > overwrite the timestamp on disk anyways, no?
> 
> Modifying the block devicee under an active filesystem is fraught
> with danger, and there's no guarantee it will work if the metadata
> being modified is still active in memory. Corrupting the filesystem
> is a sure way to get noticed....
> 
> > > (e.g. we can't find files that were created and
> > > unlinked during the break in window anymore) and lay false
> > > trails....
> > 
> > Fair point, although there's still ctime during the break-in window,
> 
> Unless you're smart enough to know how to trigger S_NOCMTIME or
> FMODE_NOCMTIME....
> 
> > which I assume you'd be looking for anyways since files modified during
> > the break-in window are also of interest.
> 
> ... and then that also can't be guaranteed. :/
> 
> > I see a few options, none of which are particularly nice:
> > 
> > 1. Filesystems like XFS could choose not to support setting btime even
> >    if they support reading it.
> > 2. XFS could add a second, writeable btime which is used for
> >    statx/utimes when available (it would fit in di_pad2...).
> > 3. We could add a btime_writable sysctl/mount option/mkfs option.
> 
> 4. create time remains a read-only field, and btrfs grows its own
> special interface to twiddle it in btrfs-recv if it really is
> necessary.

I'm curious to hear what the ext4/f2fs/CIFS developers think. If no one
else wants btime to be mutable, then I might as well make it
Btrfs-specific. That is, assuming we reach consensus on the Btrfs side
that btrfs receive should set btime.

> I'm still not convinced that even backup/restore should be doing this,
> because there's so much other metadata that is unique even on
> restored files that it doesn't really make any sense to me to lie
> about it being created in the past....

I suppose it depends on how you interpret btime: if it's strictly
filesystem metadata, then it makes sense that it should be immutable; if
it's metadata for the user's own purposes, then we should allow setting
it.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-15  6:59       ` Omar Sandoval
@ 2019-02-15 13:57         ` David Disseldorp
  2019-02-17  1:57         ` Andreas Dilger
  1 sibling, 0 replies; 29+ messages in thread
From: David Disseldorp @ 2019-02-15 13:57 UTC (permalink / raw)
  To: Omar Sandoval, Samba Technical
  Cc: Dave Chinner, linux-fsdevel, Al Viro, kernel-team, linux-api,
	linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Steve French

On Thu, 14 Feb 2019 22:59:47 -0800, Omar Sandoval wrote:

> On Fri, Feb 15, 2019 at 11:16:57AM +1100, Dave Chinner wrote:
> > On Thu, Feb 14, 2019 at 03:14:29PM -0800, Omar Sandoval wrote:  
> > > On Fri, Feb 15, 2019 at 09:06:26AM +1100, Dave Chinner wrote:  
...
> > > > Inode create time is forensic metadata in XFS  - information we use
> > > > for sequence of event and inode lifetime analysis during examination
> > > > of broken filesystem images and systems that have been broken into.
> > > > Just because it's exposed to userspace via statx(), it doesn't mean
> > > > that it is information that users should be allowed to change. i.e.
> > > > allowing users to be able to change the create time on files makes
> > > > it completely useless for the purpose it was added to XFS for...
> > > > 
> > > > And allowing root to change the create time doesn't really help,
> > > > because once you've broken into a system, this makes it really easy
> > > > to cover tracks  
> > > 
> > > If the threat model is that the attacker has root, then they can
> > > overwrite the timestamp on disk anyways, no?  
> > 
> > Modifying the block devicee under an active filesystem is fraught
> > with danger, and there's no guarantee it will work if the metadata
> > being modified is still active in memory. Corrupting the filesystem
> > is a sure way to get noticed....
> >   
> > > > (e.g. we can't find files that were created and
> > > > unlinked during the break in window anymore) and lay false
> > > > trails....  
> > > 
> > > Fair point, although there's still ctime during the break-in window,  
> > 
> > Unless you're smart enough to know how to trigger S_NOCMTIME or
> > FMODE_NOCMTIME....
> >   
> > > which I assume you'd be looking for anyways since files modified during
> > > the break-in window are also of interest.  

I'm not sure I follow the forensics use-case for immutable btime. I'd
expect dm-verity or selinux/apparmor audits to do a better job for those
worried about this kind of attack.

> > ... and then that also can't be guaranteed. :/
> >   
> > > I see a few options, none of which are particularly nice:
> > > 
> > > 1. Filesystems like XFS could choose not to support setting btime even
> > >    if they support reading it.
> > > 2. XFS could add a second, writeable btime which is used for
> > >    statx/utimes when available (it would fit in di_pad2...).
> > > 3. We could add a btime_writable sysctl/mount option/mkfs option.  
> > 
> > 4. create time remains a read-only field, and btrfs grows its own
> > special interface to twiddle it in btrfs-recv if it really is
> > necessary.  
> 
> I'm curious to hear what the ext4/f2fs/CIFS developers think. If no one
> else wants btime to be mutable, then I might as well make it
> Btrfs-specific. That is, assuming we reach consensus on the Btrfs side
> that btrfs receive should set btime.

Samba currently uses a user.DOSATTRIB xattr for tracking creation time.
IMO a mutable btime accessible via statx would be useful for
cross-protocol interoperability.

Cheers, David

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-15  5:39   ` Omar Sandoval
@ 2019-02-15 18:25     ` Hans van Kranenburg
  0 siblings, 0 replies; 29+ messages in thread
From: Hans van Kranenburg @ 2019-02-15 18:25 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On 2/15/19 6:39 AM, Omar Sandoval wrote:
> On Fri, Feb 15, 2019 at 01:57:39AM +0000, Hans van Kranenburg wrote:
>> Hi,
>>
>> On 2/14/19 11:00 AM, Omar Sandoval wrote:
>>> From: Omar Sandoval <osandov@fb.com>
>>>
>>> Since statx was added in 4.11, userspace has had an interface for
>>> reading btime (file creation time), but no way to set it. This RFC patch
>>> series adds support for changing btime with utimensat(). Patch 1 adds
>>> the VFS infrastructure, patch 2 adds the support to utimensat() with a
>>> new flag, and the rest of the patches add filesystem support; I excluded
>>> CIFS for now because I don't have a CIFS setup to test it on.
>>>
>>> Updating btime is useful for at least a couple of use cases:
>>>
>>> - Backup/restore programs (my motivation for this feature is btrfs send)
>>
>> Can you give an example of such usefulness? What's the thing you run
>> into that you can't do without having this?
> 
> That boils down to what's useful about having the file creation time,
> and it's really just another tidbit of information which you may or may
> not care about. Maybe you have a document that you've been editing for
> awhile, and you want to know when you started working on it. Or, you
> want to know when a user created some directory that they keep adding
> files to.
> 
> If the file creation time is useful to you, then you likely want it
> preserved if you have to restore from backups. If I had to restore from
> backups yesterday and I'm trying to figure out when I started that
> document, I don't care that I restored that file yesterday, I want the
> real creation date.
> 
> If you've never wondered when a file was created, then I'm sure you
> won't care whether btrfs send/receive preserves it :)

Thanks for the elaborate answer. I was just curious if this was to make
send "more feature complete" or if there was something else special to
it, and it seems to be the first one. ;]

And for myself, I mostly treat btrfs send/receive like a improved
replacement for rsync and usually don't care that much about btime  indeed.

Thanks,
-- 
Hans van Kranenburg

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-15  6:59       ` Omar Sandoval
  2019-02-15 13:57         ` David Disseldorp
@ 2019-02-17  1:57         ` Andreas Dilger
  2019-02-18 22:18           ` Dave Chinner
  1 sibling, 1 reply; 29+ messages in thread
From: Andreas Dilger @ 2019-02-17  1:57 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: Dave Chinner, linux-fsdevel, Al Viro, kernel-team, Linux API,
	linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Steve French

[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]

On Feb 14, 2019, at 11:59 PM, Omar Sandoval <osandov@osandov.com> wrote:
> 
> On Fri, Feb 15, 2019 at 11:16:57AM +1100, Dave Chinner wrote:
>> On Thu, Feb 14, 2019 at 03:14:29PM -0800, Omar Sandoval wrote:
>>> I see a few options, none of which are particularly nice:
>>> 
>>> 1. Filesystems like XFS could choose not to support setting btime even
>>>   if they support reading it.
>>> 2. XFS could add a second, writeable btime which is used for
>>>   statx/utimes when available (it would fit in di_pad2...).
>>> 3. We could add a btime_writable sysctl/mount option/mkfs option.
>> 
>> 4. create time remains a read-only field, and btrfs grows its own
>> special interface to twiddle it in btrfs-recv if it really is
>> necessary.
> 
> I'm curious to hear what the ext4/f2fs/CIFS developers think. If no one
> else wants btime to be mutable, then I might as well make it
> Btrfs-specific. That is, assuming we reach consensus on the Btrfs side
> that btrfs receive should set btime.
> 
>> I'm still not convinced that even backup/restore should be doing this,
>> because there's so much other metadata that is unique even on
>> restored files that it doesn't really make any sense to me to lie
>> about it being created in the past....
> 
> I suppose it depends on how you interpret btime: if it's strictly
> filesystem metadata, then it makes sense that it should be immutable; if
> it's metadata for the user's own purposes, then we should allow setting
> it.

My personal preference is that crtime/btime be read-only information that
tells when the file itself was created in this filesystem, not some added
metadata that is managed by userspace.  There have been many times when
I've needed to know when a file was _actually_ created in the filesystem,
not what mtime/ctime report.

While it may be a bit of a stretch to call this "forensic evidence", making
it hard to change from except via total root compromise by a skilled hacker
is very useful.

If this were to go in (which I'm not in favour of), then there would need to
be a CONFIG and/or runtime knob to turn it off (or better to only turn it on),
similar to how FIPS and other security options can only go in one direction.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 22:06 ` [RFC PATCH 0/6] Allow setting file birth time with utimensat() Dave Chinner
  2019-02-14 23:14   ` Omar Sandoval
@ 2019-02-17 16:35   ` Boaz Harrosh
  2019-02-17 17:54     ` Adam Borowski
  2019-02-20  7:47     ` Andreas Dilger
  1 sibling, 2 replies; 29+ messages in thread
From: Boaz Harrosh @ 2019-02-17 16:35 UTC (permalink / raw)
  To: Dave Chinner, Omar Sandoval
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On 15/02/19 00:06, Dave Chinner wrote:
> On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
>> From: Omar Sandoval <osandov@fb.com>
>>
>> Hi,
>>
>> Since statx was added in 4.11, userspace has had an interface for
>> reading btime (file creation time), but no way to set it. This RFC patch
>> series adds support for changing btime with utimensat(). Patch 1 adds
>> the VFS infrastructure, patch 2 adds the support to utimensat() with a
>> new flag, and the rest of the patches add filesystem support; I excluded
>> CIFS for now because I don't have a CIFS setup to test it on.
>>
>> Updating btime is useful for at least a couple of use cases:
>>
[1]
>> - Backup/restore programs (my motivation for this feature is btrfs send)
>> - File servers which interoperate with operating systems that allow
>>   updating file creation time, including Mac OS [1] and Windows [2]
> 
> So you're adding an interface that allows users to change the create
> time of files without needing any privileges?
> 
[2]
> Inode create time is forensic metadata in XFS  - information we use
> for sequence of event and inode lifetime analysis during examination
> of broken filesystem images and systems that have been broken into.
> Just because it's exposed to userspace via statx(), it doesn't mean
> that it is information that users should be allowed to change. i.e.
> allowing users to be able to change the create time on files makes
> it completely useless for the purpose it was added to XFS for...
> 
<snap>

I think the difference in opinion here is that there are two totally
different BTIme out in the world. For two somewhat opposite motivations
and it seems they both try to be crammed into the same on disk space.

One - Author creation time
  This is a Windows originated creature and later MAC (and all vendors who
  make a living by serving cifs (hint see my email address))

  This is a tag carried globally on the globe denoting the time of the
  original creator of the file. copy, download, backup-restore and so
  on preserve it from the very first original creation.
  This creature is a user oriented information. That needs to be carefully
  orchestrated by all parties

Two - Local creation time
  This is an immutable local FS information that helps in debugging and
  FS-checking / recovery of data. It is an information that kind of denotes
  the order of creation of files on a local FS.

So it looks like both sides are correct trying to preserve their own guy?

XFS invented [2] I'd let it be. If you need [1] on XFS better push for
a well defined standardized xattr and be in peace.

BTRFS should decide which one of [2] or [1] it has space for in the inode
and commit to it. Documenting well what it is.

That said putting my Netapp hat. I would love to see an easy API
for Author-creation-time BTime type of value. That is accessed
uniformly by user-mode and/or Network file servers (NFS/CIFS).
And would love to see a generic implementation of that interface
that puts it into a standardized xattr if the FS in question does
not have a native support for it [1].

So I love these patches. And would want to see this through. But
let us understand each other?

Thanks
Boaz


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-17 16:35   ` Boaz Harrosh
@ 2019-02-17 17:54     ` Adam Borowski
  2019-02-17 20:40       ` Andy Lutomirski
  2019-02-20  7:47     ` Andreas Dilger
  1 sibling, 1 reply; 29+ messages in thread
From: Adam Borowski @ 2019-02-17 17:54 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Dave Chinner, Omar Sandoval, linux-fsdevel, Al Viro, kernel-team,
	linux-api, linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs

On Sun, Feb 17, 2019 at 06:35:25PM +0200, Boaz Harrosh wrote:
> On 15/02/19 00:06, Dave Chinner wrote:
> > So you're adding an interface that allows users to change the create
> > time of files without needing any privileges?

> > Inode create time is forensic metadata in XFS  - information we use
> > for sequence of event and inode lifetime analysis during examination
> > of broken filesystem images and systems that have been broken into.

> I think the difference in opinion here is that there are two totally
> different BTIme out in the world. For two somewhat opposite motivations
> and it seems they both try to be crammed into the same on disk space.
> 
> One - Author creation time
> Two - Local creation time

> So it looks like both sides are correct trying to preserve their own guy?

I'd say that [2] is too easily gameable to be worth the effort.  You can
just change it on the disk.  That right now it'd take some skill to find the
right place to edit doesn't matter -- a tool to update the btime against
your wishes would need to be written just once.  Unlike btrfs, XFS doesn't
even have a chain of checksums all the way to the root.

On the other hand, [1] has a lot of uses.  It can also be preserved in
backups and version control (svnt and git-restore-mtime could be easily
extended).

I'd thus go with [2] -- any uses for [1] are better delegated to filesystem
specific tools.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Have you accepted Khorne as your lord and saviour?
⠈⠳⣄⠀⠀⠀⠀

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-17 17:54     ` Adam Borowski
@ 2019-02-17 20:40       ` Andy Lutomirski
  2019-02-19  4:04         ` Matthew Wilcox
  0 siblings, 1 reply; 29+ messages in thread
From: Andy Lutomirski @ 2019-02-17 20:40 UTC (permalink / raw)
  To: Adam Borowski
  Cc: Boaz Harrosh, Dave Chinner, Omar Sandoval, Linux FS Devel,
	Al Viro, kernel-team, Linux API, Linux btrfs Developers List,
	Ext4 Developers List, linux-f2fs-devel, linux-xfs

On Sun, Feb 17, 2019 at 9:55 AM Adam Borowski <kilobyte@angband.pl> wrote:
>
> On Sun, Feb 17, 2019 at 06:35:25PM +0200, Boaz Harrosh wrote:
> > On 15/02/19 00:06, Dave Chinner wrote:
> > > So you're adding an interface that allows users to change the create
> > > time of files without needing any privileges?
>
> > > Inode create time is forensic metadata in XFS  - information we use
> > > for sequence of event and inode lifetime analysis during examination
> > > of broken filesystem images and systems that have been broken into.
>
> > I think the difference in opinion here is that there are two totally
> > different BTIme out in the world. For two somewhat opposite motivations
> > and it seems they both try to be crammed into the same on disk space.
> >
> > One - Author creation time
> > Two - Local creation time
>
> > So it looks like both sides are correct trying to preserve their own guy?
>
> I'd say that [2] is too easily gameable to be worth the effort.  You can
> just change it on the disk.  That right now it'd take some skill to find the
> right place to edit doesn't matter -- a tool to update the btime against
> your wishes would need to be written just once.  Unlike btrfs, XFS doesn't
> even have a chain of checksums all the way to the root.
>
> On the other hand, [1] has a lot of uses.  It can also be preserved in
> backups and version control (svnt and git-restore-mtime could be easily
> extended).
>
> I'd thus go with [2] -- any uses for [1] are better delegated to filesystem
> specific tools.
>

I started out in the Windows world, and I found it quite handy to
right-click a file and see when it was created.  When I started using
Linux, I saw things like this:

Access: 2019-02-16 22:19:32.024284060 -0800
Modify: 2016-02-02 19:26:47.901766778 -0800
Change: 2016-02-02 19:26:47.907766649 -0800

and my mind boggled a bit.  Modify makes sense.  Change?  What's that?
 Why do I care?

Adding "birth" makes sense, and I think that filesystem-agnostic
backup tools *should* be able to write it.

So I'm highly in favor of this patch.  If XFS wants to disallow
writing the birth time, fine, but I think that behavior should be
overridable.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-17  1:57         ` Andreas Dilger
@ 2019-02-18 22:18           ` Dave Chinner
  2019-02-22 19:00             ` Omar Sandoval
  0 siblings, 1 reply; 29+ messages in thread
From: Dave Chinner @ 2019-02-18 22:18 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Omar Sandoval, linux-fsdevel, Al Viro, kernel-team, Linux API,
	linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Steve French

On Sat, Feb 16, 2019 at 06:57:45PM -0700, Andreas Dilger wrote:
> While it may be a bit of a stretch to call this "forensic evidence", making

We do forensic analysis of corrupt filesystems looking for evidence
of what went wrong, not just looking for evidence of what happened
on systems that have been broken into.

> it hard to change from except via total root compromise by a skilled hacker
> is very useful.

*nod*.

> If this were to go in (which I'm not in favour of), then there would need to
> be a CONFIG and/or runtime knob to turn it off (or better to only turn it on),
> similar to how FIPS and other security options can only go in one direction.

The problem here is that "inode birth time" is being conflated with
"user document creation time". These two things are very different.

i.e. One is filesystem internal information and is not related to
when the original copy of the data in the file was created, the
other is user specified metadata that is related to the file data
contents and needs to travel with the data, not the filesystem.

IMO, trying to make one on-disk field hold two different types of
information defeats one or the other purpose, and nobody knows which
one the field stores for any given file.

I'd suggest that "authored date" should be a generic system xattr so
most filesystems support it, not just those that have a birth time
field on disk. Sure, modify it through utimesat() and expose it
through statx() (as authored time, not birth time), but store it a
system xattr rather than an internal filesystem metadata field that
requires was never intended to be user modifiable.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-17 20:40       ` Andy Lutomirski
@ 2019-02-19  4:04         ` Matthew Wilcox
  2019-02-19  4:28           ` Dave Chinner
  0 siblings, 1 reply; 29+ messages in thread
From: Matthew Wilcox @ 2019-02-19  4:04 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Adam Borowski, Boaz Harrosh, Dave Chinner, Omar Sandoval,
	Linux FS Devel, Al Viro, kernel-team, Linux API,
	Linux btrfs Developers List, Ext4 Developers List,
	linux-f2fs-devel, linux-xfs

On Sun, Feb 17, 2019 at 12:40:09PM -0800, Andy Lutomirski wrote:
> So I'm highly in favor of this patch.  If XFS wants to disallow
> writing the birth time, fine, but I think that behavior should be
> overridable.

Please, no.  We need to have consistent behaviour between at least
Linux local filesystems.  Not "Chris thinks this is a good idea,
while Dave and Ted think its a bad idea, so btrfs supports it and
XFS and ext4 disallow it".

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-19  4:04         ` Matthew Wilcox
@ 2019-02-19  4:28           ` Dave Chinner
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Chinner @ 2019-02-19  4:28 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andy Lutomirski, Adam Borowski, Boaz Harrosh, Omar Sandoval,
	Linux FS Devel, Al Viro, kernel-team, Linux API,
	Linux btrfs Developers List, Ext4 Developers List,
	linux-f2fs-devel, linux-xfs

On Mon, Feb 18, 2019 at 08:04:47PM -0800, Matthew Wilcox wrote:
> On Sun, Feb 17, 2019 at 12:40:09PM -0800, Andy Lutomirski wrote:
> > So I'm highly in favor of this patch.  If XFS wants to disallow
> > writing the birth time, fine, but I think that behavior should be
> > overridable.
> 
> Please, no.  We need to have consistent behaviour between at least
> Linux local filesystems.  Not "Chris thinks this is a good idea,
> while Dave and Ted think its a bad idea, so btrfs supports it and
> XFS and ext4 disallow it".

And, quite frankly, this is the entire reason xattrs exist. i.e.
so that generic file attributes can be stored persitently without
each individual having to support them in their on-disk format.

I wish people would stop trying to implement stuff like this in
filesystem code and instead added it to the VFS and stored it in VFS
defined system xattrs so that it is common across all filesystems.
It also means that backup applications can preserve them during file
copies without really even being aware of their meaning, simply by
copying all the xattrs on the file...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-17 16:35   ` Boaz Harrosh
  2019-02-17 17:54     ` Adam Borowski
@ 2019-02-20  7:47     ` Andreas Dilger
  1 sibling, 0 replies; 29+ messages in thread
From: Andreas Dilger @ 2019-02-20  7:47 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Dave Chinner, Omar Sandoval, linux-fsdevel, Al Viro, kernel-team,
	linux-api, linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

On Feb 17, 2019, at 8:35 AM, Boaz Harrosh <openosd@gmail.com> wrote:
> 
> On 15/02/19 00:06, Dave Chinner wrote:
>> On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
>>> From: Omar Sandoval <osandov@fb.com>
>>> 
>>> Hi,
>>> 
>>> Since statx was added in 4.11, userspace has had an interface for
>>> reading btime (file creation time), but no way to set it. This RFC patch
>>> series adds support for changing btime with utimensat(). Patch 1 adds
>>> the VFS infrastructure, patch 2 adds the support to utimensat() with a
>>> new flag, and the rest of the patches add filesystem support; I excluded
>>> CIFS for now because I don't have a CIFS setup to test it on.
>>> 
>>> Updating btime is useful for at least a couple of use cases:
>>> 
> [1]
>>> - Backup/restore programs (my motivation for this feature is btrfs send)
>>> - File servers which interoperate with operating systems that allow
>>>  updating file creation time, including Mac OS [1] and Windows [2]
>> 
>> So you're adding an interface that allows users to change the create
>> time of files without needing any privileges?
>> 
> [2]
>> Inode create time is forensic metadata in XFS  - information we use
>> for sequence of event and inode lifetime analysis during examination
>> of broken filesystem images and systems that have been broken into.
>> Just because it's exposed to userspace via statx(), it doesn't mean
>> that it is information that users should be allowed to change. i.e.
>> allowing users to be able to change the create time on files makes
>> it completely useless for the purpose it was added to XFS for...
>> 
> <snap>
> 
> I think the difference in opinion here is that there are two totally
> different BTIme out in the world. For two somewhat opposite motivations
> and it seems they both try to be crammed into the same on disk space.
> 
> One - Author creation time
>  This is a Windows originated creature and later MAC (and all vendors who
>  make a living by serving cifs (hint see my email address))
> 
>  This is a tag carried globally on the globe denoting the time of the
>  original creator of the file. copy, download, backup-restore and so
>  on preserve it from the very first original creation.
>  This creature is a user oriented information. That needs to be carefully
>  orchestrated by all parties.

One of the issues with user-supplied "creation time" is that it is vague.
If Vim (or many other programs) is writing out an existing file, it
will actually create a new file and rename it over the old file, so even
though the newly-written file is "created" at write time, the content is
older.  Unless the creation time is actually stored internal to the file
itself, rather than as filesystem metadata, it is unlikely to be kept
across filesystems, between systems, etc.

Conversely, there is already such metadata in existing file formats, e.g.
.jpg (Exif.Image.DateTime), .png (tEXt Creation Time), .docx (Date Completed),
etc. that sticks with the file regardless of the underlying storage medium.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
                   ` (10 preceding siblings ...)
  2019-02-15  1:57 ` Hans van Kranenburg
@ 2019-02-22 15:02 ` David Sterba
  11 siblings, 0 replies; 29+ messages in thread
From: David Sterba @ 2019-02-22 15:02 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: linux-fsdevel, Al Viro, kernel-team, linux-api, linux-btrfs,
	linux-ext4, linux-f2fs-devel, linux-xfs

On Thu, Feb 14, 2019 at 02:00:07AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> Since statx was added in 4.11, userspace has had an interface for
> reading btime (file creation time), but no way to set it. This RFC patch
> series adds support for changing btime with utimensat(). Patch 1 adds
> the VFS infrastructure, patch 2 adds the support to utimensat() with a
> new flag, and the rest of the patches add filesystem support; I excluded
> CIFS for now because I don't have a CIFS setup to test it on.
> 
> Updating btime is useful for at least a couple of use cases:
> 
> - Backup/restore programs (my motivation for this feature is btrfs send)
> - File servers which interoperate with operating systems that allow
>   updating file creation time, including Mac OS [1] and Windows [2]

I don't have anything new to add to what has been said in the thread.
The creation time is property of the filesystem and if user wants to
track additional metadata, then as external attributes.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-18 22:18           ` Dave Chinner
@ 2019-02-22 19:00             ` Omar Sandoval
  2019-02-23 18:32               ` Andreas Dilger
  0 siblings, 1 reply; 29+ messages in thread
From: Omar Sandoval @ 2019-02-22 19:00 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Andreas Dilger, linux-fsdevel, Al Viro, kernel-team, Linux API,
	linux-btrfs, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Steve French

On Tue, Feb 19, 2019 at 09:18:20AM +1100, Dave Chinner wrote:
> On Sat, Feb 16, 2019 at 06:57:45PM -0700, Andreas Dilger wrote:
> > While it may be a bit of a stretch to call this "forensic evidence", making
> 
> We do forensic analysis of corrupt filesystems looking for evidence
> of what went wrong, not just looking for evidence of what happened
> on systems that have been broken into.
> 
> > it hard to change from except via total root compromise by a skilled hacker
> > is very useful.
> 
> *nod*.
> 
> > If this were to go in (which I'm not in favour of), then there would need to
> > be a CONFIG and/or runtime knob to turn it off (or better to only turn it on),
> > similar to how FIPS and other security options can only go in one direction.
> 
> The problem here is that "inode birth time" is being conflated with
> "user document creation time". These two things are very different.
> 
> i.e. One is filesystem internal information and is not related to
> when the original copy of the data in the file was created, the
> other is user specified metadata that is related to the file data
> contents and needs to travel with the data, not the filesystem.
> 
> IMO, trying to make one on-disk field hold two different types of
> information defeats one or the other purpose, and nobody knows which
> one the field stores for any given file.
> 
> I'd suggest that "authored date" should be a generic system xattr so
> most filesystems support it, not just those that have a birth time
> field on disk. Sure, modify it through utimesat() and expose it
> through statx() (as authored time, not birth time), but store it a
> system xattr rather than an internal filesystem metadata field that
> requires was never intended to be user modifiable.

It seems that this is the general consensus, so I'll look into
implementing this functionality as an xattr.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH 0/6] Allow setting file birth time with utimensat()
  2019-02-22 19:00             ` Omar Sandoval
@ 2019-02-23 18:32               ` Andreas Dilger
  0 siblings, 0 replies; 29+ messages in thread
From: Andreas Dilger @ 2019-02-23 18:32 UTC (permalink / raw)
  To: Omar Sandoval
  Cc: Dave Chinner, linux-fsdevel, Al Viro, kernel-team, Linux API,
	linux-btrfs, Ext4 Developers List, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Steve French

[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]


> On Feb 22, 2019, at 11:00 AM, Omar Sandoval <osandov@osandov.com> wrote:
> 
> On Tue, Feb 19, 2019 at 09:18:20AM +1100, Dave Chinner wrote:
>> On Sat, Feb 16, 2019 at 06:57:45PM -0700, Andreas Dilger wrote:
>>> While it may be a bit of a stretch to call this "forensic evidence",
>> 
>> We do forensic analysis of corrupt filesystems looking for evidence
>> of what went wrong, not just looking for evidence of what happened
>> on systems that have been broken into.
>> 
>>> making it hard to change from except via total root compromise by a
>>> skilled hacker is very useful.
>> 
>> *nod*.
>> 
>>> If this were to go in (which I'm not in favour of), then there would
>>> need to be a CONFIG and/or runtime knob to turn it off (or better to
>>> only turn it on), similar to how FIPS and other security options can
>>> only go in one direction.
>> 
>> The problem here is that "inode birth time" is being conflated with
>> "user document creation time". These two things are very different.
>> 
>> i.e. One is filesystem internal information and is not related to
>> when the original copy of the data in the file was created, the
>> other is user specified metadata that is related to the file data
>> contents and needs to travel with the data, not the filesystem.
>> 
>> IMO, trying to make one on-disk field hold two different types of
>> information defeats one or the other purpose, and nobody knows which
>> one the field stores for any given file.
>> 
>> I'd suggest that "authored date" should be a generic system xattr so
>> most filesystems support it, not just those that have a birth time
>> field on disk. Sure, modify it through utimesat() and expose it
>> through statx() (as authored time, not birth time), but store it a
>> system xattr rather than an internal filesystem metadata field that
>> requires was never intended to be user modifiable.
> 
> It seems that this is the general consensus, so I'll look into
> implementing this functionality as an xattr.

I would recommend to look at how Samba is storing these attributes
today, and do the same thing, maybe add support into GNU coreutils
to handle this transparently.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-02-23 18:32 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 10:00 [RFC PATCH 0/6] Allow setting file birth time with utimensat() Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 1/6] fs: add btime to struct iattr Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 2/6] fs: add AT_UTIME_BTIME for utimensat() Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 3/6] Btrfs: add support for setting btime Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 4/6] ext4: " Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 5/6] f2fs: " Omar Sandoval
2019-02-14 10:00 ` [RFC PATCH 6/6] xfs: " Omar Sandoval
2019-02-14 10:00 ` [PATCH] generic: add a test for AT_UTIME_BTIME Omar Sandoval
2019-02-14 10:00 ` [PATCH] utimensat2: document AT_UTIME_BTIME Omar Sandoval
2019-02-14 10:00 ` [PATCH] xfs_io: add AT_UTIME_BTIME support Omar Sandoval
2019-02-14 22:06 ` [RFC PATCH 0/6] Allow setting file birth time with utimensat() Dave Chinner
2019-02-14 23:14   ` Omar Sandoval
2019-02-15  0:16     ` Dave Chinner
2019-02-15  6:59       ` Omar Sandoval
2019-02-15 13:57         ` David Disseldorp
2019-02-17  1:57         ` Andreas Dilger
2019-02-18 22:18           ` Dave Chinner
2019-02-22 19:00             ` Omar Sandoval
2019-02-23 18:32               ` Andreas Dilger
2019-02-17 16:35   ` Boaz Harrosh
2019-02-17 17:54     ` Adam Borowski
2019-02-17 20:40       ` Andy Lutomirski
2019-02-19  4:04         ` Matthew Wilcox
2019-02-19  4:28           ` Dave Chinner
2019-02-20  7:47     ` Andreas Dilger
2019-02-15  1:57 ` Hans van Kranenburg
2019-02-15  5:39   ` Omar Sandoval
2019-02-15 18:25     ` Hans van Kranenburg
2019-02-22 15:02 ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).