linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/5] vfs: Add timestamp range check support
@ 2017-04-08 19:37 Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 1/5] vfs: Add file timestamp range support Deepa Dinamani
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

The series is aimed at adding timestamp checking and policy
related to it to vfs.

The series was developed with discussions and guidance from
Arnd Bergmann.

The original thread is at https://lkml.org/lkml/2016/11/2/294

Associated test: xfstests generic/402
Note that the above test will be run and will fail all filesystems that
do not have correct limits specified in the xfstests or the kernel or
that don't support times beyond the test dates. I will be submitting a
follow up xfstest and kernel patches to update all filesystems.
Currently ext4 is the only filesystem that reflects correct limits.

The branch is available at
https://github.com/deepa-hub/vfs.git refs/heads/vfs_timestamp_policy

Changes since v4:
* Added documentation for boot param
Changes since v3:
* Remove redundant initializations in libfs.c
* Change early_param to __setup similar to other root mount options.
* Fix documentation warning
Changes since v2:
* Introduce early boot param override for checks.
* Drop afs patch for timestamp limits.
Changes since v1:
* return EROFS on mount errors
* fix mtime copy/paste error in utimes

Deepa Dinamani (5):
  vfs: Add file timestamp range support
  vfs: Add checks for filesystem timestamp limits
  ext4: Initialize timestamps limits
  vfs: Add timestamp_truncate() api
  utimes: Clamp the timestamps before update

 Documentation/admin-guide/kernel-parameters.txt |  8 +++++
 fs/ext4/ext4.h                                  |  4 +++
 fs/ext4/super.c                                 |  7 +++-
 fs/inode.c                                      | 47 ++++++++++++++++++++++++-
 fs/internal.h                                   |  2 ++
 fs/namespace.c                                  | 12 +++++++
 fs/super.c                                      |  9 +++++
 fs/utimes.c                                     | 17 ++++++---
 include/linux/fs.h                              |  4 +++
 include/linux/time64.h                          |  6 ++++
 include/uapi/linux/fs.h                         |  6 +++-
 kernel/sysctl.c                                 |  7 ++++
 12 files changed, 122 insertions(+), 7 deletions(-)

-- 
2.7.4

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

* [PATCH v5 1/5] vfs: Add file timestamp range support
  2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
@ 2017-04-08 19:37 ` Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits Deepa Dinamani
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

Add fields to the superblock to track the min and max
timestamps supported by filesystems.

Initially, when a superblock is allocated, initialize
it to the max and min values the fields can hold.
Individual filesystems override these to match their
actual limits.

Pseudo filesystems are assumed to always support the
min and max allowable values for the fields.

Note that the time ranges are saved in type time64_t
rather than time_t.
This is required because if we save ranges in time_t
then we would not be able to save timestamp ranges
for files that support timestamps beyond y2038.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/super.c             | 2 ++
 include/linux/fs.h     | 3 +++
 include/linux/time64.h | 2 ++
 3 files changed, 7 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index b8b6a08..f9c2241 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -247,6 +247,8 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
 	s->s_maxbytes = MAX_NON_LFS;
 	s->s_op = &default_op;
 	s->s_time_gran = 1000000000;
+	s->s_time_min = TIME64_MIN;
+	s->s_time_max = TIME64_MAX;
 	s->cleancache_poolid = CLEANCACHE_NO_POOL;
 
 	s->s_shrink.seeks = DEFAULT_SEEKS;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3c18fa6..63f83440 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1337,6 +1337,9 @@ struct super_block {
 	/* Granularity of c/m/atime in ns.
 	   Cannot be worse than a second */
 	u32		   s_time_gran;
+	/* Time limits for c/m/atime in seconds. */
+	time64_t           s_time_min;
+	time64_t           s_time_max;
 
 	/*
 	 * The next field is for VFS *only*. No filesystems have any business
diff --git a/include/linux/time64.h b/include/linux/time64.h
index 980c71b..25433b18 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -38,6 +38,8 @@ struct itimerspec64 {
 
 /* Located here for timespec[64]_valid_strict */
 #define TIME64_MAX			((s64)~((u64)1 << 63))
+#define TIME64_MIN			(-TIME64_MAX - 1)
+
 #define KTIME_MAX			((s64)~((u64)1 << 63))
 #define KTIME_SEC_MAX			(KTIME_MAX / NSEC_PER_SEC)
 
-- 
2.7.4

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

* [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 1/5] vfs: Add file timestamp range support Deepa Dinamani
@ 2017-04-08 19:37 ` Deepa Dinamani
  2017-04-08 20:04   ` Linus Torvalds
  2017-04-08 19:37 ` [PATCH v5 3/5] ext4: Initialize timestamps limits Deepa Dinamani
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

Allow read only mounts for filesystems that do not
have maximum timestamps beyond the y2038 expiry
timestamp.

Also, allow a sysctl override to all such filesystems
to be mounted with write permissions.
A boot param supports initial override of these
checks from the early boot without recompilation.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
 fs/inode.c                                      | 15 +++++++++++++++
 fs/internal.h                                   |  2 ++
 fs/namespace.c                                  | 12 ++++++++++++
 fs/super.c                                      |  7 +++++++
 include/linux/fs.h                              |  1 +
 include/linux/time64.h                          |  4 ++++
 include/uapi/linux/fs.h                         |  6 +++++-
 kernel/sysctl.c                                 |  7 +++++++
 9 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c2f220d..57f4a50 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1193,6 +1193,14 @@
 			can be changed at run time by the max_graph_depth file
 			in the tracefs tracing directory. default: 0 (no limit)
 
+	fstimestampcheck
+			Enable checking of max filesystem time supported
+			at mount time. The value is checked against y2038
+			date: Mon Jan 18 19:14:07 PST 2038. The option
+			disables rw mount of filesystems that are not able
+			to represent times beyond y2038 time mentioned above.
+			This check is off by default.
+
 	gamecon.map[2|3]=
 			[HW,JOY] Multisystem joystick and NES/SNES/PSX pad
 			support via parallel port (up to 5 devices per port)
diff --git a/fs/inode.c b/fs/inode.c
index a9caf53..a0c1522 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -75,6 +75,21 @@ static DEFINE_PER_CPU(unsigned long, nr_unused);
 
 static struct kmem_cache *inode_cachep __read_mostly;
 
+struct vfs_max_timestamp_check timestamp_check = {
+	.timestamp_supported = Y2038_EXPIRY_TIMESTAMP,
+	.check_on = 0,
+};
+
+static int __init setup_timestamp_check(char *str)
+{
+	if (*str)
+		return 0;
+	timestamp_check.check_on = 1;
+	return 1;
+}
+
+__setup("fstimestampcheck", setup_timestamp_check);
+
 static long get_nr_inodes(void)
 {
 	int i;
diff --git a/fs/internal.h b/fs/internal.h
index cef253a..76fbcde 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -67,6 +67,8 @@ extern int finish_automount(struct vfsmount *, struct path *);
 
 extern int sb_prepare_remount_readonly(struct super_block *);
 
+extern bool sb_file_times_updatable(struct super_block *sb);
+
 extern void __init mnt_init(void);
 
 extern int __mnt_want_write(struct vfsmount *);
diff --git a/fs/namespace.c b/fs/namespace.c
index 6b81c20..fd6e479 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -538,6 +538,18 @@ static void __mnt_unmake_readonly(struct mount *mnt)
 	unlock_mount_hash();
 }
 
+bool sb_file_times_updatable(struct super_block *sb)
+{
+
+	if (!timestamp_check.check_on)
+		return true;
+
+	if (sb->s_time_max > timestamp_check.timestamp_supported)
+		return true;
+
+	return false;
+}
+
 int sb_prepare_remount_readonly(struct super_block *sb)
 {
 	struct mount *mnt;
diff --git a/fs/super.c b/fs/super.c
index f9c2241..4e7577b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1245,6 +1245,13 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
 	WARN((sb->s_maxbytes < 0), "%s set sb->s_maxbytes to "
 		"negative value (%lld)\n", type->name, sb->s_maxbytes);
 
+	if (!(sb->s_flags & MS_RDONLY) && !sb_file_times_updatable(sb)) {
+		WARN(1, "File times cannot be updated on the filesystem.\n");
+		WARN(1, "Retry mounting the filesystem readonly.\n");
+		error = -EROFS;
+		goto out_sb;
+	}
+
 	up_write(&sb->s_umount);
 	free_secdata(secdata);
 	return root;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 63f83440..a39dc8e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -68,6 +68,7 @@ extern struct inodes_stat_t inodes_stat;
 extern int leases_enable, lease_break_time;
 extern int sysctl_protected_symlinks;
 extern int sysctl_protected_hardlinks;
+extern struct vfs_max_timestamp_check timestamp_check;
 
 struct buffer_head;
 typedef int (get_block_t)(struct inode *inode, sector_t iblock,
diff --git a/include/linux/time64.h b/include/linux/time64.h
index 25433b18..906e0b3 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -43,6 +43,10 @@ struct itimerspec64 {
 #define KTIME_MAX			((s64)~((u64)1 << 63))
 #define KTIME_SEC_MAX			(KTIME_MAX / NSEC_PER_SEC)
 
+/* Timestamps on boundary */
+#define Y2038_EXPIRY_TIMESTAMP		S32_MAX /* 2147483647 */
+#define Y2106_EXPIRY_TIMESTAMP		U32_MAX /* 4294967295 */
+
 #if __BITS_PER_LONG == 64
 
 static inline struct timespec timespec64_to_timespec(const struct timespec64 ts64)
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 048a85e..125e4ae 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -91,6 +91,11 @@ struct files_stat_struct {
 	unsigned long max_files;		/* tunable */
 };
 
+struct vfs_max_timestamp_check {
+	time64_t timestamp_supported;
+	int check_on;
+};
+
 struct inodes_stat_t {
 	long nr_inodes;
 	long nr_unused;
@@ -100,7 +105,6 @@ struct inodes_stat_t {
 
 #define NR_FILE  8192	/* this can well be larger on a larger system */
 
-
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
  */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 60474df..d88487c 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1668,6 +1668,13 @@ static struct ctl_table fs_table[] = {
 		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
+		.procname	= "fs-timestamp-check-on",
+		.data		= &timestamp_check.check_on,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+	{
 		.procname	= "nr_open",
 		.data		= &sysctl_nr_open,
 		.maxlen		= sizeof(unsigned int),
-- 
2.7.4

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

* [PATCH v5 3/5] ext4: Initialize timestamps limits
  2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 1/5] vfs: Add file timestamp range support Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits Deepa Dinamani
@ 2017-04-08 19:37 ` Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 4/5] vfs: Add timestamp_truncate() api Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 5/5] utimes: Clamp the timestamps before update Deepa Dinamani
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

ext4 has different overflow limits for max filesystem
timestamps based on the extra bytes available.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: linux-ext4@vger.kernel.org
---
 fs/ext4/ext4.h  | 4 ++++
 fs/ext4/super.c | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index fb69ee2..3292d4e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1640,6 +1640,10 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
 
 #define EXT4_GOOD_OLD_INODE_SIZE 128
 
+#define EXT4_EXTRA_TIMESTAMP_MAX	(((s64)1 << 34) - 1  + S32_MIN)
+#define EXT4_NON_EXTRA_TIMESTAMP_MAX	Y2038_EXPIRY_TIMESTAMP
+#define EXT4_TIMESTAMP_MIN		S32_MIN
+
 /*
  * Feature set definitions
  */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 73cae0c..0c1a864 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3687,8 +3687,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			       sbi->s_inode_size);
 			goto failed_mount;
 		}
-		if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE)
+		if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE) {
 			sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
+			sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
+		} else
+			sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
+
+		sb->s_time_min = EXT4_TIMESTAMP_MIN;
 	}
 
 	sbi->s_desc_size = le16_to_cpu(es->s_desc_size);
-- 
2.7.4

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

* [PATCH v5 4/5] vfs: Add timestamp_truncate() api
  2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
                   ` (2 preceding siblings ...)
  2017-04-08 19:37 ` [PATCH v5 3/5] ext4: Initialize timestamps limits Deepa Dinamani
@ 2017-04-08 19:37 ` Deepa Dinamani
  2017-04-08 19:37 ` [PATCH v5 5/5] utimes: Clamp the timestamps before update Deepa Dinamani
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

timespec_trunc() function is used to truncate a
filesystem timestamp to the right granularity.
But, the function does not clamp tv_sec part of the
timestamps according to the filesystem timestamp limits.

Also, timespec_trunc() is exclusively used for filesystem
timestamps. Move the api to be part of vfs.

The replacement api: timestamp_truncate() also alters the
signature of the function to accommodate filesystem
timestamp clamping according to flesystem limits.

Note that the clamp_t macro is used for clamping here as vfs
is not yet using struct timespec64 internally. This is
required for compilation purposes.
Also note that clamp won't do the right thing for timestamps
beyond 2038 on 32-bit machines until the vfs uses timespec64.
After the vfs is transitioned to use timespec64 for timestamps,
clamp_t() can be replaced by clamp().

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/inode.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index a0c1522..8ad5561 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2103,6 +2103,36 @@ void inode_nohighmem(struct inode *inode)
 EXPORT_SYMBOL(inode_nohighmem);
 
 /**
+ * timestamp_truncate - Truncate timespec to a granularity
+ * @t: Timespec
+ * @inode: inode being updated
+ *
+ * Truncate a timespec to the granularity supported by the fs
+ * containing the inode. Always rounds down. gran must
+ * not be 0 nor greater than a second (NSEC_PER_SEC, or 10^9 ns).
+ */
+struct timespec timestamp_truncate(struct timespec t, struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	unsigned int gran = sb->s_time_gran;
+
+	t.tv_sec = clamp_t(time64_t, t.tv_sec, sb->s_time_min, sb->s_time_max);
+
+	/* Avoid division in the common cases 1 ns and 1 s. */
+	if (gran == 1) {
+		/* nothing */
+	} else if (gran == NSEC_PER_SEC) {
+		t.tv_nsec = 0;
+	} else if (gran > 1 && gran < NSEC_PER_SEC) {
+		t.tv_nsec -= t.tv_nsec % gran;
+	} else {
+		WARN(1, "illegal file time granularity: %u", gran);
+	}
+	return t;
+}
+EXPORT_SYMBOL(timestamp_truncate);
+
+/**
  * current_time - Return FS time
  * @inode: inode.
  *
@@ -2121,6 +2151,6 @@ struct timespec current_time(struct inode *inode)
 		return now;
 	}
 
-	return timespec_trunc(now, inode->i_sb->s_time_gran);
+	return timestamp_truncate(now, inode);
 }
 EXPORT_SYMBOL(current_time);
-- 
2.7.4

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

* [PATCH v5 5/5] utimes: Clamp the timestamps before update
  2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
                   ` (3 preceding siblings ...)
  2017-04-08 19:37 ` [PATCH v5 4/5] vfs: Add timestamp_truncate() api Deepa Dinamani
@ 2017-04-08 19:37 ` Deepa Dinamani
  4 siblings, 0 replies; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-08 19:37 UTC (permalink / raw)
  To: viro, tytso, adilger.kernel, linux-ext4
  Cc: torvalds, tglx, linux-fsdevel, linux-kernel, arnd, y2038

POSIX.1 section for futimens, utimensat and utimes says:
The file's relevant timestamp shall be set to the
greatest value supported by the file system that is
not greater than the specified time.

Clamp the timestamps accordingly before assignment.

Note that the clamp_t macro is used for clamping here as vfs
is not yet using struct timespec64 internally. This is
required for compilation purposes.
Also note that clamp won't do the right thing for timestamps
beyond 2038 on 32-bit machines until the vfs uses timespec64.
After the vfs is transitioned to use timespec64 for timestamps,
clamp_t() can be replaced by clamp().

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/utimes.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/utimes.c b/fs/utimes.c
index 32b15b3..052fe5d 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -53,6 +53,7 @@ static int utimes_common(const struct path *path, struct timespec *times)
 	int error;
 	struct iattr newattrs;
 	struct inode *inode = path->dentry->d_inode;
+	struct super_block *sb = inode->i_sb;
 	struct inode *delegated_inode = NULL;
 
 	error = mnt_want_write(path->mnt);
@@ -68,16 +69,24 @@ static int utimes_common(const struct path *path, struct timespec *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_atime.tv_sec =
+				clamp_t(time64_t, times[0].tv_sec, sb->s_time_min, sb->s_time_max);
+			if (times[0].tv_sec >= sb->s_time_max)
+				newattrs.ia_atime.tv_nsec = 0;
+			else
+				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_mtime.tv_sec =
+				clamp_t(time64_t, times[1].tv_sec, sb->s_time_min, sb->s_time_max);
+			if (times[1].tv_sec >= sb->s_time_max)
+				newattrs.ia_mtime.tv_nsec = 0;
+			else
+				newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
 			newattrs.ia_valid |= ATTR_MTIME_SET;
 		}
 		/*
-- 
2.7.4

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

* Re: [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-08 19:37 ` [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits Deepa Dinamani
@ 2017-04-08 20:04   ` Linus Torvalds
  2017-04-09  2:58     ` Deepa Dinamani
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2017-04-08 20:04 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Al Viro, Theodore Ts'o, adilger.kernel, linux-ext4,
	Thomas Gleixner, linux-fsdevel, Linux Kernel Mailing List,
	Arnd Bergmann, y2038

On Sat, Apr 8, 2017 at 12:37 PM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> Allow read only mounts for filesystems that do not
> have maximum timestamps beyond the y2038 expiry
> timestamp.

This option seems arbitrary and pointless.

Nobody sane should ever enable it except for testing, but for testing
it would be much better to simply specify what the limit should be:
2038 is not magical for all filesystems, because the base may be
different.

And honestly, for testing, it would be much better to just make it a
mount option rather than some crazy system-wide one.

                   Linus

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

* Re: [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-08 20:04   ` Linus Torvalds
@ 2017-04-09  2:58     ` Deepa Dinamani
  2017-04-25 19:47       ` Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: Deepa Dinamani @ 2017-04-09  2:58 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Al Viro, Theodore Ts'o, adilger.kernel, linux-ext4,
	Thomas Gleixner, linux-fsdevel, Linux Kernel Mailing List,
	Arnd Bergmann, y2038 Mailman List

>> Allow read only mounts for filesystems that do not
>> have maximum timestamps beyond the y2038 expiry
>> timestamp.
>
> This option seems arbitrary and pointless.
>
> Nobody sane should ever enable it except for testing, but for testing
> it would be much better to simply specify what the limit should be:
> 2038 is not magical for all filesystems, because the base may be
> different.

Yes, the way the patch is right now, it is meant only for testing
y2038 readiness.
The feature is meant for system wide tests and not individual filesystem tests.

The original idea was to disallow writes on all filesystem mounts that
were not able to update times at the time of mount, meaning max time
supported by the filesystem should be greater than current system
time. But, then we end up with the problem of what to do about mounts
whose max time exceeds current time after mount. This can be handled
by some logic while updating inode times. But, maybe this level of
complexity is not required and we could just stick to the former use
case. And, just print a warning in the latter case. This is what
pushes the feature to be something more than y2038 readiness.

> And honestly, for testing, it would be much better to just make it a
> mount option rather than some crazy system-wide one.

The patch allows the y2038 number to be changed at compile time. I can
extend the sysctl and boot option to allow changing of this limit also
if that is preferred.

We also proposed the mount option route in the RFC. But, we received
no preferences/ comments. We proceeded with the sysctl option because
this allows us to extend this feature into disallowing writes on non
updatable time filesystems.

I could change this to providing a mount option instead if you think
that is better.

-Deepa

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

* Re: [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-09  2:58     ` Deepa Dinamani
@ 2017-04-25 19:47       ` Arnd Bergmann
  2017-04-25 20:02         ` Linus Torvalds
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2017-04-25 19:47 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Linus Torvalds, Al Viro, Theodore Ts'o, adilger.kernel,
	linux-ext4, Thomas Gleixner, linux-fsdevel,
	Linux Kernel Mailing List, y2038 Mailman List

On Sun, Apr 9, 2017 at 4:58 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>>> Allow read only mounts for filesystems that do not
>>> have maximum timestamps beyond the y2038 expiry
>>> timestamp.
>>
>> This option seems arbitrary and pointless.
>>
>> Nobody sane should ever enable it except for testing, but for testing
>> it would be much better to simply specify what the limit should be:
>> 2038 is not magical for all filesystems, because the base may be
>> different.
>
> Yes, the way the patch is right now, it is meant only for testing
> y2038 readiness.
> The feature is meant for system wide tests and not individual filesystem tests.

There is one global option that I want to see, and that is for completely
disabling all components that are known to be broken in y2038.

We could do this with just a compile-time option that primarily
turns off all drivers using the 32-bit time_t, but the same compile-time
option can also force the file system to be read-only.

I don't see this just as something we want to do for testing, but
also as a safeguard for people shipping embedded systems with
long service life: If something can go wrong after write-mounting
an ext3 file system after 2038, it's better to force a behavior now
that can be reasonably expected not to change.

Between doing a compile-time option or a boot-time option, doing
it purely compile-time is probably better as it gives us the possible
additional checking when we hide the time_t definition.

We can do the boot-time option as well, to set a particular limit
other than the one enforced at compile time. Passing a year
number like "fstimestampcheck=2099" would address Linus'
concern about the cutoff being arbitrary.

I would also make the default limit higher than 2038, as at
least the Apple HFS/HFS+ file systems break only a bit later
in 2040. However, I don't think any other file system breaks
until 2099 (some Microsoft file systems), which would be
the next reasonably default cutoff IMO.

>> And honestly, for testing, it would be much better to just make it a
>> mount option rather than some crazy system-wide one.
>
> The patch allows the y2038 number to be changed at compile time. I can
> extend the sysctl and boot option to allow changing of this limit also
> if that is preferred.
>
> We also proposed the mount option route in the RFC. But, we received
> no preferences/ comments. We proceeded with the sysctl option because
> this allows us to extend this feature into disallowing writes on non
> updatable time filesystems.
>
> I could change this to providing a mount option instead if you think
> that is better.

I don't see much value in a mount option that prevents the use,
but maybe a mount option to override the global setting to make
an exception for someone who does want to mount a particular
(known-broken) file system despite having the stricter global setting.

         Arnd

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

* Re: [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-25 19:47       ` Arnd Bergmann
@ 2017-04-25 20:02         ` Linus Torvalds
  2017-04-25 20:31           ` [Y2038] " Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2017-04-25 20:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Deepa Dinamani, Al Viro, Theodore Ts'o, adilger.kernel,
	linux-ext4, Thomas Gleixner, linux-fsdevel,
	Linux Kernel Mailing List, y2038 Mailman List

On Tue, Apr 25, 2017 at 12:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> There is one global option that I want to see, and that is for completely
> disabling all components that are known to be broken in y2038.

I really don't see the point.

Don't do it. Make it some local hack, I'm not taking crazy patches.

                    Linus

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

* Re: [Y2038] [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-25 20:02         ` Linus Torvalds
@ 2017-04-25 20:31           ` Arnd Bergmann
  2017-04-25 20:35             ` Linus Torvalds
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2017-04-25 20:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Theodore Ts'o, y2038 Mailman List, Linux Kernel Mailing List,
	adilger.kernel, Deepa Dinamani, linux-fsdevel, Thomas Gleixner,
	linux-ext4, Al Viro

On Tue, Apr 25, 2017 at 10:02 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Apr 25, 2017 at 12:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> There is one global option that I want to see, and that is for completely
>> disabling all components that are known to be broken in y2038.
>
> I really don't see the point.
>
> Don't do it. Make it some local hack, I'm not taking crazy patches.

I have the local hack , and used it to find all the drivers that use a
32-bit time_t internally (and mark them with a Kconfig dependency
for testing).

Would it be ok to have a simple way of removing the time_t definition (e.g.
by passing '-DREQUIRE_TIME64' to the compiler, but without the Kconfig
option? That way, someone who wants to ship a product can at least
find the obvious dependencies on stuff that remains broken.

       Arnd

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

* Re: [Y2038] [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-25 20:31           ` [Y2038] " Arnd Bergmann
@ 2017-04-25 20:35             ` Linus Torvalds
  2017-04-25 21:23               ` Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2017-04-25 20:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Theodore Ts'o, y2038 Mailman List, Linux Kernel Mailing List,
	adilger.kernel, Deepa Dinamani, linux-fsdevel, Thomas Gleixner,
	linux-ext4, Al Viro

On Tue, Apr 25, 2017 at 1:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> Would it be ok to have a simple way of removing the time_t definition (e.g.
> by passing '-DREQUIRE_TIME64' to the compiler, but without the Kconfig
> option? That way, someone who wants to ship a product can at least
> find the obvious dependencies on stuff that remains broken.

How would you find them?

People don't necessarily use "time_t". They might use "int" or whatever.

There is absolutely zero point to making this some kind of crazy
config option, because such an option will prove absolutely *NOTHING*.

Seriously. This whole concept is  completely stupid.

The only possible thing you can do is to

 (a) have an actual test-suite
 (b) set the time to 32+ bits
 (c) see what breaks

because otherwise it seems entirely pointless.

And no, we're not adding random crazy source modifications for pointless crap.

                      Linus

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

* Re: [Y2038] [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits
  2017-04-25 20:35             ` Linus Torvalds
@ 2017-04-25 21:23               ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2017-04-25 21:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Theodore Ts'o, y2038 Mailman List, Linux Kernel Mailing List,
	adilger.kernel, Deepa Dinamani, linux-fsdevel, Thomas Gleixner,
	linux-ext4, Al Viro

On Tue, Apr 25, 2017 at 10:35 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Apr 25, 2017 at 1:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> Would it be ok to have a simple way of removing the time_t definition (e.g.
>> by passing '-DREQUIRE_TIME64' to the compiler, but without the Kconfig
>> option? That way, someone who wants to ship a product can at least
>> find the obvious dependencies on stuff that remains broken.
>
> How would you find them?
>
> People don't necessarily use "time_t". They might use "int" or whatever.

My main approach has been:

* Assume that all of the time_t based interfaces are broken on 32-bit systems
  (some are not, but almost all are)

* For each interface that exposes a time_t to other files, introduce a
  replacement interface that is known to work

* Change users of the old interface over to the new one, one at a time,
  while manually reviewing all other code this interacts with.

Note that the vast majority of all the in-kernel uses of time_t variables
actually use timespec or timeval structures because they require
sub-second resolution, so we already know that they cannot
accidentally get assigned to 'int'. Also, we typically replace them with
ktime_t for efficiency. In case we replace a timespec with timespec64,
we do have to be careful to ensure that no code just treats the
tv_sec member as 'int' or 'long' though.

      Arnd

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

end of thread, other threads:[~2017-04-25 21:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-08 19:37 [PATCH v5 0/5] vfs: Add timestamp range check support Deepa Dinamani
2017-04-08 19:37 ` [PATCH v5 1/5] vfs: Add file timestamp range support Deepa Dinamani
2017-04-08 19:37 ` [PATCH v5 2/5] vfs: Add checks for filesystem timestamp limits Deepa Dinamani
2017-04-08 20:04   ` Linus Torvalds
2017-04-09  2:58     ` Deepa Dinamani
2017-04-25 19:47       ` Arnd Bergmann
2017-04-25 20:02         ` Linus Torvalds
2017-04-25 20:31           ` [Y2038] " Arnd Bergmann
2017-04-25 20:35             ` Linus Torvalds
2017-04-25 21:23               ` Arnd Bergmann
2017-04-08 19:37 ` [PATCH v5 3/5] ext4: Initialize timestamps limits Deepa Dinamani
2017-04-08 19:37 ` [PATCH v5 4/5] vfs: Add timestamp_truncate() api Deepa Dinamani
2017-04-08 19:37 ` [PATCH v5 5/5] utimes: Clamp the timestamps before update Deepa Dinamani

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).