linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/20] vfs: Add support for timestamp limits
@ 2019-07-30  1:49 Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 01/20] vfs: Add file timestamp range support Deepa Dinamani
                   ` (19 more replies)
  0 siblings, 20 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, adilger.kernel, adrian.hunter,
	aivazian.tigran, al, anna.schumaker, anton, anton, asmadeus,
	ccross, ceph-devel, coda, codalist, darrick.wong, dedekind1,
	devel, dsterba, dushistov, dwmw2, ericvh, gregkh, hch, hch,
	hirofumi, hubcap, idryomov, jack, jaegeuk, jaharkes,
	jfs-discussion, jlbec, keescook, linux-cifs, linux-ext4,
	linux-f2fs-devel, linux-karma-devel, linux-mtd, linux-nfs,
	linux-ntfs-dev, linux-xfs, lucho, luisbg, martin, me, mikulas,
	nico, phillip, reiserfs-devel, richard, sage, salah.triki,
	sfrench, shaggy, tj, tony.luck, trond.myklebust, tytso,
	v9fs-developer, yuchao0, zyan

The series is an update and a more complete version of the
previously posted series at
https://lore.kernel.org/linux-fsdevel/20180122020426.2988-1-deepa.kernel@gmail.com/

Thanks to Arnd Bergmann for doing a few preliminary reviews.
They helped me fix a few issues I had overlooked.

The limits (sometimes granularity also) for the filesystems updated here are according to the
following table:

File system   Time type                      Start year Expiration year Granularity
cramfs        fixed                          0          0
romfs         fixed                          0          0
pstore        ascii seconds (27 digit ascii) S64_MIN    S64_MAX         NSEC_PER_USEC
coda          INT64                          S64_MIN    S64_MAX         1
omfs          64-bit milliseconds            0          U64_MAX/ 1000   NSEC_PER_MSEC
befs          unsigned 48-bit seconds        0          0xffffffffffff  alloc_super
bfs           unsigned 32-bit seconds        0          U32_MAX         alloc_super
efs           unsigned 32-bit seconds        0          U32_MAX         alloc_super
ext2          signed 32-bit seconds          S32_MIN    S32_MAX         alloc_super
ext3          signed 32-bit seconds          S32_MIN    S32_MAX         alloc_super
ext4 (old)    signed 32-bit seconds          S32_MIN    S32_MAX         alloc_super
ext4 (extra)  34-bit seconds, 30-bit ns      S32_MIN    0x37fffffff	1
freevxfs      u32 secs/usecs                 0          U32_MAX         alloc_super
jffs2         unsigned 32-bit seconds        0          U32_MAX         alloc_super
jfs           unsigned 32-bit seconds/ns     0          U32_MAX         1
minix         unsigned 32-bit seconds        0          U32_MAX         alloc_super
orangefs      u64 seconds                    0          U64_MAX         alloc_super
qnx4          unsigned 32-bit seconds        0          U32_MAX         alloc_super
qnx6          unsigned 32-bit seconds        0          U32_MAX         alloc_super
reiserfs      unsigned 32-bit seconds        0          U32_MAX         alloc_super
squashfs      unsigned 32-bit seconds        0          U32_MAX         alloc_super
ufs1          signed 32-bit seconds          S32_MIN    S32_MAX         NSEC_PER_SEC
ufs2          signed 64-bit seconds/u32 ns   S64_MIN    S64_MAX         1
xfs           signed 32-bit seconds/ns       S32_MIN    S32_MAX         1
ceph          unsigned 32-bit second/ns      0          U32_MAX         1000
sysv          unsigned 32-bit seconds        0          U32_MAX         alloc_super
affs          u32 day, min, ticks            1978       u32_max days    NSEC_PER_SEC
nfsv2         unsigned 32-bit seconds/ns     0          U32_MAX         1
nfsv3         unsigned 32-bit seconds/ns     0          U32_MAX         1000
nfsv4         u64 seconds/u32 ns             S64_MIN    S64_MAX         1000
isofs         u8 year since 1900 (fixable)   1900       2155            alloc_super
hpfs          unsigned 32-bit seconds        1970       2106            alloc_super
fat           7-bit years, 2s resolution     1980       2107
cifs (smb)    7-bit years                    1980       2107
cifs (modern) 64-bit 100ns since 1601        1601       30828
adfs          40-bit cs since 1900           1900       2248
9p (9P2000)   unsigned 32-bit seconds        1970       2106
9p (9P2000.L) signed 64-bit seconds, ns      1970       S64_MAX

Granularity column filled in by the alloc_super() in the above table indicates that
the granularity is NSEC_PER_SEC.
Note that anything not mentioned above still has the default limits
S64_MIN..S64_MAX.

The patches in the series are as structured below:
1. Add vfs support to maintain the limits per filesystem.
2. Add a new timestamp_truncate() api for clamping timestamps
   according to the filesystem limits.
3. Add a warning for mount syscall to indicate the impending
   expiry of timestamps.
4. Modify utimes to clamp the timestamps.
5. Fill in limits for filesystems.

An updated version of the test for checking file system timestamp limits has been posted
at https://www.spinics.net/lists/fstests/msg12262.html

Changes from previous version:
* No change in mount behavior because of expiry of timestamps.
* Included limits for more filesystems.

Deepa Dinamani (20):
  vfs: Add file timestamp range support
  vfs: Add timestamp_truncate() api
  timestamp_truncate: Replace users of timespec64_trunc
  mount: Add mount warning for impending timestamp expiry
  utimes: Clamp the timestamps before update
  fs: Fill in max and min timestamps in superblock
  9p: Fill min and max timestamps in sb
  adfs: Fill in max and min timestamps in sb
  ext4: Initialize timestamps limits
  fs: nfs: Initialize filesystem timestamp ranges
  fs: cifs: Initialize filesystem timestamp ranges
  fs: fat: Initialize filesystem timestamp ranges
  fs: affs: Initialize filesystem timestamp ranges
  fs: sysv: Initialize filesystem timestamp ranges
  fs: ceph: Initialize filesystem timestamp ranges
  fs: orangefs: Initialize filesystem timestamp ranges
  fs: hpfs: Initialize filesystem timestamp ranges
  fs: omfs: Initialize filesystem timestamp ranges
  pstore: fs superblock limits
  isofs: Initialize filesystem timestamp ranges

 fs/9p/vfs_super.c        |  6 +++++-
 fs/adfs/adfs.h           | 13 +++++++++++++
 fs/adfs/inode.c          |  8 ++------
 fs/adfs/super.c          |  2 ++
 fs/affs/amigaffs.c       |  2 +-
 fs/affs/amigaffs.h       |  3 +++
 fs/affs/inode.c          |  4 ++--
 fs/affs/super.c          |  4 ++++
 fs/attr.c                | 21 ++++++++++++---------
 fs/befs/linuxvfs.c       |  2 ++
 fs/bfs/inode.c           |  2 ++
 fs/ceph/super.c          |  2 ++
 fs/cifs/cifsfs.c         | 22 ++++++++++++++++++++++
 fs/cifs/netmisc.c        | 14 +++++++-------
 fs/coda/inode.c          |  3 +++
 fs/configfs/inode.c      | 12 ++++++------
 fs/cramfs/inode.c        |  2 ++
 fs/efs/super.c           |  2 ++
 fs/ext2/super.c          |  2 ++
 fs/ext4/ext4.h           |  4 ++++
 fs/ext4/super.c          | 17 +++++++++++++++--
 fs/f2fs/file.c           | 21 ++++++++++++---------
 fs/fat/inode.c           | 12 ++++++++++++
 fs/fat/misc.c            |  5 +++--
 fs/freevxfs/vxfs_super.c |  2 ++
 fs/hpfs/hpfs_fn.h        |  6 ++----
 fs/hpfs/super.c          |  2 ++
 fs/inode.c               | 33 ++++++++++++++++++++++++++++++++-
 fs/isofs/inode.c         |  7 +++++++
 fs/jffs2/fs.c            |  3 +++
 fs/jfs/super.c           |  2 ++
 fs/kernfs/inode.c        |  6 +++---
 fs/minix/inode.c         |  2 ++
 fs/namespace.c           | 11 +++++++++++
 fs/nfs/super.c           | 20 +++++++++++++++++++-
 fs/ntfs/inode.c          | 21 ++++++++++++---------
 fs/omfs/inode.c          |  4 ++++
 fs/orangefs/super.c      |  2 ++
 fs/pstore/inode.c        |  4 +++-
 fs/qnx4/inode.c          |  2 ++
 fs/qnx6/inode.c          |  2 ++
 fs/reiserfs/super.c      |  3 +++
 fs/romfs/super.c         |  2 ++
 fs/squashfs/super.c      |  2 ++
 fs/super.c               |  2 ++
 fs/sysv/super.c          |  5 ++++-
 fs/ubifs/file.c          | 21 ++++++++++++---------
 fs/ufs/super.c           |  7 +++++++
 fs/utimes.c              | 17 +++++++++++++----
 fs/xfs/xfs_super.c       |  2 ++
 include/linux/fs.h       |  5 +++++
 include/linux/time64.h   |  2 ++
 52 files changed, 304 insertions(+), 78 deletions(-)

-- 
2.17.1

Cc: adilger.kernel@dilger.ca
Cc: adrian.hunter@intel.com
Cc: aivazian.tigran@gmail.com
Cc: al@alarsen.net
Cc: anna.schumaker@netapp.com
Cc: anton@enomsg.org
Cc: anton@tuxera.com
Cc: asmadeus@codewreck.org
Cc: ccross@android.com
Cc: ceph-devel@vger.kernel.org
Cc: coda@cs.cmu.edu
Cc: codalist@coda.cs.cmu.edu
Cc: darrick.wong@oracle.com
Cc: dedekind1@gmail.com
Cc: devel@lists.orangefs.org
Cc: dsterba@suse.com
Cc: dushistov@mail.ru
Cc: dwmw2@infradead.org
Cc: ericvh@gmail.com
Cc: gregkh@linuxfoundation.org
Cc: hch@infradead.org
Cc: hch@lst.de
Cc: hirofumi@mail.parknet.co.jp
Cc: hubcap@omnibond.com
Cc: idryomov@gmail.com
Cc: jack@suse.com
Cc: jaegeuk@kernel.org
Cc: jaharkes@cs.cmu.edu
Cc: jfs-discussion@lists.sourceforge.net
Cc: jlbec@evilplan.org
Cc: keescook@chromium.org
Cc: linux-cifs@vger.kernel.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-karma-devel@lists.sourceforge.net
Cc: linux-mtd@lists.infradead.org
Cc: linux-nfs@vger.kernel.org
Cc: linux-ntfs-dev@lists.sourceforge.net
Cc: linux-xfs@vger.kernel.org
Cc: lucho@ionkov.net
Cc: luisbg@kernel.org
Cc: martin@omnibond.com
Cc: me@bobcopeland.com
Cc: mikulas@artax.karlin.mff.cuni.cz
Cc: nico@fluxnic.net
Cc: phillip@squashfs.org.uk
Cc: reiserfs-devel@vger.kernel.org
Cc: richard@nod.at
Cc: sage@redhat.com
Cc: salah.triki@gmail.com
Cc: sfrench@samba.org
Cc: shaggy@kernel.org
Cc: tj@kernel.org
Cc: tony.luck@intel.com
Cc: trond.myklebust@hammerspace.com
Cc: tytso@mit.edu
Cc: v9fs-developer@lists.sourceforge.net
Cc: yuchao0@huawei.com
Cc: zyan@redhat.com

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

* [PATCH 01/20] vfs: Add file timestamp range support
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 02/20] vfs: Add timestamp_truncate() api Deepa Dinamani
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, 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.

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 2739f57515f8..e5835c38d336 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -257,6 +257,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 f1a6ca872943..e9d04e4e5628 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1448,6 +1448,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;
 #ifdef CONFIG_FSNOTIFY
 	__u32			s_fsnotify_mask;
 	struct fsnotify_mark_connector __rcu	*s_fsnotify_marks;
diff --git a/include/linux/time64.h b/include/linux/time64.h
index a620ee610b9f..19125489ae94 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -30,6 +30,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.17.1


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

* [PATCH 02/20] vfs: Add timestamp_truncate() api
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 01/20] vfs: Add file timestamp range support Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc Deepa Dinamani
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, 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.

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

Note that the tv_nsec part is set to 0 if tv_sec is not within
the range supported for the filesystem.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/inode.c         | 33 ++++++++++++++++++++++++++++++++-
 include/linux/fs.h |  2 ++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 5f5431ec3d62..0fb1f0fb296a 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2166,6 +2166,37 @@ struct timespec64 timespec64_trunc(struct timespec64 t, unsigned gran)
 }
 EXPORT_SYMBOL(timespec64_trunc);
 
+/**
+ * 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 timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	unsigned int gran = sb->s_time_gran;
+
+	t.tv_sec = clamp(t.tv_sec, sb->s_time_min, sb->s_time_max);
+	if (unlikely(t.tv_sec == sb->s_time_max || t.tv_sec == sb->s_time_min))
+		t.tv_nsec = 0;
+
+	/* 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, "invalid file time granularity: %u", gran);
+	return t;
+}
+EXPORT_SYMBOL(timestamp_truncate);
+
 /**
  * current_time - Return FS time
  * @inode: inode.
@@ -2187,6 +2218,6 @@ struct timespec64 current_time(struct inode *inode)
 		return now;
 	}
 
-	return timespec64_trunc(now, inode->i_sb->s_time_gran);
+	return timestamp_truncate(now, inode);
 }
 EXPORT_SYMBOL(current_time);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e9d04e4e5628..fdfe51d096fa 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -726,6 +726,8 @@ struct inode {
 	void			*i_private; /* fs or device private pointer */
 } __randomize_layout;
 
+struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode);
+
 static inline unsigned int i_blocksize(const struct inode *node)
 {
 	return (1 << node->i_blkbits);
-- 
2.17.1


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

* [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 01/20] vfs: Add file timestamp range support Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 02/20] vfs: Add timestamp_truncate() api Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  8:27   ` OGAWA Hirofumi
  2019-07-30  1:49 ` [PATCH 04/20] mount: Add mount warning for impending timestamp expiry Deepa Dinamani
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, adrian.hunter, anton, dedekind1,
	gregkh, hch, hirofumi, jaegeuk, jlbec, richard, tj, yuchao0,
	linux-f2fs-devel, linux-ntfs-dev, linux-mtd

Update the inode timestamp updates to use timestamp_truncate()
instead of timespec64_trunc().

The change was mostly generated by the following coccinelle
script.

virtual context
virtual patch

@r1 depends on patch forall@
struct inode *inode;
identifier i_xtime =~ "^i_[acm]time$";
expression e;
@@

inode->i_xtime =
- timespec64_trunc(
+ timestamp_truncate(
...,
- e);
+ inode);

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: adrian.hunter@intel.com
Cc: anton@tuxera.com
Cc: dedekind1@gmail.com
Cc: gregkh@linuxfoundation.org
Cc: hch@lst.de
Cc: hirofumi@mail.parknet.co.jp
Cc: jaegeuk@kernel.org
Cc: jlbec@evilplan.org
Cc: richard@nod.at
Cc: tj@kernel.org
Cc: yuchao0@huawei.com
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ntfs-dev@lists.sourceforge.net
Cc: linux-mtd@lists.infradead.org
---
 fs/attr.c           | 21 ++++++++++++---------
 fs/configfs/inode.c | 12 ++++++------
 fs/f2fs/file.c      | 21 ++++++++++++---------
 fs/fat/misc.c       |  5 +++--
 fs/kernfs/inode.c   |  6 +++---
 fs/ntfs/inode.c     | 21 ++++++++++++---------
 fs/ubifs/file.c     | 21 ++++++++++++---------
 7 files changed, 60 insertions(+), 47 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index d22e8187477f..df28035aa23e 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -183,15 +183,18 @@ void setattr_copy(struct inode *inode, const struct iattr *attr)
 		inode->i_uid = attr->ia_uid;
 	if (ia_valid & ATTR_GID)
 		inode->i_gid = attr->ia_gid;
-	if (ia_valid & ATTR_ATIME)
-		inode->i_atime = timespec64_trunc(attr->ia_atime,
-						  inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		inode->i_mtime = timespec64_trunc(attr->ia_mtime,
-						  inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		inode->i_ctime = timespec64_trunc(attr->ia_ctime,
-						  inode->i_sb->s_time_gran);
+	if (ia_valid & ATTR_ATIME) {
+		inode->i_atime = timestamp_truncate(attr->ia_atime,
+						  inode);
+	}
+	if (ia_valid & ATTR_MTIME) {
+		inode->i_mtime = timestamp_truncate(attr->ia_mtime,
+						  inode);
+	}
+	if (ia_valid & ATTR_CTIME) {
+		inode->i_ctime = timestamp_truncate(attr->ia_ctime,
+						  inode);
+	}
 	if (ia_valid & ATTR_MODE) {
 		umode_t mode = attr->ia_mode;
 
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index ab0284321912..884dcf06cfbe 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -76,14 +76,14 @@ int configfs_setattr(struct dentry * dentry, struct iattr * iattr)
 	if (ia_valid & ATTR_GID)
 		sd_iattr->ia_gid = iattr->ia_gid;
 	if (ia_valid & ATTR_ATIME)
-		sd_iattr->ia_atime = timespec64_trunc(iattr->ia_atime,
-						      inode->i_sb->s_time_gran);
+		sd_iattr->ia_atime = timestamp_truncate(iattr->ia_atime,
+						      inode);
 	if (ia_valid & ATTR_MTIME)
-		sd_iattr->ia_mtime = timespec64_trunc(iattr->ia_mtime,
-						      inode->i_sb->s_time_gran);
+		sd_iattr->ia_mtime = timestamp_truncate(iattr->ia_mtime,
+						      inode);
 	if (ia_valid & ATTR_CTIME)
-		sd_iattr->ia_ctime = timespec64_trunc(iattr->ia_ctime,
-						      inode->i_sb->s_time_gran);
+		sd_iattr->ia_ctime = timestamp_truncate(iattr->ia_ctime,
+						      inode);
 	if (ia_valid & ATTR_MODE) {
 		umode_t mode = iattr->ia_mode;
 
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 45b45f37d347..faf1e160961b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -744,15 +744,18 @@ static void __setattr_copy(struct inode *inode, const struct iattr *attr)
 		inode->i_uid = attr->ia_uid;
 	if (ia_valid & ATTR_GID)
 		inode->i_gid = attr->ia_gid;
-	if (ia_valid & ATTR_ATIME)
-		inode->i_atime = timespec64_trunc(attr->ia_atime,
-						  inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		inode->i_mtime = timespec64_trunc(attr->ia_mtime,
-						  inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		inode->i_ctime = timespec64_trunc(attr->ia_ctime,
-						  inode->i_sb->s_time_gran);
+	if (ia_valid & ATTR_ATIME) {
+		inode->i_atime = timestamp_truncate(attr->ia_atime,
+						  inode);
+	}
+	if (ia_valid & ATTR_MTIME) {
+		inode->i_mtime = timestamp_truncate(attr->ia_mtime,
+						  inode);
+	}
+	if (ia_valid & ATTR_CTIME) {
+		inode->i_ctime = timestamp_truncate(attr->ia_ctime,
+						  inode);
+	}
 	if (ia_valid & ATTR_MODE) {
 		umode_t mode = attr->ia_mode;
 
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 1e08bd54c5fb..53bb7c6bf993 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -307,8 +307,9 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
 		inode->i_atime = (struct timespec64){ seconds, 0 };
 	}
 	if (flags & S_CTIME) {
-		if (sbi->options.isvfat)
-			inode->i_ctime = timespec64_trunc(*now, 10000000);
+		if (sbi->options.isvfat) {
+			inode->i_ctime = timestamp_truncate(*now, inode);
+		}
 		else
 			inode->i_ctime = fat_timespec64_trunc_2secs(*now);
 	}
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index f3f3984cce80..892a58cfe7a1 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -161,9 +161,9 @@ static inline void set_inode_attr(struct inode *inode,
 	struct super_block *sb = inode->i_sb;
 	inode->i_uid = attrs->ia_uid;
 	inode->i_gid = attrs->ia_gid;
-	inode->i_atime = timespec64_trunc(attrs->ia_atime, sb->s_time_gran);
-	inode->i_mtime = timespec64_trunc(attrs->ia_mtime, sb->s_time_gran);
-	inode->i_ctime = timespec64_trunc(attrs->ia_ctime, sb->s_time_gran);
+	inode->i_atime = timestamp_truncate(attrs->ia_atime, inode);
+	inode->i_mtime = timestamp_truncate(attrs->ia_mtime, inode);
+	inode->i_ctime = timestamp_truncate(attrs->ia_ctime, inode);
 }
 
 static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode)
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 8baa34baf548..6c7388430ad3 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -2899,15 +2899,18 @@ int ntfs_setattr(struct dentry *dentry, struct iattr *attr)
 			ia_valid |= ATTR_MTIME | ATTR_CTIME;
 		}
 	}
-	if (ia_valid & ATTR_ATIME)
-		vi->i_atime = timespec64_trunc(attr->ia_atime,
-					       vi->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		vi->i_mtime = timespec64_trunc(attr->ia_mtime,
-					       vi->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		vi->i_ctime = timespec64_trunc(attr->ia_ctime,
-					       vi->i_sb->s_time_gran);
+	if (ia_valid & ATTR_ATIME) {
+		vi->i_atime = timestamp_truncate(attr->ia_atime,
+					       vi);
+	}
+	if (ia_valid & ATTR_MTIME) {
+		vi->i_mtime = timestamp_truncate(attr->ia_mtime,
+					       vi);
+	}
+	if (ia_valid & ATTR_CTIME) {
+		vi->i_ctime = timestamp_truncate(attr->ia_ctime,
+					       vi);
+	}
 	mark_inode_dirty(vi);
 out:
 	return err;
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 400970d740bb..cd52585c8f4f 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1078,15 +1078,18 @@ static void do_attr_changes(struct inode *inode, const struct iattr *attr)
 		inode->i_uid = attr->ia_uid;
 	if (attr->ia_valid & ATTR_GID)
 		inode->i_gid = attr->ia_gid;
-	if (attr->ia_valid & ATTR_ATIME)
-		inode->i_atime = timespec64_trunc(attr->ia_atime,
-						  inode->i_sb->s_time_gran);
-	if (attr->ia_valid & ATTR_MTIME)
-		inode->i_mtime = timespec64_trunc(attr->ia_mtime,
-						  inode->i_sb->s_time_gran);
-	if (attr->ia_valid & ATTR_CTIME)
-		inode->i_ctime = timespec64_trunc(attr->ia_ctime,
-						  inode->i_sb->s_time_gran);
+	if (attr->ia_valid & ATTR_ATIME) {
+		inode->i_atime = timestamp_truncate(attr->ia_atime,
+						  inode);
+	}
+	if (attr->ia_valid & ATTR_MTIME) {
+		inode->i_mtime = timestamp_truncate(attr->ia_mtime,
+						  inode);
+	}
+	if (attr->ia_valid & ATTR_CTIME) {
+		inode->i_ctime = timestamp_truncate(attr->ia_ctime,
+						  inode);
+	}
 	if (attr->ia_valid & ATTR_MODE) {
 		umode_t mode = attr->ia_mode;
 
-- 
2.17.1


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

* [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (2 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-08-05 14:12   ` [Y2038] " Ben Hutchings
  2019-08-05 14:14   ` Ben Hutchings
  2019-07-30  1:49 ` [PATCH 05/20] utimes: Clamp the timestamps before update Deepa Dinamani
                   ` (15 subsequent siblings)
  19 siblings, 2 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038

The warning reuses the uptime max of 30 years used by the
setitimeofday().

Note that the warning is only added for new filesystem mounts
through the mount syscall. Automounts do not have the same warning.

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

diff --git a/fs/namespace.c b/fs/namespace.c
index b26778bdc236..5314fac8035e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
 	error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
 	if (error < 0)
 		mntput(mnt);
+
+	if (!error && sb->s_time_max &&
+	    (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
+		char *buf = (char *)__get_free_page(GFP_KERNEL);
+		char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
+
+		pr_warn("Mounted %s file system at %s supports timestamps until 0x%llx\n",
+			fc->fs_type->name, mntpath, (unsigned long long)sb->s_time_max);
+		free_page((unsigned long)buf);
+	}
+
 	return error;
 }
 
-- 
2.17.1


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

* [PATCH 05/20] utimes: Clamp the timestamps before update
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (3 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 04/20] mount: Add mount warning for impending timestamp expiry Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-31 15:14   ` Darrick J. Wong
  2019-08-05 13:30   ` [Y2038] " Ben Hutchings
  2019-07-30  1:49 ` [PATCH 06/20] fs: Fill in max and min timestamps in superblock Deepa Dinamani
                   ` (14 subsequent siblings)
  19 siblings, 2 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038

POSIX is ambiguous on the behavior of timestamps for
futimens, utimensat and utimes. Whether to return an
error or silently clamp a timestamp beyond the range
supported by the underlying filesystems is not clear.

POSIX.1 section for futimens, utimensat and utimes says:
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html)

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.

If the tv_nsec field of a timespec structure has the special
value UTIME_NOW, the file's relevant timestamp shall be set
to the greatest value supported by the file system that is
not greater than the current time.

[EINVAL]
    A new file timestamp would be a value whose tv_sec
    component is not a value supported by the file system.

The patch chooses to clamp the timestamps according to the
filesystem timestamp ranges and does not return an error.
This is in line with the behavior of utime syscall also
since the POSIX page(http://pubs.opengroup.org/onlinepubs/009695399/functions/utime.html)
for utime does not mention returning an error or clamping like above.

Same for utimes http://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html

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 350c9c16ace1..4c1a2ce90bbc 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -21,6 +21,7 @@ static int utimes_common(const struct path *path, struct timespec64 *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);
@@ -36,16 +37,24 @@ static int utimes_common(const struct path *path, struct timespec64 *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(times[0].tv_sec, sb->s_time_min, sb->s_time_max);
+			if (times[0].tv_sec == sb->s_time_max || times[0].tv_sec == sb->s_time_min)
+				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(times[1].tv_sec, sb->s_time_min, sb->s_time_max);
+			if (times[1].tv_sec >= sb->s_time_max || times[1].tv_sec == sb->s_time_min)
+				newattrs.ia_mtime.tv_nsec = 0;
+			else
+				newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
 			newattrs.ia_valid |= ATTR_MTIME_SET;
 		}
 		/*
-- 
2.17.1


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

* [PATCH 06/20] fs: Fill in max and min timestamps in superblock
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (4 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 05/20] utimes: Clamp the timestamps before update Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-31 15:28   ` Darrick J. Wong
  2019-07-30  1:49 ` [PATCH 07/20] 9p: Fill min and max timestamps in sb Deepa Dinamani
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, aivazian.tigran, al, coda,
	darrick.wong, dushistov, dwmw2, hch, jack, jaharkes, luisbg,
	nico, phillip, richard, salah.triki, shaggy, linux-xfs, codalist,
	linux-ext4, linux-mtd, jfs-discussion, reiserfs-devel

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Even though some filesystems are read-only, fill in the
timestamps to reflect the on-disk representation.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: aivazian.tigran@gmail.com
Cc: al@alarsen.net
Cc: coda@cs.cmu.edu
Cc: darrick.wong@oracle.com
Cc: dushistov@mail.ru
Cc: dwmw2@infradead.org
Cc: hch@infradead.org
Cc: jack@suse.com
Cc: jaharkes@cs.cmu.edu
Cc: luisbg@kernel.org
Cc: nico@fluxnic.net
Cc: phillip@squashfs.org.uk
Cc: richard@nod.at
Cc: salah.triki@gmail.com
Cc: shaggy@kernel.org
Cc: linux-xfs@vger.kernel.org
Cc: codalist@coda.cs.cmu.edu
Cc: linux-ext4@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Cc: jfs-discussion@lists.sourceforge.net
Cc: reiserfs-devel@vger.kernel.org
---
 fs/befs/linuxvfs.c       | 2 ++
 fs/bfs/inode.c           | 2 ++
 fs/coda/inode.c          | 3 +++
 fs/cramfs/inode.c        | 2 ++
 fs/efs/super.c           | 2 ++
 fs/ext2/super.c          | 2 ++
 fs/freevxfs/vxfs_super.c | 2 ++
 fs/jffs2/fs.c            | 3 +++
 fs/jfs/super.c           | 2 ++
 fs/minix/inode.c         | 2 ++
 fs/qnx4/inode.c          | 2 ++
 fs/qnx6/inode.c          | 2 ++
 fs/reiserfs/super.c      | 3 +++
 fs/romfs/super.c         | 2 ++
 fs/squashfs/super.c      | 2 ++
 fs/ufs/super.c           | 7 +++++++
 fs/xfs/xfs_super.c       | 2 ++
 17 files changed, 42 insertions(+)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 462d096ff3e9..64cdf4d8e424 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -893,6 +893,8 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
 	sb_set_blocksize(sb, (ulong) befs_sb->block_size);
 	sb->s_op = &befs_sops;
 	sb->s_export_op = &befs_export_operations;
+	sb->s_time_min = 0;
+	sb->s_time_max = 0xffffffffffffll;
 	root = befs_iget(sb, iaddr2blockno(sb, &(befs_sb->root_dir)));
 	if (IS_ERR(root)) {
 		ret = PTR_ERR(root);
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 5e97bed073d7..f8ce1368218b 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -324,6 +324,8 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
 		return -ENOMEM;
 	mutex_init(&info->bfs_lock);
 	s->s_fs_info = info;
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
 
 	sb_set_blocksize(s, BFS_BSIZE);
 
diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index 321f56e487cb..b1c70e2b9b1e 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -188,6 +188,9 @@ static int coda_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_magic = CODA_SUPER_MAGIC;
 	sb->s_op = &coda_super_operations;
 	sb->s_d_op = &coda_dentry_operations;
+	sb->s_time_gran = 1;
+	sb->s_time_min = S64_MIN;
+	sb->s_time_max = S64_MAX;
 
 	error = super_setup_bdi(sb);
 	if (error)
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 9352487bd0fc..4d1d8b7761ed 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -597,6 +597,8 @@ static int cramfs_finalize_super(struct super_block *sb,
 
 	/* Set it all up.. */
 	sb->s_flags |= SB_RDONLY;
+	sb->s_time_min = 0;
+	sb->s_time_max = 0;
 	sb->s_op = &cramfs_ops;
 	root = get_cramfs_inode(sb, cramfs_root, 0);
 	if (IS_ERR(root))
diff --git a/fs/efs/super.c b/fs/efs/super.c
index 867fc24dee20..4a6ebff2af76 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -257,6 +257,8 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	if (!sb)
 		return -ENOMEM;
 	s->s_fs_info = sb;
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
  
 	s->s_magic		= EFS_SUPER_MAGIC;
 	if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) {
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 44eb6e7eb492..baa36c6fb71e 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1002,6 +1002,8 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_maxbytes = ext2_max_size(sb->s_blocksize_bits);
 	sb->s_max_links = EXT2_LINK_MAX;
+	sb->s_time_min = S32_MIN;
+	sb->s_time_max = S32_MAX;
 
 	if (le32_to_cpu(es->s_rev_level) == EXT2_GOOD_OLD_REV) {
 		sbi->s_inode_size = EXT2_GOOD_OLD_INODE_SIZE;
diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
index a89f68c3cbed..578a5062706e 100644
--- a/fs/freevxfs/vxfs_super.c
+++ b/fs/freevxfs/vxfs_super.c
@@ -229,6 +229,8 @@ static int vxfs_fill_super(struct super_block *sbp, void *dp, int silent)
 
 	sbp->s_op = &vxfs_super_ops;
 	sbp->s_fs_info = infp;
+	sbp->s_time_min = 0;
+	sbp->s_time_max = U32_MAX;
 
 	if (!vxfs_try_sb_magic(sbp, silent, 1,
 			(__force __fs32)cpu_to_le32(VXFS_SUPER_MAGIC))) {
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 8a20ddd25f2d..d0b59d03a7a9 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -590,6 +590,9 @@ int jffs2_do_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_blocksize = PAGE_SIZE;
 	sb->s_blocksize_bits = PAGE_SHIFT;
 	sb->s_magic = JFFS2_SUPER_MAGIC;
+	sb->s_time_min = 0;
+	sb->s_time_max = U32_MAX;
+
 	if (!sb_rdonly(sb))
 		jffs2_start_garbage_collect_thread(c);
 	return 0;
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index f4e10cb9f734..b2dc4d1f9dcc 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -503,6 +503,8 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_fs_info = sbi;
 	sb->s_max_links = JFS_LINK_MAX;
+	sb->s_time_min = 0;
+	sb->s_time_max = U32_MAX;
 	sbi->sb = sb;
 	sbi->uid = INVALID_UID;
 	sbi->gid = INVALID_GID;
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index f96073f25432..7cb5fd38eb14 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -277,6 +277,8 @@ static int minix_fill_super(struct super_block *s, void *data, int silent)
 
 	/* set up enough so that it can read an inode */
 	s->s_op = &minix_sops;
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
 	root_inode = minix_iget(s, MINIX_ROOT_INO);
 	if (IS_ERR(root_inode)) {
 		ret = PTR_ERR(root_inode);
diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 922d083bbc7c..e8da1cde87b9 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -201,6 +201,8 @@ static int qnx4_fill_super(struct super_block *s, void *data, int silent)
 	s->s_op = &qnx4_sops;
 	s->s_magic = QNX4_SUPER_MAGIC;
 	s->s_flags |= SB_RDONLY;	/* Yup, read-only yet */
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
 
 	/* Check the superblock signature. Since the qnx4 code is
 	   dangerous, we should leave as quickly as possible
diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 0f8b0ff1ba43..345db56c98fd 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -429,6 +429,8 @@ static int qnx6_fill_super(struct super_block *s, void *data, int silent)
 	s->s_op = &qnx6_sops;
 	s->s_magic = QNX6_SUPER_MAGIC;
 	s->s_flags |= SB_RDONLY;        /* Yup, read-only yet */
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
 
 	/* ease the later tree level calculations */
 	sbi = QNX6_SB(s);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index ab028ea0e561..d69b4ac0ae2f 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1976,6 +1976,9 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
 		goto error_unlocked;
 	}
 
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
+
 	rs = SB_DISK_SUPER_BLOCK(s);
 	/*
 	 * Let's do basic sanity check to verify that underlying device is not
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 7d580f7c3f1d..a42c0e3079dc 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -478,6 +478,8 @@ static int romfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_maxbytes = 0xFFFFFFFF;
 	sb->s_magic = ROMFS_MAGIC;
 	sb->s_flags |= SB_RDONLY | SB_NOATIME;
+	sb->s_time_min = 0;
+	sb->s_time_max = 0;
 	sb->s_op = &romfs_super_ops;
 
 #ifdef CONFIG_ROMFS_ON_MTD
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index effa638d6d85..a9e9837617a9 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -183,6 +183,8 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
 		(u64) le64_to_cpu(sblk->id_table_start));
 
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	sb->s_time_min = 0;
+	sb->s_time_max = U32_MAX;
 	sb->s_flags |= SB_RDONLY;
 	sb->s_op = &squashfs_super_ops;
 
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 4ed0dca52ec8..1da0be667409 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -843,6 +843,10 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 
+	sb->s_time_gran = NSEC_PER_SEC;
+	sb->s_time_min = S32_MIN;
+	sb->s_time_max = S32_MAX;
+
 	switch (sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) {
 	case UFS_MOUNT_UFSTYPE_44BSD:
 		UFSD("ufstype=44bsd\n");
@@ -861,6 +865,9 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 		uspi->s_fshift = 9;
 		uspi->s_sbsize = super_block_size = 1536;
 		uspi->s_sbbase =  0;
+		sb->s_time_gran = 1;
+		sb->s_time_min = S64_MIN;
+		sb->s_time_max = S64_MAX;
 		flags |= UFS_TYPE_UFS2 | UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
 		break;
 		
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index a14d11d78bd8..1a0daf46bae8 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1685,6 +1685,8 @@ xfs_fs_fill_super(
 	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
 	sb->s_max_links = XFS_MAXLINK;
 	sb->s_time_gran = 1;
+	sb->s_time_min = S32_MIN;
+	sb->s_time_max = S32_MAX;
 	set_posix_acl_flag(sb);
 
 	/* version 5 superblocks support inode version counters. */
-- 
2.17.1


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

* [PATCH 07/20] 9p: Fill min and max timestamps in sb
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (5 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 06/20] fs: Fill in max and min timestamps in superblock Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 08/20] adfs: Fill in max and min " Deepa Dinamani
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, ericvh, lucho, asmadeus, v9fs-developer

struct p9_wstat and struct p9_stat_dotl indicate that the
wire transport uses u32 and u64 fields for timestamps.
Fill in the appropriate limits to avoid inconsistencies in
the vfs cached inode times when timestamps are outside the
permitted range.

Note that the upper bound for V9FS_PROTO_2000L is retained as S64_MAX.
This is because that is the upper bound supported by vfs.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: ericvh@gmail.com
Cc: lucho@ionkov.net
Cc: asmadeus@codewreck.org
Cc: v9fs-developer@lists.sourceforge.net
---
 fs/9p/vfs_super.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 08112fbcaece..ca243e658d71 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -69,8 +69,12 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 	if (v9fs_proto_dotl(v9ses)) {
 		sb->s_op = &v9fs_super_ops_dotl;
 		sb->s_xattr = v9fs_xattr_handlers;
-	} else
+	} else {
 		sb->s_op = &v9fs_super_ops;
+		sb->s_time_max = U32_MAX;
+	}
+
+	sb->s_time_min = 0;
 
 	ret = super_setup_bdi(sb);
 	if (ret)
-- 
2.17.1


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

* [PATCH 08/20] adfs: Fill in max and min timestamps in sb
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (6 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 07/20] 9p: Fill min and max timestamps in sb Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 09/20] ext4: Initialize timestamps limits Deepa Dinamani
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Note that the min timestamp is assumed to be
01 Jan 1970 00:00:00 (Unix epoch). This is consistent
with the way we convert timestamps in adfs_adfs2unix_time().

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/adfs/adfs.h  | 13 +++++++++++++
 fs/adfs/inode.c |  8 ++------
 fs/adfs/super.c |  2 ++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/adfs/adfs.h b/fs/adfs/adfs.h
index 804c6a77c5db..781b1c3817e0 100644
--- a/fs/adfs/adfs.h
+++ b/fs/adfs/adfs.h
@@ -2,6 +2,19 @@
 #include <linux/fs.h>
 #include <linux/adfs_fs.h>
 
+/*
+ * 01 Jan 1970 00:00:00 (Unix epoch) as seconds since
+ * 01 Jan 1900 00:00:00 (RISC OS epoch)
+ */
+#define RISC_OS_EPOCH_DELTA 2208988800LL
+
+/*
+ * Convert 40 bit centi seconds to seconds
+ * since 01 Jan 1900 00:00:00 (RISC OS epoch)
+ * The result is 2248-06-03 06:57:57 GMT
+ */
+#define ADFS_MAX_TIMESTAMP ((0xFFFFFFFFFFLL / 100) - RISC_OS_EPOCH_DELTA)
+
 /* Internal data structures for ADFS */
 
 #define ADFS_FREE_FRAG		 0
diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c
index 66621e96f9af..3f75cefc0380 100644
--- a/fs/adfs/inode.c
+++ b/fs/adfs/inode.c
@@ -170,11 +170,7 @@ static void
 adfs_adfs2unix_time(struct timespec64 *tv, struct inode *inode)
 {
 	unsigned int high, low;
-	/* 01 Jan 1970 00:00:00 (Unix epoch) as nanoseconds since
-	 * 01 Jan 1900 00:00:00 (RISC OS epoch)
-	 */
-	static const s64 nsec_unix_epoch_diff_risc_os_epoch =
-							2208988800000000000LL;
+	static const s64 nsec_unix_epoch_diff_risc_os_epoch = RISC_OS_EPOCH_DELTA * NSEC_PER_SEC;
 	s64 nsec;
 
 	if (ADFS_I(inode)->stamped == 0)
@@ -219,7 +215,7 @@ adfs_unix2adfs_time(struct inode *inode, unsigned int secs)
 	if (ADFS_I(inode)->stamped) {
 		/* convert 32-bit seconds to 40-bit centi-seconds */
 		low  = (secs & 255) * 100;
-		high = (secs / 256) * 100 + (low >> 8) + 0x336e996a;
+		high = (secs / 256) * 100 + (low >> 8) + (RISC_OS_EPOCH_DELTA*100/256);
 
 		ADFS_I(inode)->loadaddr = (high >> 24) |
 				(ADFS_I(inode)->loadaddr & ~0xff);
diff --git a/fs/adfs/super.c b/fs/adfs/super.c
index 2a83655c408f..0a0854ef9e3c 100644
--- a/fs/adfs/super.c
+++ b/fs/adfs/super.c
@@ -449,6 +449,8 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 	asb->s_size    		= adfs_discsize(dr, sb->s_blocksize_bits);
 	asb->s_version 		= dr->format_version;
 	asb->s_log2sharesize	= dr->log2sharesize;
+	sb->s_time_min		= 0;
+	sb->s_time_max		= ADFS_MAX_TIMESTAMP;
 
 	asb->s_map = adfs_read_map(sb, dr);
 	if (IS_ERR(asb->s_map)) {
-- 
2.17.1


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

* [PATCH 09/20] ext4: Initialize timestamps limits
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (7 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 08/20] adfs: Fill in max and min " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-31 15:26   ` Darrick J. Wong
  2019-07-30  1:49 ` [PATCH 10/20] fs: nfs: Initialize filesystem timestamp ranges Deepa Dinamani
                   ` (10 subsequent siblings)
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, tytso, adilger.kernel, linux-ext4

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

The timestamp limits are calculated according to the
encoding table in
a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):

* extra  msb of                         adjust for signed
* epoch  32-bit                         32-bit tv_sec to
* bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
* 0 0    1    -0x80000000..-0x00000001  0x000000000   1901-12-13..1969-12-31
* 0 0    0    0x000000000..0x07fffffff  0x000000000   1970-01-01..2038-01-19
* 0 1    1    0x080000000..0x0ffffffff  0x100000000   2038-01-19..2106-02-07
* 0 1    0    0x100000000..0x17fffffff  0x100000000   2106-02-07..2174-02-25
* 1 0    1    0x180000000..0x1ffffffff  0x200000000   2174-02-25..2242-03-16
* 1 0    0    0x200000000..0x27fffffff  0x200000000   2242-03-16..2310-04-04
* 1 1    1    0x280000000..0x2ffffffff  0x300000000   2310-04-04..2378-04-22
* 1 1    0    0x300000000..0x37fffffff  0x300000000   2378-04-22..2446-05-10

Note that the time limits are not correct for deletion times.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Cc: tytso@mit.edu
Cc: adilger.kernel@dilger.ca
Cc: linux-ext4@vger.kernel.org
---
 fs/ext4/ext4.h  |  4 ++++
 fs/ext4/super.c | 17 +++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1cb67859e051..3f13cf12ae7f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1631,6 +1631,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	S32_MAX
+#define EXT4_TIMESTAMP_MIN		S32_MIN
+
 /*
  * Feature set definitions
  */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4079605d437a..3ea2d60f33aa 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4035,8 +4035,21 @@ 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)
-			sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
+		/*
+		 * i_atime_extra is the last extra field available for [acm]times in
+		 * struct ext4_inode. Checking for that field should suffice to ensure
+		 * we have extra space for all three.
+		 */
+		if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
+			sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
+			sb->s_time_gran = 1;
+			sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
+		} else {
+			sb->s_time_gran = NSEC_PER_SEC;
+			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.17.1


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

* [PATCH 10/20] fs: nfs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (8 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 09/20] ext4: Initialize timestamps limits Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 11/20] fs: cifs: " Deepa Dinamani
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, trond.myklebust, anna.schumaker, linux-nfs

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

The time formats for various verious is detailed in the
RFCs as below:

https://tools.ietf.org/html/rfc7862(time metadata)
https://tools.ietf.org/html/rfc7530:

nfstime4

   struct nfstime4 {
           int64_t         seconds;
           uint32_t        nseconds;
   };

https://tools.ietf.org/html/rfc1094

          struct timeval {
              unsigned int seconds;
              unsigned int useconds;
          };

https://tools.ietf.org/html/rfc1813

struct nfstime3 {
         uint32   seconds;
         uint32   nseconds;
      };

Use the limits as per the RFC.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: trond.myklebust@hammerspace.com
Cc: anna.schumaker@netapp.com
Cc: linux-nfs@vger.kernel.org
---
 fs/nfs/super.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index f88ddac2dcdf..54eb5a47f180 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2360,6 +2360,15 @@ void nfs_fill_super(struct super_block *sb, struct nfs_mount_info *mount_info)
 		sb->s_flags |= SB_POSIXACL;
 		sb->s_time_gran = 1;
 		sb->s_export_op = &nfs_export_ops;
+	} else
+		sb->s_time_gran = 1000;
+
+	if (server->nfs_client->rpc_ops->version != 4) {
+		sb->s_time_min = 0;
+		sb->s_time_max = U32_MAX;
+	} else {
+		sb->s_time_min = S64_MIN;
+		sb->s_time_max = S64_MAX;
 	}
 
  	nfs_initialise_sb(sb);
@@ -2380,7 +2389,6 @@ static void nfs_clone_super(struct super_block *sb,
 	sb->s_maxbytes = old_sb->s_maxbytes;
 	sb->s_xattr = old_sb->s_xattr;
 	sb->s_op = old_sb->s_op;
-	sb->s_time_gran = 1;
 	sb->s_export_op = old_sb->s_export_op;
 
 	if (server->nfs_client->rpc_ops->version != 2) {
@@ -2388,6 +2396,16 @@ static void nfs_clone_super(struct super_block *sb,
 		 * so ourselves when necessary.
 		 */
 		sb->s_flags |= SB_POSIXACL;
+		sb->s_time_gran = 1;
+	} else
+		sb->s_time_gran = 1000;
+
+	if (server->nfs_client->rpc_ops->version != 4) {
+		sb->s_time_min = 0;
+		sb->s_time_max = U32_MAX;
+	} else {
+		sb->s_time_min = S64_MIN;
+		sb->s_time_max = S64_MAX;
 	}
 
  	nfs_initialise_sb(sb);
-- 
2.17.1


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

* [PATCH 11/20] fs: cifs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (9 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 10/20] fs: nfs: Initialize filesystem timestamp ranges Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 12/20] fs: fat: " Deepa Dinamani
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, sfrench, linux-cifs

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Also fixed cnvrtDosUnixTm calculations to avoid int overflow
while computing maximum date.

References:

http://cifs.com/

https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-cifs/d416ff7c-c536-406e-a951-4f04b2fd1d2b

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: sfrench@samba.org
Cc: linux-cifs@vger.kernel.org
---
 fs/cifs/cifsfs.c  | 22 ++++++++++++++++++++++
 fs/cifs/netmisc.c | 14 +++++++-------
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 81a16b4e1b48..94a52a63b9ea 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -56,6 +56,15 @@
 #include "dfs_cache.h"
 #endif
 
+/*
+ * DOS dates from 1980/1/1 through 2107/12/31
+ * Protocol specifications indicate the range should be to 119, which
+ * limits maximum year to 2099. But this range has not been checked.
+ */
+#define SMB_DATE_MAX (127<<9 | 12<<5 | 31)
+#define SMB_DATE_MIN (0<<9 | 1<<5 | 1)
+#define SMB_TIME_MAX (23<<11 | 59<<5 | 29)
+
 int cifsFYI = 0;
 bool traceSMB;
 bool enable_oplocks = true;
@@ -142,6 +151,7 @@ cifs_read_super(struct super_block *sb)
 	struct inode *inode;
 	struct cifs_sb_info *cifs_sb;
 	struct cifs_tcon *tcon;
+	struct timespec64 ts;
 	int rc = 0;
 
 	cifs_sb = CIFS_SB(sb);
@@ -161,6 +171,18 @@ cifs_read_super(struct super_block *sb)
 	/* BB FIXME fix time_gran to be larger for LANMAN sessions */
 	sb->s_time_gran = 100;
 
+	if (tcon->unix_ext) {
+		ts = cifs_NTtimeToUnix(0);
+		sb->s_time_min = ts.tv_sec;
+		ts = cifs_NTtimeToUnix(cpu_to_le64(S64_MAX));
+		sb->s_time_max = ts.tv_sec;
+	} else {
+		ts = cnvrtDosUnixTm(cpu_to_le16(SMB_DATE_MIN), 0, 0);
+		sb->s_time_min = ts.tv_sec;
+		ts = cnvrtDosUnixTm(cpu_to_le16(SMB_DATE_MAX), cpu_to_le16(SMB_TIME_MAX), 0);
+		sb->s_time_max = ts.tv_sec;
+	}
+
 	sb->s_magic = CIFS_MAGIC_NUMBER;
 	sb->s_op = &cifs_super_ops;
 	sb->s_xattr = cifs_xattr_handlers;
diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
index ed92958e842d..49c17ee18254 100644
--- a/fs/cifs/netmisc.c
+++ b/fs/cifs/netmisc.c
@@ -949,8 +949,8 @@ static const int total_days_of_prev_months[] = {
 struct timespec64 cnvrtDosUnixTm(__le16 le_date, __le16 le_time, int offset)
 {
 	struct timespec64 ts;
-	time64_t sec;
-	int min, days, month, year;
+	time64_t sec, days;
+	int min, day, month, year;
 	u16 date = le16_to_cpu(le_date);
 	u16 time = le16_to_cpu(le_time);
 	SMB_TIME *st = (SMB_TIME *)&time;
@@ -966,15 +966,15 @@ struct timespec64 cnvrtDosUnixTm(__le16 le_date, __le16 le_time, int offset)
 	sec += 60 * 60 * st->Hours;
 	if (st->Hours > 24)
 		cifs_dbg(VFS, "illegal hours %d\n", st->Hours);
-	days = sd->Day;
+	day = sd->Day;
 	month = sd->Month;
-	if (days < 1 || days > 31 || month < 1 || month > 12) {
-		cifs_dbg(VFS, "illegal date, month %d day: %d\n", month, days);
-		days = clamp(days, 1, 31);
+	if (day < 1 || day > 31 || month < 1 || month > 12) {
+		cifs_dbg(VFS, "illegal date, month %d day: %d\n", month, day);
+		day = clamp(day, 1, 31);
 		month = clamp(month, 1, 12);
 	}
 	month -= 1;
-	days += total_days_of_prev_months[month];
+	days = day + total_days_of_prev_months[month];
 	days += 3652; /* account for difference in days between 1980 and 1970 */
 	year = sd->Year;
 	days += year * 365;
-- 
2.17.1


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

* [PATCH 12/20] fs: fat: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (10 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 11/20] fs: cifs: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  9:31   ` OGAWA Hirofumi
  2019-07-30  1:49 ` [PATCH 13/20] fs: affs: " Deepa Dinamani
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, hirofumi

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Some FAT variants indicate that the years after 2099 are not supported.
Since commit 7decd1cb0305 ("fat: Fix and cleanup timestamp conversion"),
we support the full range of years that can be represented, up to 2107.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: hirofumi@mail.parknet.co.jp
---
 fs/fat/inode.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 05689198f5af..5f04c5c810fb 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -31,6 +31,11 @@
 
 #define KB_IN_SECTORS 2
 
+/* DOS dates from 1980/1/1 through 2107/12/31 */
+#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
+#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
+#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
+
 /*
  * A deserialized copy of the on-disk structure laid out in struct
  * fat_boot_sector.
@@ -1605,6 +1610,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	int debug;
 	long error;
 	char buf[50];
+	struct timespec64 ts;
 
 	/*
 	 * GFP_KERNEL is ok here, because while we do hold the
@@ -1698,6 +1704,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	sbi->free_clus_valid = 0;
 	sbi->prev_free = FAT_START_ENT;
 	sb->s_maxbytes = 0xffffffff;
+	fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
+	sb->s_time_min = ts.tv_sec;
+
+	fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
+			  cpu_to_le16(FAT_DATE_MAX), 0);
+	sb->s_time_max = ts.tv_sec;
 
 	if (!sbi->fat_length && bpb.fat32_length) {
 		struct fat_boot_fsinfo *fsinfo;
-- 
2.17.1


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

* [PATCH 13/20] fs: affs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (11 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 12/20] fs: fat: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-08-01 11:28   ` David Sterba
  2019-07-30  1:49 ` [PATCH 14/20] fs: sysv: " Deepa Dinamani
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, dsterba

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Also fix timestamp calculation to avoid overflow
while converting from days to seconds.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: dsterba@suse.com
---
 fs/affs/amigaffs.c | 2 +-
 fs/affs/amigaffs.h | 3 +++
 fs/affs/inode.c    | 4 ++--
 fs/affs/super.c    | 4 ++++
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/affs/amigaffs.c b/fs/affs/amigaffs.c
index 14a6c1b90c9f..f708c45d5f66 100644
--- a/fs/affs/amigaffs.c
+++ b/fs/affs/amigaffs.c
@@ -375,7 +375,7 @@ affs_secs_to_datestamp(time64_t secs, struct affs_date *ds)
 	u32	 minute;
 	s32	 rem;
 
-	secs -= sys_tz.tz_minuteswest * 60 + ((8 * 365 + 2) * 24 * 60 * 60);
+	secs -= sys_tz.tz_minuteswest * 60 + AFFS_EPOCH_DELTA;
 	if (secs < 0)
 		secs = 0;
 	days    = div_s64_rem(secs, 86400, &rem);
diff --git a/fs/affs/amigaffs.h b/fs/affs/amigaffs.h
index f9bef9056659..81fb396d4dfa 100644
--- a/fs/affs/amigaffs.h
+++ b/fs/affs/amigaffs.h
@@ -32,6 +32,9 @@
 
 #define AFFS_ROOT_BMAPS		25
 
+/* Seconds since Amiga epoch of 1978/01/01 to UNIX */
+#define AFFS_EPOCH_DELTA ((8 * 365 + 2) * 86400LL)
+
 struct affs_date {
 	__be32 days;
 	__be32 mins;
diff --git a/fs/affs/inode.c b/fs/affs/inode.c
index 73598bff8506..a346cf7659f1 100644
--- a/fs/affs/inode.c
+++ b/fs/affs/inode.c
@@ -150,10 +150,10 @@ struct inode *affs_iget(struct super_block *sb, unsigned long ino)
 	}
 
 	inode->i_mtime.tv_sec = inode->i_atime.tv_sec = inode->i_ctime.tv_sec
-		       = (be32_to_cpu(tail->change.days) * (24 * 60 * 60) +
+		       = (be32_to_cpu(tail->change.days) * 86400LL +
 		         be32_to_cpu(tail->change.mins) * 60 +
 			 be32_to_cpu(tail->change.ticks) / 50 +
-			 ((8 * 365 + 2) * 24 * 60 * 60)) +
+			 AFFS_EPOCH_DELTA) +
 			 sys_tz.tz_minuteswest * 60;
 	inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec = inode->i_atime.tv_nsec = 0;
 	affs_brelse(bh);
diff --git a/fs/affs/super.c b/fs/affs/super.c
index e7d036efbaa1..cc463ae47c12 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -355,6 +355,10 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op                = &affs_sops;
 	sb->s_flags |= SB_NODIRATIME;
 
+	sb->s_time_gran = NSEC_PER_SEC;
+	sb->s_time_min = sys_tz.tz_minuteswest * 60 + AFFS_EPOCH_DELTA;
+	sb->s_time_max = 86400LL * U32_MAX + 86400 + sb->s_time_min;
+
 	sbi = kzalloc(sizeof(struct affs_sb_info), GFP_KERNEL);
 	if (!sbi)
 		return -ENOMEM;
-- 
2.17.1


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

* [PATCH 14/20] fs: sysv: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (12 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 13/20] fs: affs: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 15/20] fs: ceph: " Deepa Dinamani
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, hch

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: hch@infradead.org
---
 fs/sysv/super.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index d788b1daa7eb..cc8e2ed155c8 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -368,7 +368,8 @@ static int sysv_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_block_base = 0;
 	mutex_init(&sbi->s_lock);
 	sb->s_fs_info = sbi;
-
+	sb->s_time_min = 0;
+	sb->s_time_max = U32_MAX;
 	sb_set_blocksize(sb, BLOCK_SIZE);
 
 	for (i = 0; i < ARRAY_SIZE(flavours) && !size; i++) {
@@ -487,6 +488,8 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_type = FSTYPE_V7;
 	mutex_init(&sbi->s_lock);
 	sb->s_fs_info = sbi;
+	sb->s_time_min = 0;
+	sb->s_time_max = U32_MAX;
 	
 	sb_set_blocksize(sb, 512);
 
-- 
2.17.1


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

* [PATCH 15/20] fs: ceph: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (13 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 14/20] fs: sysv: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 16/20] fs: orangefs: " Deepa Dinamani
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, zyan, sage, idryomov, ceph-devel

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

According to the disscussion in
https://patchwork.kernel.org/patch/8308691/ we agreed to use
unsigned 32 bit timestamps on ceph.
Update the limits accordingly.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: zyan@redhat.com
Cc: sage@redhat.com
Cc: idryomov@gmail.com
Cc: ceph-devel@vger.kernel.org
---
 fs/ceph/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index d57fa60dcd43..6cf3a4fceac1 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -981,6 +981,8 @@ static int ceph_set_super(struct super_block *s, void *data)
 	s->s_export_op = &ceph_export_ops;
 
 	s->s_time_gran = 1000;  /* 1000 ns == 1 us */
+	s->s_time_min = 0;
+	s->s_time_max = U32_MAX;
 
 	ret = set_anon_super(s, NULL);  /* what is that second arg for? */
 	if (ret != 0)
-- 
2.17.1


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

* [PATCH 16/20] fs: orangefs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (14 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 15/20] fs: ceph: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 17/20] fs: hpfs: " Deepa Dinamani
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, hubcap, martin, devel

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Assume the limits as unsigned according to the below
commit 98e8eef557a9 ("changed PVFS_time from int64_t to uint64_t")
in https://github.com/waltligon/orangefs

Author: Neill Miller <neillm@mcs.anl.gov>
Date:   Thu Sep 2 15:00:38 2004 +0000

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: hubcap@omnibond.com
Cc: martin@omnibond.com
Cc: devel@lists.orangefs.org
---
 fs/orangefs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index ee5efdc35cc1..dcd97e8158b1 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -439,6 +439,8 @@ static int orangefs_fill_sb(struct super_block *sb,
 	sb->s_blocksize = PAGE_SIZE;
 	sb->s_blocksize_bits = PAGE_SHIFT;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	sb->s_time_min = 0;
+	sb->s_time_max = S64_MAX;
 
 	ret = super_setup_bdi(sb);
 	if (ret)
-- 
2.17.1


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

* [PATCH 17/20] fs: hpfs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (15 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 16/20] fs: orangefs: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 18/20] fs: omfs: " Deepa Dinamani
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, mikulas

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Also change the local_to_gmt() to use time64_t instead
of time32_t.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: mikulas@artax.karlin.mff.cuni.cz
---
 fs/hpfs/hpfs_fn.h | 6 ++----
 fs/hpfs/super.c   | 2 ++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/hpfs/hpfs_fn.h b/fs/hpfs/hpfs_fn.h
index ab2e7cc2ff33..1cca83218fb5 100644
--- a/fs/hpfs/hpfs_fn.h
+++ b/fs/hpfs/hpfs_fn.h
@@ -334,7 +334,7 @@ long hpfs_ioctl(struct file *file, unsigned cmd, unsigned long arg);
  * local time (HPFS) to GMT (Unix)
  */
 
-static inline time64_t local_to_gmt(struct super_block *s, time32_t t)
+static inline time64_t local_to_gmt(struct super_block *s, time64_t t)
 {
 	extern struct timezone sys_tz;
 	return t + sys_tz.tz_minuteswest * 60 + hpfs_sb(s)->sb_timeshift;
@@ -343,9 +343,7 @@ static inline time64_t local_to_gmt(struct super_block *s, time32_t t)
 static inline time32_t gmt_to_local(struct super_block *s, time64_t t)
 {
 	extern struct timezone sys_tz;
-	t = t - sys_tz.tz_minuteswest * 60 - hpfs_sb(s)->sb_timeshift;
-
-	return clamp_t(time64_t, t, 0, U32_MAX);
+	return t - sys_tz.tz_minuteswest * 60 - hpfs_sb(s)->sb_timeshift;
 }
 
 static inline time32_t local_get_seconds(struct super_block *s)
diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
index 9db6d84f0d62..0a677a9aaf34 100644
--- a/fs/hpfs/super.c
+++ b/fs/hpfs/super.c
@@ -614,6 +614,8 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 	s->s_magic = HPFS_SUPER_MAGIC;
 	s->s_op = &hpfs_sops;
 	s->s_d_op = &hpfs_dentry_operations;
+	s->s_time_min =  local_to_gmt(s, 0);
+	s->s_time_max =  local_to_gmt(s, U32_MAX);
 
 	sbi->sb_root = le32_to_cpu(superblock->root);
 	sbi->sb_fs_size = le32_to_cpu(superblock->n_sectors);
-- 
2.17.1


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

* [PATCH 18/20] fs: omfs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (16 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 17/20] fs: hpfs: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30 14:25   ` Bob Copeland
  2019-07-30  1:49 ` [PATCH 19/20] pstore: fs superblock limits Deepa Dinamani
  2019-07-30  1:49 ` [PATCH 20/20] isofs: Initialize filesystem timestamp ranges Deepa Dinamani
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038, me, linux-karma-devel

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: me@bobcopeland.com
Cc: linux-karma-devel@lists.sourceforge.net
---
 fs/omfs/inode.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c
index 08226a835ec3..b76ec6b88ded 100644
--- a/fs/omfs/inode.c
+++ b/fs/omfs/inode.c
@@ -478,6 +478,10 @@ static int omfs_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_maxbytes = 0xffffffff;
 
+	sb->s_time_gran = NSEC_PER_MSEC;
+	sb->s_time_min = 0;
+	sb->s_time_max = U64_MAX / MSEC_PER_SEC;
+
 	sb_set_blocksize(sb, 0x200);
 
 	bh = sb_bread(sb, 0);
-- 
2.17.1


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

* [PATCH 19/20] pstore: fs superblock limits
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (17 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 18/20] fs: omfs: " Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  2019-07-30  4:31   ` Kees Cook
  2019-07-30  1:49 ` [PATCH 20/20] isofs: Initialize filesystem timestamp ranges Deepa Dinamani
  19 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel
  Cc: linux-fsdevel, arnd, y2038, anton, ccross, keescook, tony.luck

Also update the gran since pstore has microsecond granularity.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: anton@enomsg.org
Cc: ccross@android.com
Cc: keescook@chromium.org
Cc: tony.luck@intel.com
---
 fs/pstore/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 89a80b568a17..ee752f9fda57 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -388,7 +388,9 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_blocksize_bits	= PAGE_SHIFT;
 	sb->s_magic		= PSTOREFS_MAGIC;
 	sb->s_op		= &pstore_ops;
-	sb->s_time_gran		= 1;
+	sb->s_time_gran         = NSEC_PER_USEC;
+	sb->s_time_min		= S64_MIN;
+	sb->s_time_max		= S64_MAX;
 
 	parse_options(data);
 
-- 
2.17.1


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

* [PATCH 20/20] isofs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
                   ` (18 preceding siblings ...)
  2019-07-30  1:49 ` [PATCH 19/20] pstore: fs superblock limits Deepa Dinamani
@ 2019-07-30  1:49 ` Deepa Dinamani
  19 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30  1:49 UTC (permalink / raw)
  To: viro, linux-kernel; +Cc: linux-fsdevel, arnd, y2038

Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Reference: http://www.ecma-international.org/publications/standards/Ecma-119.htm

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

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 9e30d8703735..62c0462dc89f 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -30,6 +30,9 @@
 #include "isofs.h"
 #include "zisofs.h"
 
+/* max tz offset is 13 hours */
+#define MAX_TZ_OFFSET (52*15*60)
+
 #define BEQUIET
 
 static int isofs_hashi(const struct dentry *parent, struct qstr *qstr);
@@ -801,6 +804,10 @@ static int isofs_fill_super(struct super_block *s, void *data, int silent)
 	 */
 	s->s_maxbytes = 0x80000000000LL;
 
+	/* ECMA-119 timestamp from 1900/1/1 with tz offset */
+	s->s_time_min = mktime64(1900, 1, 1, 0, 0, 0) - MAX_TZ_OFFSET;
+	s->s_time_max = mktime64(U8_MAX+1900, 12, 31, 23, 59, 59) + MAX_TZ_OFFSET;
+
 	/* Set this for reference. Its not currently used except on write
 	   which we don't have .. */
 
-- 
2.17.1


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

* Re: [PATCH 19/20] pstore: fs superblock limits
  2019-07-30  1:49 ` [PATCH 19/20] pstore: fs superblock limits Deepa Dinamani
@ 2019-07-30  4:31   ` Kees Cook
  2019-07-30  7:36     ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Kees Cook @ 2019-07-30  4:31 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038, anton, ccross, tony.luck

On Mon, Jul 29, 2019 at 06:49:23PM -0700, Deepa Dinamani wrote:
> Also update the gran since pstore has microsecond granularity.

So, I'm fine with this, but technically the granularity depends on the
backend storage... many have no actual time keeping, though. My point is,
pstore's timestamps are really mostly a lie, but the most common backend
(ramoops) is seconds-granularity.

So, I'm fine with this, but it's a lie but it's a lie that doesn't
matter, so ...

Acked-by: Kees Cook <keescook@chromium.org>

I'm open to suggestions to improve it...

-Kees

> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: anton@enomsg.org
> Cc: ccross@android.com
> Cc: keescook@chromium.org
> Cc: tony.luck@intel.com
> ---
>  fs/pstore/inode.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 89a80b568a17..ee752f9fda57 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -388,7 +388,9 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
>  	sb->s_blocksize_bits	= PAGE_SHIFT;
>  	sb->s_magic		= PSTOREFS_MAGIC;
>  	sb->s_op		= &pstore_ops;
> -	sb->s_time_gran		= 1;
> +	sb->s_time_gran         = NSEC_PER_USEC;
> +	sb->s_time_min		= S64_MIN;
> +	sb->s_time_max		= S64_MAX;
>  
>  	parse_options(data);
>  
> -- 
> 2.17.1
> 

-- 
Kees Cook

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

* Re: [PATCH 19/20] pstore: fs superblock limits
  2019-07-30  4:31   ` Kees Cook
@ 2019-07-30  7:36     ` Arnd Bergmann
  2019-08-02  2:26       ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-07-30  7:36 UTC (permalink / raw)
  To: Kees Cook
  Cc: Deepa Dinamani, Al Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Anton Vorontsov,
	Colin Cross, Tony Luck

On Tue, Jul 30, 2019 at 6:31 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jul 29, 2019 at 06:49:23PM -0700, Deepa Dinamani wrote:
> > Also update the gran since pstore has microsecond granularity.
>
> So, I'm fine with this, but technically the granularity depends on the
> backend storage... many have no actual time keeping, though. My point is,
> pstore's timestamps are really mostly a lie, but the most common backend
> (ramoops) is seconds-granularity.
>
> So, I'm fine with this, but it's a lie but it's a lie that doesn't
> matter, so ...
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> I'm open to suggestions to improve it...

If we don't care about using sub-second granularity, then setting it
to one second unconditionally here will make it always use that and
report it correctly.

       Arnd

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

* Re: [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc
  2019-07-30  1:49 ` [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc Deepa Dinamani
@ 2019-07-30  8:27   ` OGAWA Hirofumi
  2019-07-30 17:26     ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: OGAWA Hirofumi @ 2019-07-30  8:27 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038, adrian.hunter,
	anton, dedekind1, gregkh, hch, jaegeuk, jlbec, richard, tj,
	yuchao0, linux-f2fs-devel, linux-ntfs-dev, linux-mtd

Deepa Dinamani <deepa.kernel@gmail.com> writes:

> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 1e08bd54c5fb..53bb7c6bf993 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -307,8 +307,9 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
>  		inode->i_atime = (struct timespec64){ seconds, 0 };
>  	}
>  	if (flags & S_CTIME) {
> -		if (sbi->options.isvfat)
> -			inode->i_ctime = timespec64_trunc(*now, 10000000);
> +		if (sbi->options.isvfat) {
> +			inode->i_ctime = timestamp_truncate(*now, inode);
> +		}
>  		else
>  			inode->i_ctime = fat_timespec64_trunc_2secs(*now);
>  	}

Looks like broken. It changed to sb->s_time_gran from 10000000, and
changed coding style.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

* Re: [PATCH 12/20] fs: fat: Initialize filesystem timestamp ranges
  2019-07-30  1:49 ` [PATCH 12/20] fs: fat: " Deepa Dinamani
@ 2019-07-30  9:31   ` OGAWA Hirofumi
  2019-07-30 17:39     ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: OGAWA Hirofumi @ 2019-07-30  9:31 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038

Deepa Dinamani <deepa.kernel@gmail.com> writes:

> +/* DOS dates from 1980/1/1 through 2107/12/31 */
> +#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
> +#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
> +#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
> +
>  /*
>   * A deserialized copy of the on-disk structure laid out in struct
>   * fat_boot_sector.
> @@ -1605,6 +1610,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>  	int debug;
>  	long error;
>  	char buf[50];
> +	struct timespec64 ts;
>  
>  	/*
>  	 * GFP_KERNEL is ok here, because while we do hold the
> @@ -1698,6 +1704,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>  	sbi->free_clus_valid = 0;
>  	sbi->prev_free = FAT_START_ENT;
>  	sb->s_maxbytes = 0xffffffff;
> +	fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
> +	sb->s_time_min = ts.tv_sec;
> +
> +	fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
> +			  cpu_to_le16(FAT_DATE_MAX), 0);
> +	sb->s_time_max = ts.tv_sec;

At least, it is wrong to call fat_time_fat2unix() before setup parameters
in sbi.

And please move those timestamp stuff to fat/misc.c like other fat
timestamp helpers. (Maybe, provide fat_time_{min,max}() from fat/misc.c,
or fat_init_time() such?).

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

* Re: [PATCH 18/20] fs: omfs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 ` [PATCH 18/20] fs: omfs: " Deepa Dinamani
@ 2019-07-30 14:25   ` Bob Copeland
  0 siblings, 0 replies; 62+ messages in thread
From: Bob Copeland @ 2019-07-30 14:25 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038, linux-karma-devel

On Mon, Jul 29, 2019 at 06:49:22PM -0700, Deepa Dinamani wrote:
> Fill in the appropriate limits to avoid inconsistencies
> in the vfs cached inode times when timestamps are
> outside the permitted range.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: me@bobcopeland.com
> Cc: linux-karma-devel@lists.sourceforge.net
> ---
>  fs/omfs/inode.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c
> index 08226a835ec3..b76ec6b88ded 100644
> --- a/fs/omfs/inode.c
> +++ b/fs/omfs/inode.c
> @@ -478,6 +478,10 @@ static int omfs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_maxbytes = 0xffffffff;
>  
> +	sb->s_time_gran = NSEC_PER_MSEC;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = U64_MAX / MSEC_PER_SEC;
> +

I honestly don't know if it should be s64 rather than u64, but considering
that none of the devices with this filesystem ever exposed dates to users in
the negative era, it should be fine.

Acked-by: Bob Copeland <me@bobcopeland.com>

-- 
Bob Copeland %% https://bobcopeland.com/

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

* Re: [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc
  2019-07-30  8:27   ` OGAWA Hirofumi
@ 2019-07-30 17:26     ` Deepa Dinamani
  2019-07-30 22:28       ` Anton Altaparmakov
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30 17:26 UTC (permalink / raw)
  To: OGAWA Hirofumi
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Adrian Hunter, anton, Artem Bityutskiy, Greg KH, stoph Hellwig,
	Jaegeuk Kim, Joel Becker, Richard Weinberger, Tejun Heo, yuchao0,
	Linux F2FS DEV, Mailing List, linux-ntfs-dev, linux-mtd

On Tue, Jul 30, 2019 at 1:27 AM OGAWA Hirofumi
<hirofumi@mail.parknet.co.jp> wrote:
>
> Deepa Dinamani <deepa.kernel@gmail.com> writes:
>
> > diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> > index 1e08bd54c5fb..53bb7c6bf993 100644
> > --- a/fs/fat/misc.c
> > +++ b/fs/fat/misc.c
> > @@ -307,8 +307,9 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
> >               inode->i_atime = (struct timespec64){ seconds, 0 };
> >       }
> >       if (flags & S_CTIME) {
> > -             if (sbi->options.isvfat)
> > -                     inode->i_ctime = timespec64_trunc(*now, 10000000);
> > +             if (sbi->options.isvfat) {
> > +                     inode->i_ctime = timestamp_truncate(*now, inode);
> > +             }
> >               else
> >                       inode->i_ctime = fat_timespec64_trunc_2secs(*now);
> >       }
>
> Looks like broken. It changed to sb->s_time_gran from 10000000, and
> changed coding style.

This is using a new api: timestamp_truncate(). granularity is gotten
by inode->sb->s_time_gran. See Patch [2/20]:
https://lkml.org/lkml/2019/7/29/1853

So this is not broken if fat is filling in the right granularity in the sb.

-Deepa

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

* Re: [PATCH 12/20] fs: fat: Initialize filesystem timestamp ranges
  2019-07-30  9:31   ` OGAWA Hirofumi
@ 2019-07-30 17:39     ` Deepa Dinamani
  2019-07-31  0:48       ` OGAWA Hirofumi
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-30 17:39 UTC (permalink / raw)
  To: OGAWA Hirofumi
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List

On Tue, Jul 30, 2019 at 2:31 AM OGAWA Hirofumi
<hirofumi@mail.parknet.co.jp> wrote:
>
> Deepa Dinamani <deepa.kernel@gmail.com> writes:
>
> > +/* DOS dates from 1980/1/1 through 2107/12/31 */
> > +#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
> > +#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
> > +#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
> > +
> >  /*
> >   * A deserialized copy of the on-disk structure laid out in struct
> >   * fat_boot_sector.
> > @@ -1605,6 +1610,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
> >       int debug;
> >       long error;
> >       char buf[50];
> > +     struct timespec64 ts;
> >
> >       /*
> >        * GFP_KERNEL is ok here, because while we do hold the
> > @@ -1698,6 +1704,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
> >       sbi->free_clus_valid = 0;
> >       sbi->prev_free = FAT_START_ENT;
> >       sb->s_maxbytes = 0xffffffff;
> > +     fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
> > +     sb->s_time_min = ts.tv_sec;
> > +
> > +     fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
> > +                       cpu_to_le16(FAT_DATE_MAX), 0);
> > +     sb->s_time_max = ts.tv_sec;
>
> At least, it is wrong to call fat_time_fat2unix() before setup parameters
> in sbi.

All the parameters that fat_time_fat2unix() cares in sbi is accessed through

static inline int fat_tz_offset(struct msdos_sb_info *sbi)
{
    return (sbi->options.tz_set ?
           -sbi->options.time_offset :
           sys_tz.tz_minuteswest) * SECS_PER_MIN;
}

Both the sbi fields sbi->options.tz_set and sbi->options.time_offset
are set by the call to parse_options(). And, parse_options() is called
before the calls to fat_time_fat2unix().:

int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
           void (*setup)(struct super_block *))
{
     <snip>

    error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
    if (error)
        goto out_fail;

   <snip>

    sbi->prev_free = FAT_START_ENT;
    sb->s_maxbytes = 0xffffffff;
    fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
    sb->s_time_min = ts.tv_sec;

    fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
              cpu_to_le16(FAT_DATE_MAX), 0);
    sb->s_time_max = ts.tv_sec;

   <snip>
}

I do not see what the problem is.

-Deepa

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

* Re: [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc
  2019-07-30 17:26     ` Deepa Dinamani
@ 2019-07-30 22:28       ` Anton Altaparmakov
  2019-07-31  0:08         ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Anton Altaparmakov @ 2019-07-30 22:28 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: OGAWA Hirofumi, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Adrian Hunter, Artem Bityutskiy, Greg KH, stoph Hellwig,
	Jaegeuk Kim, Joel Becker, Richard Weinberger, Tejun Heo, yuchao0,
	Linux F2FS DEV, Mailing List, linux-ntfs-dev, linux-mtd

Hi Deepa,

> On 30 Jul 2019, at 18:26, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> 
> On Tue, Jul 30, 2019 at 1:27 AM OGAWA Hirofumi
> <hirofumi@mail.parknet.co.jp> wrote:
>> 
>> Deepa Dinamani <deepa.kernel@gmail.com> writes:
>> 
>>> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
>>> index 1e08bd54c5fb..53bb7c6bf993 100644
>>> --- a/fs/fat/misc.c
>>> +++ b/fs/fat/misc.c
>>> @@ -307,8 +307,9 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
>>>              inode->i_atime = (struct timespec64){ seconds, 0 };
>>>      }
>>>      if (flags & S_CTIME) {
>>> -             if (sbi->options.isvfat)
>>> -                     inode->i_ctime = timespec64_trunc(*now, 10000000);
>>> +             if (sbi->options.isvfat) {
>>> +                     inode->i_ctime = timestamp_truncate(*now, inode);
>>> +             }
>>>              else
>>>                      inode->i_ctime = fat_timespec64_trunc_2secs(*now);
>>>      }
>> 
>> Looks like broken. It changed to sb->s_time_gran from 10000000, and
>> changed coding style.
> 
> This is using a new api: timestamp_truncate(). granularity is gotten
> by inode->sb->s_time_gran. See Patch [2/20]:
> https://lkml.org/lkml/2019/7/29/1853
> 
> So this is not broken if fat is filling in the right granularity in the sb.

It is broken for FAT because FAT has different granularities for different timestamps so it cannot put the correct value in the sb as that only allows one granularity.  Your patch is totally broken for fat as it would be immediately obvious if you spent a few minutes looking at the code...

Best regards,

	Anton

> 
> -Deepa


-- 
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer


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

* Re: [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc
  2019-07-30 22:28       ` Anton Altaparmakov
@ 2019-07-31  0:08         ` Deepa Dinamani
  0 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-31  0:08 UTC (permalink / raw)
  To: Anton Altaparmakov
  Cc: OGAWA Hirofumi, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Adrian Hunter, Artem Bityutskiy, Greg KH, stoph Hellwig,
	Jaegeuk Kim, Joel Becker, Richard Weinberger, Tejun Heo, yuchao0,
	Linux F2FS DEV, Mailing List, linux-ntfs-dev, linux-mtd

On Tue, Jul 30, 2019 at 3:28 PM Anton Altaparmakov <anton@tuxera.com> wrote:
>
> Hi Deepa,
>
> > On 30 Jul 2019, at 18:26, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > On Tue, Jul 30, 2019 at 1:27 AM OGAWA Hirofumi
> > <hirofumi@mail.parknet.co.jp> wrote:
> >>
> >> Deepa Dinamani <deepa.kernel@gmail.com> writes:
> >>
> >>> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> >>> index 1e08bd54c5fb..53bb7c6bf993 100644
> >>> --- a/fs/fat/misc.c
> >>> +++ b/fs/fat/misc.c
> >>> @@ -307,8 +307,9 @@ int fat_truncate_time(struct inode *inode, struct timespec64 *now, int flags)
> >>>              inode->i_atime = (struct timespec64){ seconds, 0 };
> >>>      }
> >>>      if (flags & S_CTIME) {
> >>> -             if (sbi->options.isvfat)
> >>> -                     inode->i_ctime = timespec64_trunc(*now, 10000000);
> >>> +             if (sbi->options.isvfat) {
> >>> +                     inode->i_ctime = timestamp_truncate(*now, inode);
> >>> +             }
> >>>              else
> >>>                      inode->i_ctime = fat_timespec64_trunc_2secs(*now);
> >>>      }
> >>
> >> Looks like broken. It changed to sb->s_time_gran from 10000000, and
> >> changed coding style.
> >
> > This is using a new api: timestamp_truncate(). granularity is gotten
> > by inode->sb->s_time_gran. See Patch [2/20]:
> > https://lkml.org/lkml/2019/7/29/1853
> >
> > So this is not broken if fat is filling in the right granularity in the sb.
>
> It is broken for FAT because FAT has different granularities for different timestamps so it cannot put the correct value in the sb as that only allows one granularity.  Your patch is totally broken for fat as it would be immediately obvious if you spent a few minutes looking at the code...

It seemed to me that FAT had already covered the special cases (2s and
1d) granularities by using internal functions. This one could also be
an inlined calculation, but I will just drop the FAT part from this
patch and leave it as is for now.

Thanks,
Deepa

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

* Re: [PATCH 12/20] fs: fat: Initialize filesystem timestamp ranges
  2019-07-30 17:39     ` Deepa Dinamani
@ 2019-07-31  0:48       ` OGAWA Hirofumi
  0 siblings, 0 replies; 62+ messages in thread
From: OGAWA Hirofumi @ 2019-07-31  0:48 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List

Deepa Dinamani <deepa.kernel@gmail.com> writes:

>> At least, it is wrong to call fat_time_fat2unix() before setup parameters
>> in sbi.
>
> All the parameters that fat_time_fat2unix() cares in sbi is accessed through
>
> static inline int fat_tz_offset(struct msdos_sb_info *sbi)
> {
>     return (sbi->options.tz_set ?
>            -sbi->options.time_offset :
>            sys_tz.tz_minuteswest) * SECS_PER_MIN;
> }
>
> Both the sbi fields sbi->options.tz_set and sbi->options.time_offset
> are set by the call to parse_options(). And, parse_options() is called
> before the calls to fat_time_fat2unix().:
>
> int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>            void (*setup)(struct super_block *))
> {
>      <snip>
>
>     error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
>     if (error)
>         goto out_fail;
>
>    <snip>
>
>     sbi->prev_free = FAT_START_ENT;
>     sb->s_maxbytes = 0xffffffff;
>     fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
>     sb->s_time_min = ts.tv_sec;
>
>     fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
>               cpu_to_le16(FAT_DATE_MAX), 0);
>     sb->s_time_max = ts.tv_sec;
>
>    <snip>
> }
>
> I do not see what the problem is.

Ouch, you are right. I was reading that patch wrongly, sorry.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

* Re: [PATCH 05/20] utimes: Clamp the timestamps before update
  2019-07-30  1:49 ` [PATCH 05/20] utimes: Clamp the timestamps before update Deepa Dinamani
@ 2019-07-31 15:14   ` Darrick J. Wong
  2019-07-31 15:33     ` Deepa Dinamani
  2019-08-05 13:30   ` [Y2038] " Ben Hutchings
  1 sibling, 1 reply; 62+ messages in thread
From: Darrick J. Wong @ 2019-07-31 15:14 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038

On Mon, Jul 29, 2019 at 06:49:09PM -0700, Deepa Dinamani wrote:
> POSIX is ambiguous on the behavior of timestamps for
> futimens, utimensat and utimes. Whether to return an
> error or silently clamp a timestamp beyond the range
> supported by the underlying filesystems is not clear.
> 
> POSIX.1 section for futimens, utimensat and utimes says:
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html)
> 
> 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.
> 
> If the tv_nsec field of a timespec structure has the special
> value UTIME_NOW, the file's relevant timestamp shall be set
> to the greatest value supported by the file system that is
> not greater than the current time.
> 
> [EINVAL]
>     A new file timestamp would be a value whose tv_sec
>     component is not a value supported by the file system.
> 
> The patch chooses to clamp the timestamps according to the
> filesystem timestamp ranges and does not return an error.
> This is in line with the behavior of utime syscall also
> since the POSIX page(http://pubs.opengroup.org/onlinepubs/009695399/functions/utime.html)
> for utime does not mention returning an error or clamping like above.
> 
> Same for utimes http://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html
> 
> 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 350c9c16ace1..4c1a2ce90bbc 100644
> --- a/fs/utimes.c
> +++ b/fs/utimes.c
> @@ -21,6 +21,7 @@ static int utimes_common(const struct path *path, struct timespec64 *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);
> @@ -36,16 +37,24 @@ static int utimes_common(const struct path *path, struct timespec64 *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(times[0].tv_sec, sb->s_time_min, sb->s_time_max);
> +			if (times[0].tv_sec == sb->s_time_max || times[0].tv_sec == sb->s_time_min)
> +				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(times[1].tv_sec, sb->s_time_min, sb->s_time_max);
> +			if (times[1].tv_sec >= sb->s_time_max || times[1].tv_sec == sb->s_time_min)

Line length.

Also, didn't you just introduce a function to clamp tv_sec and fix
granularity?  Why not just use it here?  I think this is the third time
I've seen this open-coded logic.

--D

> +				newattrs.ia_mtime.tv_nsec = 0;
> +			else
> +				newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
>  			newattrs.ia_valid |= ATTR_MTIME_SET;
>  		}
>  		/*
> -- 
> 2.17.1
> 

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-07-30  1:49 ` [PATCH 09/20] ext4: Initialize timestamps limits Deepa Dinamani
@ 2019-07-31 15:26   ` Darrick J. Wong
  2019-08-01 19:18     ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Darrick J. Wong @ 2019-07-31 15:26 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038, tytso,
	adilger.kernel, linux-ext4

On Mon, Jul 29, 2019 at 06:49:13PM -0700, Deepa Dinamani wrote:
> ext4 has different overflow limits for max filesystem
> timestamps based on the extra bytes available.
> 
> The timestamp limits are calculated according to the
> encoding table in
> a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
> 
> * extra  msb of                         adjust for signed
> * epoch  32-bit                         32-bit tv_sec to
> * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> * 0 0    1    -0x80000000..-0x00000001  0x000000000   1901-12-13..1969-12-31
> * 0 0    0    0x000000000..0x07fffffff  0x000000000   1970-01-01..2038-01-19
> * 0 1    1    0x080000000..0x0ffffffff  0x100000000   2038-01-19..2106-02-07
> * 0 1    0    0x100000000..0x17fffffff  0x100000000   2106-02-07..2174-02-25
> * 1 0    1    0x180000000..0x1ffffffff  0x200000000   2174-02-25..2242-03-16
> * 1 0    0    0x200000000..0x27fffffff  0x200000000   2242-03-16..2310-04-04
> * 1 1    1    0x280000000..0x2ffffffff  0x300000000   2310-04-04..2378-04-22
> * 1 1    0    0x300000000..0x37fffffff  0x300000000   2378-04-22..2446-05-10

My recollection of ext4 has gotten rusty, so this could be a bogus
question:

Say you have a filesystem with s_inode_size > 128 where not all of the
ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
don't support nanoseconds or times beyond 2038.  I think this happens on
ext3 filesystems that reserved extra space for inode attrs that are
subsequently converted to ext4?

In any case, that means that you have some inodes that support 34-bit
tv_sec and some inodes that only support 32-bit tv_sec.  For the inodes
with 32-bit tv_sec, I think all that happens is that a large timestamp
will be truncated further, right?

And no mount time warning because at least /some/ of the inodes are
ready to go for the next 30 years?

--D

> Note that the time limits are not correct for deletion times.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Cc: tytso@mit.edu
> Cc: adilger.kernel@dilger.ca
> Cc: linux-ext4@vger.kernel.org
> ---
>  fs/ext4/ext4.h  |  4 ++++
>  fs/ext4/super.c | 17 +++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..3f13cf12ae7f 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1631,6 +1631,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	S32_MAX
> +#define EXT4_TIMESTAMP_MIN		S32_MIN
> +
>  /*
>   * Feature set definitions
>   */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 4079605d437a..3ea2d60f33aa 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4035,8 +4035,21 @@ 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)
> -			sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
> +		/*
> +		 * i_atime_extra is the last extra field available for [acm]times in
> +		 * struct ext4_inode. Checking for that field should suffice to ensure
> +		 * we have extra space for all three.
> +		 */
> +		if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
> +			sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
> +			sb->s_time_gran = 1;
> +			sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
> +		} else {
> +			sb->s_time_gran = NSEC_PER_SEC;
> +			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.17.1
> 

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

* Re: [PATCH 06/20] fs: Fill in max and min timestamps in superblock
  2019-07-30  1:49 ` [PATCH 06/20] fs: Fill in max and min timestamps in superblock Deepa Dinamani
@ 2019-07-31 15:28   ` Darrick J. Wong
  0 siblings, 0 replies; 62+ messages in thread
From: Darrick J. Wong @ 2019-07-31 15:28 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: viro, linux-kernel, linux-fsdevel, arnd, y2038, aivazian.tigran,
	al, coda, dushistov, dwmw2, hch, jack, jaharkes, luisbg, nico,
	phillip, richard, salah.triki, shaggy, linux-xfs, codalist,
	linux-ext4, linux-mtd, jfs-discussion, reiserfs-devel

On Mon, Jul 29, 2019 at 06:49:10PM -0700, Deepa Dinamani wrote:
> Fill in the appropriate limits to avoid inconsistencies
> in the vfs cached inode times when timestamps are
> outside the permitted range.
> 
> Even though some filesystems are read-only, fill in the
> timestamps to reflect the on-disk representation.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: aivazian.tigran@gmail.com
> Cc: al@alarsen.net
> Cc: coda@cs.cmu.edu
> Cc: darrick.wong@oracle.com
> Cc: dushistov@mail.ru
> Cc: dwmw2@infradead.org
> Cc: hch@infradead.org
> Cc: jack@suse.com
> Cc: jaharkes@cs.cmu.edu
> Cc: luisbg@kernel.org
> Cc: nico@fluxnic.net
> Cc: phillip@squashfs.org.uk
> Cc: richard@nod.at
> Cc: salah.triki@gmail.com
> Cc: shaggy@kernel.org
> Cc: linux-xfs@vger.kernel.org
> Cc: codalist@coda.cs.cmu.edu
> Cc: linux-ext4@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> Cc: jfs-discussion@lists.sourceforge.net
> Cc: reiserfs-devel@vger.kernel.org
> ---
>  fs/befs/linuxvfs.c       | 2 ++
>  fs/bfs/inode.c           | 2 ++
>  fs/coda/inode.c          | 3 +++
>  fs/cramfs/inode.c        | 2 ++
>  fs/efs/super.c           | 2 ++
>  fs/ext2/super.c          | 2 ++
>  fs/freevxfs/vxfs_super.c | 2 ++
>  fs/jffs2/fs.c            | 3 +++
>  fs/jfs/super.c           | 2 ++
>  fs/minix/inode.c         | 2 ++
>  fs/qnx4/inode.c          | 2 ++
>  fs/qnx6/inode.c          | 2 ++
>  fs/reiserfs/super.c      | 3 +++
>  fs/romfs/super.c         | 2 ++
>  fs/squashfs/super.c      | 2 ++
>  fs/ufs/super.c           | 7 +++++++
>  fs/xfs/xfs_super.c       | 2 ++
>  17 files changed, 42 insertions(+)
> 
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 462d096ff3e9..64cdf4d8e424 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -893,6 +893,8 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  	sb_set_blocksize(sb, (ulong) befs_sb->block_size);
>  	sb->s_op = &befs_sops;
>  	sb->s_export_op = &befs_export_operations;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = 0xffffffffffffll;
>  	root = befs_iget(sb, iaddr2blockno(sb, &(befs_sb->root_dir)));
>  	if (IS_ERR(root)) {
>  		ret = PTR_ERR(root);
> diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
> index 5e97bed073d7..f8ce1368218b 100644
> --- a/fs/bfs/inode.c
> +++ b/fs/bfs/inode.c
> @@ -324,6 +324,8 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
>  		return -ENOMEM;
>  	mutex_init(&info->bfs_lock);
>  	s->s_fs_info = info;
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
>  
>  	sb_set_blocksize(s, BFS_BSIZE);
>  
> diff --git a/fs/coda/inode.c b/fs/coda/inode.c
> index 321f56e487cb..b1c70e2b9b1e 100644
> --- a/fs/coda/inode.c
> +++ b/fs/coda/inode.c
> @@ -188,6 +188,9 @@ static int coda_fill_super(struct super_block *sb, void *data, int silent)
>  	sb->s_magic = CODA_SUPER_MAGIC;
>  	sb->s_op = &coda_super_operations;
>  	sb->s_d_op = &coda_dentry_operations;
> +	sb->s_time_gran = 1;
> +	sb->s_time_min = S64_MIN;
> +	sb->s_time_max = S64_MAX;
>  
>  	error = super_setup_bdi(sb);
>  	if (error)
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 9352487bd0fc..4d1d8b7761ed 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -597,6 +597,8 @@ static int cramfs_finalize_super(struct super_block *sb,
>  
>  	/* Set it all up.. */
>  	sb->s_flags |= SB_RDONLY;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = 0;
>  	sb->s_op = &cramfs_ops;
>  	root = get_cramfs_inode(sb, cramfs_root, 0);
>  	if (IS_ERR(root))
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index 867fc24dee20..4a6ebff2af76 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -257,6 +257,8 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
>  	if (!sb)
>  		return -ENOMEM;
>  	s->s_fs_info = sb;
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
>   
>  	s->s_magic		= EFS_SUPER_MAGIC;
>  	if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) {
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 44eb6e7eb492..baa36c6fb71e 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1002,6 +1002,8 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_maxbytes = ext2_max_size(sb->s_blocksize_bits);
>  	sb->s_max_links = EXT2_LINK_MAX;
> +	sb->s_time_min = S32_MIN;
> +	sb->s_time_max = S32_MAX;
>  
>  	if (le32_to_cpu(es->s_rev_level) == EXT2_GOOD_OLD_REV) {
>  		sbi->s_inode_size = EXT2_GOOD_OLD_INODE_SIZE;
> diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
> index a89f68c3cbed..578a5062706e 100644
> --- a/fs/freevxfs/vxfs_super.c
> +++ b/fs/freevxfs/vxfs_super.c
> @@ -229,6 +229,8 @@ static int vxfs_fill_super(struct super_block *sbp, void *dp, int silent)
>  
>  	sbp->s_op = &vxfs_super_ops;
>  	sbp->s_fs_info = infp;
> +	sbp->s_time_min = 0;
> +	sbp->s_time_max = U32_MAX;
>  
>  	if (!vxfs_try_sb_magic(sbp, silent, 1,
>  			(__force __fs32)cpu_to_le32(VXFS_SUPER_MAGIC))) {
> diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
> index 8a20ddd25f2d..d0b59d03a7a9 100644
> --- a/fs/jffs2/fs.c
> +++ b/fs/jffs2/fs.c
> @@ -590,6 +590,9 @@ int jffs2_do_fill_super(struct super_block *sb, void *data, int silent)
>  	sb->s_blocksize = PAGE_SIZE;
>  	sb->s_blocksize_bits = PAGE_SHIFT;
>  	sb->s_magic = JFFS2_SUPER_MAGIC;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = U32_MAX;
> +
>  	if (!sb_rdonly(sb))
>  		jffs2_start_garbage_collect_thread(c);
>  	return 0;
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index f4e10cb9f734..b2dc4d1f9dcc 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -503,6 +503,8 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_fs_info = sbi;
>  	sb->s_max_links = JFS_LINK_MAX;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = U32_MAX;
>  	sbi->sb = sb;
>  	sbi->uid = INVALID_UID;
>  	sbi->gid = INVALID_GID;
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index f96073f25432..7cb5fd38eb14 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -277,6 +277,8 @@ static int minix_fill_super(struct super_block *s, void *data, int silent)
>  
>  	/* set up enough so that it can read an inode */
>  	s->s_op = &minix_sops;
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
>  	root_inode = minix_iget(s, MINIX_ROOT_INO);
>  	if (IS_ERR(root_inode)) {
>  		ret = PTR_ERR(root_inode);
> diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
> index 922d083bbc7c..e8da1cde87b9 100644
> --- a/fs/qnx4/inode.c
> +++ b/fs/qnx4/inode.c
> @@ -201,6 +201,8 @@ static int qnx4_fill_super(struct super_block *s, void *data, int silent)
>  	s->s_op = &qnx4_sops;
>  	s->s_magic = QNX4_SUPER_MAGIC;
>  	s->s_flags |= SB_RDONLY;	/* Yup, read-only yet */
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
>  
>  	/* Check the superblock signature. Since the qnx4 code is
>  	   dangerous, we should leave as quickly as possible
> diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
> index 0f8b0ff1ba43..345db56c98fd 100644
> --- a/fs/qnx6/inode.c
> +++ b/fs/qnx6/inode.c
> @@ -429,6 +429,8 @@ static int qnx6_fill_super(struct super_block *s, void *data, int silent)
>  	s->s_op = &qnx6_sops;
>  	s->s_magic = QNX6_SUPER_MAGIC;
>  	s->s_flags |= SB_RDONLY;        /* Yup, read-only yet */
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
>  
>  	/* ease the later tree level calculations */
>  	sbi = QNX6_SB(s);
> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index ab028ea0e561..d69b4ac0ae2f 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -1976,6 +1976,9 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
>  		goto error_unlocked;
>  	}
>  
> +	s->s_time_min = 0;
> +	s->s_time_max = U32_MAX;
> +
>  	rs = SB_DISK_SUPER_BLOCK(s);
>  	/*
>  	 * Let's do basic sanity check to verify that underlying device is not
> diff --git a/fs/romfs/super.c b/fs/romfs/super.c
> index 7d580f7c3f1d..a42c0e3079dc 100644
> --- a/fs/romfs/super.c
> +++ b/fs/romfs/super.c
> @@ -478,6 +478,8 @@ static int romfs_fill_super(struct super_block *sb, void *data, int silent)
>  	sb->s_maxbytes = 0xFFFFFFFF;
>  	sb->s_magic = ROMFS_MAGIC;
>  	sb->s_flags |= SB_RDONLY | SB_NOATIME;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = 0;
>  	sb->s_op = &romfs_super_ops;
>  
>  #ifdef CONFIG_ROMFS_ON_MTD
> diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
> index effa638d6d85..a9e9837617a9 100644
> --- a/fs/squashfs/super.c
> +++ b/fs/squashfs/super.c
> @@ -183,6 +183,8 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
>  		(u64) le64_to_cpu(sblk->id_table_start));
>  
>  	sb->s_maxbytes = MAX_LFS_FILESIZE;
> +	sb->s_time_min = 0;
> +	sb->s_time_max = U32_MAX;
>  	sb->s_flags |= SB_RDONLY;
>  	sb->s_op = &squashfs_super_ops;
>  
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 4ed0dca52ec8..1da0be667409 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -843,6 +843,10 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_maxbytes = MAX_LFS_FILESIZE;
>  
> +	sb->s_time_gran = NSEC_PER_SEC;
> +	sb->s_time_min = S32_MIN;
> +	sb->s_time_max = S32_MAX;
> +
>  	switch (sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) {
>  	case UFS_MOUNT_UFSTYPE_44BSD:
>  		UFSD("ufstype=44bsd\n");
> @@ -861,6 +865,9 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
>  		uspi->s_fshift = 9;
>  		uspi->s_sbsize = super_block_size = 1536;
>  		uspi->s_sbbase =  0;
> +		sb->s_time_gran = 1;
> +		sb->s_time_min = S64_MIN;
> +		sb->s_time_max = S64_MAX;
>  		flags |= UFS_TYPE_UFS2 | UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
>  		break;
>  		
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index a14d11d78bd8..1a0daf46bae8 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1685,6 +1685,8 @@ xfs_fs_fill_super(
>  	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
>  	sb->s_max_links = XFS_MAXLINK;
>  	sb->s_time_gran = 1;
> +	sb->s_time_min = S32_MIN;
> +	sb->s_time_max = S32_MAX;

For the XFS part,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  	set_posix_acl_flag(sb);
>  
>  	/* version 5 superblocks support inode version counters. */
> -- 
> 2.17.1
> 

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

* Re: [PATCH 05/20] utimes: Clamp the timestamps before update
  2019-07-31 15:14   ` Darrick J. Wong
@ 2019-07-31 15:33     ` Deepa Dinamani
  0 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-07-31 15:33 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List

On Wed, Jul 31, 2019 at 8:15 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Mon, Jul 29, 2019 at 06:49:09PM -0700, Deepa Dinamani wrote:
> > POSIX is ambiguous on the behavior of timestamps for
> > futimens, utimensat and utimes. Whether to return an
> > error or silently clamp a timestamp beyond the range
> > supported by the underlying filesystems is not clear.
> >
> > POSIX.1 section for futimens, utimensat and utimes says:
> > (http://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html)
> >
> > 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.
> >
> > If the tv_nsec field of a timespec structure has the special
> > value UTIME_NOW, the file's relevant timestamp shall be set
> > to the greatest value supported by the file system that is
> > not greater than the current time.
> >
> > [EINVAL]
> >     A new file timestamp would be a value whose tv_sec
> >     component is not a value supported by the file system.
> >
> > The patch chooses to clamp the timestamps according to the
> > filesystem timestamp ranges and does not return an error.
> > This is in line with the behavior of utime syscall also
> > since the POSIX page(http://pubs.opengroup.org/onlinepubs/009695399/functions/utime.html)
> > for utime does not mention returning an error or clamping like above.
> >
> > Same for utimes http://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html
> >
> > 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 350c9c16ace1..4c1a2ce90bbc 100644
> > --- a/fs/utimes.c
> > +++ b/fs/utimes.c
> > @@ -21,6 +21,7 @@ static int utimes_common(const struct path *path, struct timespec64 *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);
> > @@ -36,16 +37,24 @@ static int utimes_common(const struct path *path, struct timespec64 *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(times[0].tv_sec, sb->s_time_min, sb->s_time_max);
> > +                     if (times[0].tv_sec == sb->s_time_max || times[0].tv_sec == sb->s_time_min)
> > +                             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(times[1].tv_sec, sb->s_time_min, sb->s_time_max);
> > +                     if (times[1].tv_sec >= sb->s_time_max || times[1].tv_sec == sb->s_time_min)
>
> Line length.
>
> Also, didn't you just introduce a function to clamp tv_sec and fix
> granularity?  Why not just use it here?  I think this is the third time
> I've seen this open-coded logic.

Yes, we can use that now. Earlier we were not setting the tv_nsec to 0
in timestamp_truncate() which is why this was opencoded here.
I will make the change to include this.

Thanks,
Deepa

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

* Re: [PATCH 13/20] fs: affs: Initialize filesystem timestamp ranges
  2019-07-30  1:49 ` [PATCH 13/20] fs: affs: " Deepa Dinamani
@ 2019-08-01 11:28   ` David Sterba
  0 siblings, 0 replies; 62+ messages in thread
From: David Sterba @ 2019-08-01 11:28 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: linux-kernel, viro, arnd, y2038, David Sterba, linux-fsdevel

On Mon, Jul 29, 2019 at 06:49:17PM -0700, Deepa Dinamani wrote:
> Fill in the appropriate limits to avoid inconsistencies
> in the vfs cached inode times when timestamps are
> outside the permitted range.
> 
> Also fix timestamp calculation to avoid overflow
> while converting from days to seconds.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: dsterba@suse.com

Acked-by: David Sterba <dsterba@suse.com>

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-07-31 15:26   ` Darrick J. Wong
@ 2019-08-01 19:18     ` Deepa Dinamani
  2019-08-01 22:43       ` Theodore Y. Ts'o
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-01 19:18 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Theodore Ts'o, Andreas Dilger, linux-ext4

On Wed, Jul 31, 2019 at 8:26 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Mon, Jul 29, 2019 at 06:49:13PM -0700, Deepa Dinamani wrote:
> > ext4 has different overflow limits for max filesystem
> > timestamps based on the extra bytes available.
> >
> > The timestamp limits are calculated according to the
> > encoding table in
> > a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
> >
> > * extra  msb of                         adjust for signed
> > * epoch  32-bit                         32-bit tv_sec to
> > * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> > * 0 0    1    -0x80000000..-0x00000001  0x000000000   1901-12-13..1969-12-31
> > * 0 0    0    0x000000000..0x07fffffff  0x000000000   1970-01-01..2038-01-19
> > * 0 1    1    0x080000000..0x0ffffffff  0x100000000   2038-01-19..2106-02-07
> > * 0 1    0    0x100000000..0x17fffffff  0x100000000   2106-02-07..2174-02-25
> > * 1 0    1    0x180000000..0x1ffffffff  0x200000000   2174-02-25..2242-03-16
> > * 1 0    0    0x200000000..0x27fffffff  0x200000000   2242-03-16..2310-04-04
> > * 1 1    1    0x280000000..0x2ffffffff  0x300000000   2310-04-04..2378-04-22
> > * 1 1    0    0x300000000..0x37fffffff  0x300000000   2378-04-22..2446-05-10
>
> My recollection of ext4 has gotten rusty, so this could be a bogus
> question:
>
> Say you have a filesystem with s_inode_size > 128 where not all of the
> ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> don't support nanoseconds or times beyond 2038.  I think this happens on
> ext3 filesystems that reserved extra space for inode attrs that are
> subsequently converted to ext4?
>
> In any case, that means that you have some inodes that support 34-bit
> tv_sec and some inodes that only support 32-bit tv_sec.  For the inodes
> with 32-bit tv_sec, I think all that happens is that a large timestamp
> will be truncated further, right?
>
> And no mount time warning because at least /some/ of the inodes are
> ready to go for the next 30 years?

I'm confused about ext3 being converted to ext4. If the converted
inodes have extra space, then ext4_iget() will start using the extra
space when it modifies the on disk inode, won't it?

But, if there are 32 bit tv_sec and 34 bit tv_sec in a superblock then
from this macro below, if an inode has space for extra bits in
timestamps, it uses it. Otherwise, only the first 32 bits are copied
to the on disk timestamp. This matches the behavior today for 32 bit
tv_sec. But, the 34 bit tv_sec has a better behavior after the series
because of the clamping and warning.

#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)                \
do {                                        \
    (raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);        \
    if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     {\
        (raw_inode)->xtime ## _extra =                    \
                ext4_encode_extra_time(&(inode)->xtime);    \
        }                                \
} while (0)

I'm not sure if this corner case if important. Maybe the maintainers
can help me here. If this is important, then the inode time updates
for an ext4 inode should always be through ext4_setattr() and we can
clamp the timestamps there as a special case. And, this patch can be
added separately?

Thanks,
Deepa

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-01 19:18     ` Deepa Dinamani
@ 2019-08-01 22:43       ` Theodore Y. Ts'o
  2019-08-02 10:39         ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-01 22:43 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Darrick J. Wong, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Andreas Dilger, linux-ext4

On Thu, Aug 01, 2019 at 12:18:28PM -0700, Deepa Dinamani wrote:
> > Say you have a filesystem with s_inode_size > 128 where not all of the
> > ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> > don't support nanoseconds or times beyond 2038.  I think this happens on
> > ext3 filesystems that reserved extra space for inode attrs that are
> > subsequently converted to ext4?
> 
> I'm confused about ext3 being converted to ext4. If the converted
> inodes have extra space, then ext4_iget() will start using the extra
> space when it modifies the on disk inode, won't it?i

It is possible that you can have an ext3 file system with (for
example) 256 byte inodes, and all of the extra space was used for
extended attributes, then ext4 won't have the extra space available.
This is going toh be on an inode-by-inode basis, and if an extended
attribute is motdified or deleted, the space would become available,t
and then inode would start getting a higher resolution timestamp.

I really don't think it's worth worrying about that, though.  It's
highly unlikely ext3 file systems will be still be in service by the
time it's needed in 2038.  And if so, it's highly unlikely they would
be converted to ext4.

						- Ted

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

* Re: [PATCH 19/20] pstore: fs superblock limits
  2019-07-30  7:36     ` Arnd Bergmann
@ 2019-08-02  2:26       ` Deepa Dinamani
  2019-08-02  7:15         ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-02  2:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Kees Cook, Al Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Anton Vorontsov,
	Colin Cross, Tony Luck

On Tue, Jul 30, 2019 at 12:36 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Jul 30, 2019 at 6:31 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Jul 29, 2019 at 06:49:23PM -0700, Deepa Dinamani wrote:
> > > Also update the gran since pstore has microsecond granularity.
> >
> > So, I'm fine with this, but technically the granularity depends on the
> > backend storage... many have no actual time keeping, though. My point is,
> > pstore's timestamps are really mostly a lie, but the most common backend
> > (ramoops) is seconds-granularity.
> >
> > So, I'm fine with this, but it's a lie but it's a lie that doesn't
> > matter, so ...
> >
> > Acked-by: Kees Cook <keescook@chromium.org>
> >
> > I'm open to suggestions to improve it...
>
> If we don't care about using sub-second granularity, then setting it
> to one second unconditionally here will make it always use that and
> report it correctly.

Should this printf in ramoops_write_kmsg_hdr() also be fixed then?

        RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n",
        (time64_t)record->time.tv_sec,
        record->time.tv_nsec / 1000,
        record->compressed ? 'C' : 'D');
    persistent_ram_write(prz, hdr, len);

ramoops_read_kmsg_hdr() doesn't read this as microseconds. Seems like
a mismatch from above.

If we want to agree that we just want seconds granularity for pstore,
we could replace the tv_nsec part to be all 0's if anybody else is
depending on this format.
I could drop this patch from the series and post that patch seperately.

Thanks,
-Deepa

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

* Re: [PATCH 19/20] pstore: fs superblock limits
  2019-08-02  2:26       ` Deepa Dinamani
@ 2019-08-02  7:15         ` Arnd Bergmann
  2019-08-18 14:00           ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-02  7:15 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Kees Cook, Al Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Anton Vorontsov,
	Colin Cross, Tony Luck

On Fri, Aug 2, 2019 at 4:26 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 12:36 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Jul 30, 2019 at 6:31 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Mon, Jul 29, 2019 at 06:49:23PM -0700, Deepa Dinamani wrote:
> > > > Also update the gran since pstore has microsecond granularity.
> > >
> > > So, I'm fine with this, but technically the granularity depends on the
> > > backend storage... many have no actual time keeping, though. My point is,
> > > pstore's timestamps are really mostly a lie, but the most common backend
> > > (ramoops) is seconds-granularity.
> > >
> > > So, I'm fine with this, but it's a lie but it's a lie that doesn't
> > > matter, so ...
> > >
> > > Acked-by: Kees Cook <keescook@chromium.org>
> > >
> > > I'm open to suggestions to improve it...
> >
> > If we don't care about using sub-second granularity, then setting it
> > to one second unconditionally here will make it always use that and
> > report it correctly.
>
> Should this printf in ramoops_write_kmsg_hdr() also be fixed then?
>
>         RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n",
>         (time64_t)record->time.tv_sec,
>         record->time.tv_nsec / 1000,
>         record->compressed ? 'C' : 'D');
>     persistent_ram_write(prz, hdr, len);
>
> ramoops_read_kmsg_hdr() doesn't read this as microseconds. Seems like
> a mismatch from above.

Good catch. This seems to go back to commit 3f8f80f0cfeb ("pstore/ram:
Read and write to the 'compressed' flag of pstore"), which introduced the
nanosecond read. The write function however has always used
microseconds, and that was kept when the implementation changed
from timeval to timespec in commit 1e817fb62cd1 ("time: create
__getnstimeofday for WARNless calls").

> If we want to agree that we just want seconds granularity for pstore,
> we could replace the tv_nsec part to be all 0's if anybody else is
> depending on this format.
> I could drop this patch from the series and post that patch seperately.

We should definitely fix it to not produce a bogus nanosecond value.
Whether using full seconds or microsecond resolution is better here,
I don't know. It seems that pstore records generally get created
with a nanosecond nanosecond accurate timestamp from
ktime_get_real_fast_ns() and then truncated to the resolution of the
backend, rather than the normal jiffies-accurate inode timestamps that
we have for regular file systems.

This might mean that we do want the highest possible resolution
and not further truncate here, in case that information ends
up being useful afterwards.

         Arnd

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-01 22:43       ` Theodore Y. Ts'o
@ 2019-08-02 10:39         ` Arnd Bergmann
  2019-08-02 15:43           ` Theodore Y. Ts'o
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-02 10:39 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Deepa Dinamani, Darrick J. Wong,
	Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, Arnd Bergmann, y2038 Mailman List,
	Andreas Dilger, Ext4 Developers List

On Fri, Aug 2, 2019 at 12:43 AM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Thu, Aug 01, 2019 at 12:18:28PM -0700, Deepa Dinamani wrote:
> > > Say you have a filesystem with s_inode_size > 128 where not all of the
> > > ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> > > don't support nanoseconds or times beyond 2038.  I think this happens on
> > > ext3 filesystems that reserved extra space for inode attrs that are
> > > subsequently converted to ext4?
> >
> > I'm confused about ext3 being converted to ext4. If the converted
> > inodes have extra space, then ext4_iget() will start using the extra
> > space when it modifies the on disk inode, won't it?i
>
> It is possible that you can have an ext3 file system with (for
> example) 256 byte inodes, and all of the extra space was used for
> extended attributes, then ext4 won't have the extra space available.
> This is going toh be on an inode-by-inode basis, and if an extended
> attribute is motdified or deleted, the space would become available,t
> and then inode would start getting a higher resolution timestamp.

Is it correct to assume that this kind of file would have to be
created using the ext3.ko file system implementation that was
removed in linux-4.3, but not using ext2.ko or ext4.ko (which
would always set the extended timestamps even in "-t ext2" or
"-t ext3" mode)?

I tried to reproduce this on a modern kernel and with and
moderately old debugfs (1.42.13) but failed.

> I really don't think it's worth worrying about that, though.  It's
> highly unlikely ext3 file systems will be still be in service by the
> time it's needed in 2038.  And if so, it's highly unlikely they would
> be converted to ext4.

As the difference is easily visible even before y2038 by using
utimensat(old_inode, future_date) on a file, we should at least
decide what the sanest behavior is that we can easily implement,
and then document what is expected to happen here.

If we check for s_min_extra_isize instead of s_inode_size
to determine s_time_gran/s_time_max, we would warn
at mount time as well as and consistently truncate all
timestamps to full 32-bit seconds, regardless of whether
there is actually space or not.

Alternatively, we could warn if s_min_extra_isize is
too small, but use i_inode_size to determine
s_time_gran/s_time_max anyway.

From looking at e2fsprogs git history, I see that
s_min_extra_isize has always been set by mkfs since
2008, but I'm not sure if there would have been a
case in which it remains set but the ext3.ko would
ignore it and use that space anyway.

       Arnd

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-02 10:39         ` Arnd Bergmann
@ 2019-08-02 15:43           ` Theodore Y. Ts'o
  2019-08-02 19:00             ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-02 15:43 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Deepa Dinamani, Darrick J. Wong, Alexander Viro,
	Linux Kernel Mailing List, Linux FS-devel Mailing List,
	y2038 Mailman List, Andreas Dilger, Ext4 Developers List

On Fri, Aug 02, 2019 at 12:39:41PM +0200, Arnd Bergmann wrote:
> Is it correct to assume that this kind of file would have to be
> created using the ext3.ko file system implementation that was
> removed in linux-4.3, but not usiing ext2.ko or ext4.ko (which
> would always set the extended timestamps even in "-t ext2" or
> "-t ext3" mode)?

Correct.  Some of the enterprise distro's were using ext4 to support
"mount -t ext3" even before 4.3.  There's a CONFIG option to enable
using ext4 for ext2 or ext3 if they aren't enabled.

> If we check for s_min_extra_isize instead of s_inode_size
> to determine s_time_gran/s_time_max, we would warn
> at mount time as well as and consistently truncate all
> timestamps to full 32-bit seconds, regardless of whether
> there is actually space or not.
> 
> Alternatively, we could warn if s_min_extra_isize is
> too small, but use i_inode_size to determine
> s_time_gran/s_time_max anyway.

Even with ext4, s_min_extra_isize doesn't guarantee that will be able
to expand the inode.  This can fail if (a) we aren't able to expand
existing the transaction handle because there isn't enough space in
the journal, or (b) there is already an external xattr block which is
also full, so there is no space to evacuate an extended attribute out
of the inode's extra space.

We could be more aggressive by trying to expand make room in the inode
in ext4_iget (when we're reading in the inode, assuming the file
system isn't mounted read/only), instead of in the middle of
mark_inode_dirty().  That will eliminate failure mode (a) --- which is
statistically rare --- but it won't eliminate failure mode (b).

Ultimately, the question is which is worse: having a timestamp be
wrong, or randomly dropping an xattr from the inode to make room for
the extended timestamp.  We've come down on it being less harmful to
have the timestamp be wrong.

But again, this is a pretty rare case.  I'm not convinced it's worth
stressing about, since it's going to require multiple things to go
wrong before a timestamp will be bad.

					- Ted

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-02 15:43           ` Theodore Y. Ts'o
@ 2019-08-02 19:00             ` Arnd Bergmann
  2019-08-02 21:39               ` Theodore Y. Ts'o
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-02 19:00 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Arnd Bergmann, Deepa Dinamani,
	Darrick J. Wong, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Andreas Dilger,
	Ext4 Developers List

On Fri, Aug 2, 2019 at 5:43 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Aug 02, 2019 at 12:39:41PM +0200, Arnd Bergmann wrote:
> > Is it correct to assume that this kind of file would have to be
> > created using the ext3.ko file system implementation that was
> > removed in linux-4.3, but not usiing ext2.ko or ext4.ko (which
> > would always set the extended timestamps even in "-t ext2" or
> > "-t ext3" mode)?
>
> Correct.  Some of the enterprise distro's were using ext4 to support
> "mount -t ext3" even before 4.3.  There's a CONFIG option to enable
> using ext4 for ext2 or ext3 if they aren't enabled.
>
> > If we check for s_min_extra_isize instead of s_inode_size
> > to determine s_time_gran/s_time_max, we would warn
> > at mount time as well as and consistently truncate all
> > timestamps to full 32-bit seconds, regardless of whether
> > there is actually space or not.
> >
> > Alternatively, we could warn if s_min_extra_isize is
> > too small, but use i_inode_size to determine
> > s_time_gran/s_time_max anyway.
>
> Even with ext4, s_min_extra_isize doesn't guarantee that will be able
> to expand the inode.  This can fail if (a) we aren't able to expand
> existing the transaction handle because there isn't enough space in
> the journal, or (b) there is already an external xattr block which is
> also full, so there is no space to evacuate an extended attribute out
> of the inode's extra space.

I must have misunderstood what the field says. I expected that
with s_min_extra_isize set beyond the nanosecond fields, there
would be a guarantee that all inodes have at least as many
extra bytes already allocated. What circumstances would lead to
an i_extra_isize smaller than s_min_extra_isize?

> We could be more aggressive by trying to expand make room in the inode
> in ext4_iget (when we're reading in the inode, assuming the file
> system isn't mounted read/only), instead of in the middle of
> mark_inode_dirty().  That will eliminate failure mode (a) --- which is
> statistically rare --- but it won't eliminate failure mode (b).
>
> Ultimately, the question is which is worse: having a timestamp be
> wrong, or randomly dropping an xattr from the inode to make room for
> the extended timestamp.  We've come down on it being less harmful to
> have the timestamp be wrong.
>
> But again, this is a pretty rare case.  I'm not convinced it's worth
> stressing about, since it's going to require multiple things to go
> wrong before a timestamp will be bad.

Agreed, I'm not overly worried about this happening frequently,
I'd just feel better if we could reliably warn about the few instances
that might theoretically be affected.

        Arnd

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-02 19:00             ` Arnd Bergmann
@ 2019-08-02 21:39               ` Theodore Y. Ts'o
  2019-08-03  9:30                 ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-02 21:39 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Deepa Dinamani, Darrick J. Wong, Alexander Viro,
	Linux Kernel Mailing List, Linux FS-devel Mailing List,
	y2038 Mailman List, Andreas Dilger, Ext4 Developers List

On Fri, Aug 02, 2019 at 09:00:52PM +0200, Arnd Bergmann wrote:
> 
> I must have misunderstood what the field says. I expected that
> with s_min_extra_isize set beyond the nanosecond fields, there
> would be a guarantee that all inodes have at least as many
> extra bytes already allocated. What circumstances would lead to
> an i_extra_isize smaller than s_min_extra_isize?

When allocating new inodes, i_extra_isize is set to
s_want_extra_isize.  When modifying existing inodes, if i_extra_isize
is less than s_min_extra_isize, then we will attempt to move out
extended attribute(s) to the external xattr block.  So the
s_min_extra_isize field is not a guarantee, but rather an aspirationa
goal.  The idea is that at some point when we want to enable a new
feature, which needs more extra inode space, we can adjust
s_min_extra_size and s_want_extra_size, and the file system will
migrate things to meet these constraints.

The plan was to teach e2fsck how to fix all of the inodes to meet theh
s_min_extra_size value, but that never got implemented, and we even
then, e2fsck would have to deal with the case where tit couldn't move
the extended attribute(s) in the inode out, because there was no place
to put them.

In practice, this hasn't been that much of a limitation because we
haven't been adding that many extra inode fields.  Keep in mind that
Red Hat for example, has explicitly said they will *never* support
adding new features to an existing file system.  Their only supported
method is back up the file system, reformat it with the new file
system features, and then restore the file system.

Of course, if the backup/restore includes backing up the extended
attributes, and then restoring them, the xattr restore could fail,
unless the user also increased the inode size (e.g., from 256 bytes to
512 bytes).

Getting this right in the general case is *hard*.  Fortunately, the
corner cases really don't happen that often in practice, at least not
for pure Linux workloads.  Windows which can have arbitrarily large
security id's and ACL's might make this harder, of course --- although
ext4's EA in inode feature would make this better, modulo needing to
write more complex file system code to handle moving xattrs around.

Since the extended timestamps were one of the first extra inode fields
to be added, I strongly suggest that we not try to borrow trouble.
Solving the general case problem is *hard*.

					- Ted

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-02 21:39               ` Theodore Y. Ts'o
@ 2019-08-03  9:30                 ` Arnd Bergmann
  2019-08-03 16:02                   ` Theodore Y. Ts'o
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-03  9:30 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Arnd Bergmann, Deepa Dinamani,
	Darrick J. Wong, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Andreas Dilger,
	Ext4 Developers List

On Fri, Aug 2, 2019 at 11:39 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Aug 02, 2019 at 09:00:52PM +0200, Arnd Bergmann wrote:
> >
> > I must have misunderstood what the field says. I expected that
> > with s_min_extra_isize set beyond the nanosecond fields, there
> > would be a guarantee that all inodes have at least as many
> > extra bytes already allocated. What circumstances would lead to
> > an i_extra_isize smaller than s_min_extra_isize?
>
> When allocating new inodes, i_extra_isize is set to
> s_want_extra_isize.  When modifying existing inodes, if i_extra_isize
> is less than s_min_extra_isize, then we will attempt to move out
> extended attribute(s) to the external xattr block.  So the
> s_min_extra_isize field is not a guarantee, but rather an aspirationa
> goal.  The idea is that at some point when we want to enable a new
> feature, which needs more extra inode space, we can adjust
> s_min_extra_size and s_want_extra_size, and the file system will
> migrate things to meet these constraints.

I see in the ext4 code that we always try to expand i_extra_size
to s_want_extra_isize in ext4_mark_inode_dirty(), and that
s_want_extra_isize is always at least  s_min_extra_isize, so
we constantly try to expand the inode to fit.

What I still don't see is how any inode on the file system
image could have ended up with less than s_min_extra_isize
in the first place if s_min_extra_isize is never modified and
all inodes in the file system would have originally been
created with  i_extra_isize >= s_min_extra_isize if
EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE is set.

Did older versions of ext4 or ext3 ignore s_min_extra_isize
when creating inodes despite
EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
or is there another possibility I'm missing?

> Since the extended timestamps were one of the first extra inode fields
> to be added, I strongly suggest that we not try to borrow trouble.
> Solving the general case problem is *hard*.

As I said before, I absolutely don't suggest we solve the problem
of reliably setting the timestamps, I'm just trying to find out if there
is a way to know for sure that it cannot happen and alert the user
otherwise. So far I think we have concluded

- just checking s_inode_size is not sufficient because ext3
  may have created inodes with s_extra_isize too small
- checking s_min_extra_isize may not be sufficient either, for
  similar reasons I don't yet fully understand (see above).

If there is any other way to be sure that the file system
has never been mounted as a writable ext3, maybe that can
be used instead?

        Arnd

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-03  9:30                 ` Arnd Bergmann
@ 2019-08-03 16:02                   ` Theodore Y. Ts'o
  2019-08-03 20:24                     ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-03 16:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Deepa Dinamani, Darrick J. Wong, Alexander Viro,
	Linux Kernel Mailing List, Linux FS-devel Mailing List,
	y2038 Mailman List, Andreas Dilger, Ext4 Developers List

On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
> 
> I see in the ext4 code that we always try to expand i_extra_size
> to s_want_extra_isize in ext4_mark_inode_dirty(), and that
> s_want_extra_isize is always at least  s_min_extra_isize, so
> we constantly try to expand the inode to fit.

Yes, we *try*.  But we may not succeed.  There may actually be a
problem here if the cause is due to there simply is no space in the
external xattr block, so we might try and try every time we try to
modify that inode, and it would be a performance mess.  If it's due to
there being no room in the current transaction, then it's highly
likely it will succeed the next time.

> Did older versions of ext4 or ext3 ignore s_min_extra_isize
> when creating inodes despite
> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
> or is there another possibility I'm missing?

s_min_extra_isize could get changed in order to make room for some new
file system feature --- such as extended timestamps.  That's how we
might take an old ext3 file system with an inode size > 128, and try
to evacuate space for extended timestamps, on a best efforts basis.
And since it's best efforts is why Red Hat refuses to support that
case.  It'll work 99.9% of the time, but they don't want to deal with
the 0.01% cases showing up at their help desk.

If you want to pretend that file systems never get upgraded, then life
is much simpler.  The general approach is that for less-sophisticated
customers (e.g., most people running enterprise distros) file system
upgrades are not a thing.  But for sophisticated users, we do try to
make thing work for people who are aware of the risks / caveats /
rough edges.  Google won't have been able to upgrade thousands and
thousands of servers in data centers all over the world if we limited
ourselves to Red Hat's support restrictions.  Backup / reformat /
restore really isn't a practical rollout strategy for many exabytes of
file systems.

It sounds like your safety checks / warnings are mostly targeted at
low-information customers, no?

					- Ted

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-03 16:02                   ` Theodore Y. Ts'o
@ 2019-08-03 20:24                     ` Arnd Bergmann
  2019-08-07 18:04                       ` Andreas Dilger
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-03 20:24 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Arnd Bergmann, Deepa Dinamani,
	Darrick J. Wong, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Andreas Dilger,
	Ext4 Developers List

On Sat, Aug 3, 2019 at 6:03 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
> >
> > I see in the ext4 code that we always try to expand i_extra_size
> > to s_want_extra_isize in ext4_mark_inode_dirty(), and that
> > s_want_extra_isize is always at least  s_min_extra_isize, so
> > we constantly try to expand the inode to fit.
>
> Yes, we *try*.  But we may not succeed.  There may actually be a
> problem here if the cause is due to there simply is no space in the
> external xattr block, so we might try and try every time we try to
> modify that inode, and it would be a performance mess.  If it's due to
> there being no room in the current transaction, then it's highly
> likely it will succeed the next time.
>
> > Did older versions of ext4 or ext3 ignore s_min_extra_isize
> > when creating inodes despite
> > EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
> > or is there another possibility I'm missing?
>
> s_min_extra_isize could get changed in order to make room for some new
> file system feature --- such as extended timestamps.

Ok, that explains it. I assumed s_min_extra_isize was meant to
not be modifiable, and did not find a way to change it using the
kernel or tune2fs, but now I can see that debugfs can set it.

> If you want to pretend that file systems never get upgraded, then life
> is much simpler.  The general approach is that for less-sophisticated
> customers (e.g., most people running enterprise distros) file system
> upgrades are not a thing.  But for sophisticated users, we do try to
> make thing work for people who are aware of the risks / caveats /
> rough edges.  Google won't have been able to upgrade thousands and
> thousands of servers in data centers all over the world if we limited
> ourselves to Red Hat's support restrictions.  Backup / reformat /
> restore really isn't a practical rollout strategy for many exabytes of
> file systems.
>
> It sounds like your safety checks / warnings are mostly targeted at
> low-information customers, no?

Yes, that seems like a reasonable compromise: just warn based
on s_min_extra_isize, and assume that anyone who used debugfs
to set s_min_extra_isize to a higher value from an ext3 file system
during the migration to ext4 was aware of the risks already.

That leaves the question of what we should set the s_time_gran
and s_time_max to on a superblock with s_min_extra_isize<16
and s_want_extra_isize>=16.

If we base it on s_min_extra_isize, we never try to set a timestamp
later than 2038 and so will never fail, but anyone with a grandfathered
s_min_extra_isize from ext3 won't be able to set extended
timestamps on any files any more. Based on s_want_extra_isize
we would keep the current behavior, but could add a custom
warning in the ext4 code about the small s_min_extra_isize
indicating a theoretical problem.

       Arnd

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

* Re: [Y2038] [PATCH 05/20] utimes: Clamp the timestamps before update
  2019-07-30  1:49 ` [PATCH 05/20] utimes: Clamp the timestamps before update Deepa Dinamani
  2019-07-31 15:14   ` Darrick J. Wong
@ 2019-08-05 13:30   ` Ben Hutchings
  2019-08-10 20:36     ` Deepa Dinamani
  1 sibling, 1 reply; 62+ messages in thread
From: Ben Hutchings @ 2019-08-05 13:30 UTC (permalink / raw)
  To: Deepa Dinamani, viro, linux-kernel; +Cc: linux-fsdevel, y2038, arnd

On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> POSIX is ambiguous on the behavior of timestamps for
> futimens, utimensat and utimes. Whether to return an
> error or silently clamp a timestamp beyond the range
> supported by the underlying filesystems is not clear.
> 
> POSIX.1 section for futimens, utimensat and utimes says:
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html)
> 
> 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.
> 
> If the tv_nsec field of a timespec structure has the special
> value UTIME_NOW, the file's relevant timestamp shall be set
> to the greatest value supported by the file system that is
> not greater than the current time.
> 
> [EINVAL]
>     A new file timestamp would be a value whose tv_sec
>     component is not a value supported by the file system.
> 
> The patch chooses to clamp the timestamps according to the
> filesystem timestamp ranges and does not return an error.
> This is in line with the behavior of utime syscall also
> since the POSIX page(http://pubs.opengroup.org/onlinepubs/009695399/functions/utime.html)
> for utime does not mention returning an error or clamping like above.
> 
> Same for utimes http://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html
> 
> 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 350c9c16ace1..4c1a2ce90bbc 100644
> --- a/fs/utimes.c
> +++ b/fs/utimes.c
> @@ -21,6 +21,7 @@ static int utimes_common(const struct path *path, struct timespec64 *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);
> @@ -36,16 +37,24 @@ static int utimes_common(const struct path *path, struct timespec64 *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(times[0].tv_sec, sb->s_time_min, sb->s_time_max);
> +			if (times[0].tv_sec == sb->s_time_max || times[0].tv_sec == sb->s_time_min)

This is testing the un-clamped value.

> +				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(times[1].tv_sec, sb->s_time_min, sb->s_time_max);
> +			if (times[1].tv_sec >= sb->s_time_max || times[1].tv_sec == sb->s_time_min)

Similarly here, for the minimum.

I suggest testing for clamping like this:

			if (newattrs.ia_atime.tv_sec != times[0].tv_sec)
				...
			if (newattrs.ia_mtime.tv_sec != times[1].tv_sec)
				...

Ben.

> +				newattrs.ia_mtime.tv_nsec = 0;
> +			else
> +				newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
>  			newattrs.ia_valid |= ATTR_MTIME_SET;
>  		}
>  		/*
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom


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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-07-30  1:49 ` [PATCH 04/20] mount: Add mount warning for impending timestamp expiry Deepa Dinamani
@ 2019-08-05 14:12   ` Ben Hutchings
  2019-08-05 14:40     ` Arnd Bergmann
  2019-08-10 20:47     ` Deepa Dinamani
  2019-08-05 14:14   ` Ben Hutchings
  1 sibling, 2 replies; 62+ messages in thread
From: Ben Hutchings @ 2019-08-05 14:12 UTC (permalink / raw)
  To: Deepa Dinamani, viro, linux-kernel; +Cc: linux-fsdevel, y2038, arnd

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

On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> The warning reuses the uptime max of 30 years used by the
> setitimeofday().
> 
> Note that the warning is only added for new filesystem mounts
> through the mount syscall. Automounts do not have the same warning.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  fs/namespace.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index b26778bdc236..5314fac8035e 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
>  	error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
>  	if (error < 0)
>  		mntput(mnt);
> +
> +	if (!error && sb->s_time_max &&

I don't know why you are testing sb->s_time_max here - it should always
be non-zero since alloc_super() sets it to TIME64_MAX.

> +	    (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
> +		char *buf = (char *)__get_free_page(GFP_KERNEL);
> +		char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
> +
> +		pr_warn("Mounted %s file system at %s supports timestamps until 0x%llx\n",
> +			fc->fs_type->name, mntpath, (unsigned long long)sb->s_time_max);

This doesn't seem like a helpful way to log the time.  Maybe use
time64_to_tm() to convert to "broken down" time and then print it with
"%ptR"... but that wants struct rtc_time.  If you apply the attached
patch, however, you should then be able to print struct tm with
"%ptT".

Ben.

> +		free_page((unsigned long)buf);
> +	}
> +
>  	return error;
>  }
>  
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

[-- Attachment #2: 0001-vsprintf-Add-support-for-printing-struct-tm-in-human.patch --]
[-- Type: text/x-patch, Size: 3926 bytes --]

From 40cd51356d366110d33b891a6b9f3a428ed4ab2e Mon Sep 17 00:00:00 2001
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Date: Mon, 5 Aug 2019 14:49:33 +0100
Subject: [PATCH] vsprintf: Add support for printing struct tm in
 human-readable format

Change date_str(), time_str(), rtc_str() to take a struct tm (the main
kAPI type) instead of struct rtc_time (uAPI type), and rename rtc_str()
accordingly.

Change time_and_date() to accept either a struct rtc_time ('R') or
a struct tm ('T'), converting the former to the latter.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 Documentation/core-api/printk-formats.rst |  6 ++--
 lib/vsprintf.c                            | 34 ++++++++++++++++-------
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 75d2bbe9813f..fbae020bcc22 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -436,10 +436,10 @@ Time and date (struct rtc_time)
 	%ptR		YYYY-mm-ddTHH:MM:SS
 	%ptRd		YYYY-mm-dd
 	%ptRt		HH:MM:SS
-	%ptR[dt][r]
+	%pt[RT][dt][r]
 
-For printing date and time as represented by struct rtc_time structure in
-human readable format.
+For printing date and time as represented by struct rtc_time (R) or struct
+tm (T) structure in human readable format.
 
 By default year will be incremented by 1900 and month by 1. Use %ptRr (raw)
 to suppress this behaviour.
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 63937044c57d..dc32742876fd 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1699,7 +1699,7 @@ char *address_val(char *buf, char *end, const void *addr,
 }
 
 static noinline_for_stack
-char *date_str(char *buf, char *end, const struct rtc_time *tm, bool r)
+char *date_str(char *buf, char *end, const struct tm *tm, bool r)
 {
 	int year = tm->tm_year + (r ? 0 : 1900);
 	int mon = tm->tm_mon + (r ? 0 : 1);
@@ -1718,7 +1718,7 @@ char *date_str(char *buf, char *end, const struct rtc_time *tm, bool r)
 }
 
 static noinline_for_stack
-char *time_str(char *buf, char *end, const struct rtc_time *tm, bool r)
+char *time_str(char *buf, char *end, const struct tm *tm, bool r)
 {
 	buf = number(buf, end, tm->tm_hour, default_dec02_spec);
 	if (buf < end)
@@ -1734,16 +1734,13 @@ char *time_str(char *buf, char *end, const struct rtc_time *tm, bool r)
 }
 
 static noinline_for_stack
-char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
-	      struct printf_spec spec, const char *fmt)
+char *struct_tm_str(char *buf, char *end, const struct tm *tm,
+		    struct printf_spec spec, const char *fmt)
 {
 	bool have_t = true, have_d = true;
 	bool raw = false;
 	int count = 2;
 
-	if (check_pointer(&buf, end, tm, spec))
-		return buf;
-
 	switch (fmt[count]) {
 	case 'd':
 		have_t = false;
@@ -1775,11 +1772,28 @@ static noinline_for_stack
 char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
 		    const char *fmt)
 {
+	if (check_pointer(&buf, end, ptr, spec))
+		return buf;
+
 	switch (fmt[1]) {
-	case 'R':
-		return rtc_str(buf, end, (const struct rtc_time *)ptr, spec, fmt);
+	case 'R': {
+		const struct rtc_time *rtm = (const struct rtc_time *)ptr;
+		struct tm tm = {
+			.tm_sec  = rtm->tm_sec,
+			.tm_min  = rtm->tm_min,
+			.tm_hour = rtm->tm_hour,
+			.tm_mday = rtm->tm_mday,
+			.tm_mon  = rtm->tm_mon,
+			.tm_year = rtm->tm_year,
+		};
+
+		return struct_tm_str(buf, end, &tm, spec, fmt);
+	}
+	case 'T':
+		return struct_tm_str(buf, end, (const struct tm *)ptr,
+				     spec, fmt);
 	default:
-		return error_string(buf, end, "(%ptR?)", spec);
+		return error_string(buf, end, "(%pt?)", spec);
 	}
 }
 
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom


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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-07-30  1:49 ` [PATCH 04/20] mount: Add mount warning for impending timestamp expiry Deepa Dinamani
  2019-08-05 14:12   ` [Y2038] " Ben Hutchings
@ 2019-08-05 14:14   ` Ben Hutchings
  2019-08-10 20:44     ` Deepa Dinamani
  1 sibling, 1 reply; 62+ messages in thread
From: Ben Hutchings @ 2019-08-05 14:14 UTC (permalink / raw)
  To: Deepa Dinamani, viro, linux-kernel; +Cc: linux-fsdevel, y2038, arnd

On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> The warning reuses the uptime max of 30 years used by the
> setitimeofday().
> 
> Note that the warning is only added for new filesystem mounts
> through the mount syscall. Automounts do not have the same warning.
[...]

Another thing - perhaps this warning should be suppressed for read-only 
mounts?

Ben.
 
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom


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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-05 14:12   ` [Y2038] " Ben Hutchings
@ 2019-08-05 14:40     ` Arnd Bergmann
  2019-08-10 20:47     ` Deepa Dinamani
  1 sibling, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-05 14:40 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Deepa Dinamani, Al Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List

On Mon, Aug 5, 2019 at 4:12 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > The warning reuses the uptime max of 30 years used by the
> > setitimeofday().
> >
> > Note that the warning is only added for new filesystem mounts
> > through the mount syscall. Automounts do not have the same warning.
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > ---
> >  fs/namespace.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index b26778bdc236..5314fac8035e 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
> >       error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
> >       if (error < 0)
> >               mntput(mnt);
> > +
> > +     if (!error && sb->s_time_max &&
>
> I don't know why you are testing sb->s_time_max here - it should always
> be non-zero since alloc_super() sets it to TIME64_MAX.

I think we support some writable file systems that have no timestamps
at all, so both the minimum and maximum default to 0 (1970-01-01).

For these, there is no point in printing a warning, they just work
as designed, even though the maximum is expired.

       Arnd

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-03 20:24                     ` Arnd Bergmann
@ 2019-08-07 18:04                       ` Andreas Dilger
  2019-08-08 18:27                         ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Andreas Dilger @ 2019-08-07 18:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Theodore Y. Ts'o, Deepa Dinamani, Darrick J. Wong,
	Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List,
	Ext4 Developers List

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

On Aug 3, 2019, at 2:24 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Sat, Aug 3, 2019 at 6:03 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>> 
>> On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
>>> 
>>> I see in the ext4 code that we always try to expand i_extra_size
>>> to s_want_extra_isize in ext4_mark_inode_dirty(), and that
>>> s_want_extra_isize is always at least  s_min_extra_isize, so
>>> we constantly try to expand the inode to fit.
>> 
>> Yes, we *try*.  But we may not succeed.  There may actually be a
>> problem here if the cause is due to there simply is no space in the
>> external xattr block, so we might try and try every time we try to
>> modify that inode, and it would be a performance mess.  If it's due to
>> there being no room in the current transaction, then it's highly
>> likely it will succeed the next time.
>> 
>>> Did older versions of ext4 or ext3 ignore s_min_extra_isize
>>> when creating inodes despite
>>> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
>>> or is there another possibility I'm missing?
>> 
>> s_min_extra_isize could get changed in order to make room for some new
>> file system feature --- such as extended timestamps.
> 
> Ok, that explains it. I assumed s_min_extra_isize was meant to
> not be modifiable, and did not find a way to change it using the
> kernel or tune2fs, but now I can see that debugfs can set it.
> 
>> If you want to pretend that file systems never get upgraded, then life
>> is much simpler.  The general approach is that for less-sophisticated
>> customers (e.g., most people running enterprise distros) file system
>> upgrades are not a thing.  But for sophisticated users, we do try to
>> make thing work for people who are aware of the risks / caveats /
>> rough edges.  Google won't have been able to upgrade thousands and
>> thousands of servers in data centers all over the world if we limited
>> ourselves to Red Hat's support restrictions.  Backup / reformat /
>> restore really isn't a practical rollout strategy for many exabytes of
>> file systems.
>> 
>> It sounds like your safety checks / warnings are mostly targeted at
>> low-information customers, no?
> 
> Yes, that seems like a reasonable compromise: just warn based
> on s_min_extra_isize, and assume that anyone who used debugfs
> to set s_min_extra_isize to a higher value from an ext3 file system
> during the migration to ext4 was aware of the risks already.
> 
> That leaves the question of what we should set the s_time_gran
> and s_time_max to on a superblock with s_min_extra_isize<16
> and s_want_extra_isize>=16.
> 
> If we base it on s_min_extra_isize, we never try to set a timestamp
> later than 2038 and so will never fail, but anyone with a grandfathered
> s_min_extra_isize from ext3 won't be able to set extended
> timestamps on any files any more. Based on s_want_extra_isize
> we would keep the current behavior, but could add a custom
> warning in the ext4 code about the small s_min_extra_isize
> indicating a theoretical problem.

I think it makes the most sense to always try to set timestamps on
inodes that have enough space for them.  The chance of running into
a filesystem with 256-byte inode size but *no* space in the inode to
store an extended timestamp, but is *also* being modified by a new
kernel after 2038 is vanishingly small.  This would require formatting
the filesystem with non-default mke2fs for ext3, using the filesystem
and storing enough xattrs on inodes that there isn't space for 12 bytes
of extra isize, and using it for 30+ years without upgrading to use
ext4 (which will also try to expand the inode to store the nsec
timestamps) and then modifying the inode after 2038.

Rather than printing a warning at mount time (which may be confusing
to users for a problem they may never see), it makes sense to only
print such a warning in the vanishingly small case that someone actually
tries to modify the inode timestamp but it doesn't fit, rather than on
the theoretical case that may never happen.


Cheers, Andreas






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

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

* Re: [PATCH 09/20] ext4: Initialize timestamps limits
  2019-08-07 18:04                       ` Andreas Dilger
@ 2019-08-08 18:27                         ` Deepa Dinamani
  0 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-08 18:27 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Arnd Bergmann, Theodore Y. Ts'o, Darrick J. Wong,
	Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List,
	Ext4 Developers List

> Rather than printing a warning at mount time (which may be confusing
> to users for a problem they may never see), it makes sense to only
> print such a warning in the vanishingly small case that someone actually
> tries to modify the inode timestamp but it doesn't fit, rather than on
> the theoretical case that may never happen.

Arnd and I were discussing and we came to a similar conclusion that we
would not warn at mount. Arnd suggested maybe we include a
pr_warn_ratelimited() when we do write to these special inodes.

In general, there is an agreement to leave the fs granularity to a
higher resolution for such super blocks. And hence, the timestamps for
these special cases will never be clamped in memory.(Assuming we do
not want to make more changes and try to do something like
__ext4_expand_extra_isize() for in memory inode updates)
We could easily try to clamp these timestamps when they get written
out to the disk by modifying the EXT4_INODE_SET_XTIME to include such
a clamp. And, at this point we could also warn.

If this is acceptable, I could post an update.

-Deepa

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

* Re: [Y2038] [PATCH 05/20] utimes: Clamp the timestamps before update
  2019-08-05 13:30   ` [Y2038] " Ben Hutchings
@ 2019-08-10 20:36     ` Deepa Dinamani
  0 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-10 20:36 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Arnd Bergmann

On Mon, Aug 5, 2019 at 6:30 AM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > POSIX is ambiguous on the behavior of timestamps for
> > futimens, utimensat and utimes. Whether to return an
> > error or silently clamp a timestamp beyond the range
> > supported by the underlying filesystems is not clear.
> >
> > POSIX.1 section for futimens, utimensat and utimes says:
> > (http://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html)
> >
> > 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.
> >
> > If the tv_nsec field of a timespec structure has the special
> > value UTIME_NOW, the file's relevant timestamp shall be set
> > to the greatest value supported by the file system that is
> > not greater than the current time.
> >
> > [EINVAL]
> >     A new file timestamp would be a value whose tv_sec
> >     component is not a value supported by the file system.
> >
> > The patch chooses to clamp the timestamps according to the
> > filesystem timestamp ranges and does not return an error.
> > This is in line with the behavior of utime syscall also
> > since the POSIX page(http://pubs.opengroup.org/onlinepubs/009695399/functions/utime.html)
> > for utime does not mention returning an error or clamping like above.
> >
> > Same for utimes http://pubs.opengroup.org/onlinepubs/009695399/functions/utimes.html
> >
> > 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 350c9c16ace1..4c1a2ce90bbc 100644
> > --- a/fs/utimes.c
> > +++ b/fs/utimes.c
> > @@ -21,6 +21,7 @@ static int utimes_common(const struct path *path, struct timespec64 *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);
> > @@ -36,16 +37,24 @@ static int utimes_common(const struct path *path, struct timespec64 *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(times[0].tv_sec, sb->s_time_min, sb->s_time_max);
> > +                     if (times[0].tv_sec == sb->s_time_max || times[0].tv_sec == sb->s_time_min)
>
> This is testing the un-clamped value.
>
> > +                             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(times[1].tv_sec, sb->s_time_min, sb->s_time_max);
> > +                     if (times[1].tv_sec >= sb->s_time_max || times[1].tv_sec == sb->s_time_min)
>
> Similarly here, for the minimum.
>
> I suggest testing for clamping like this:
>
>                         if (newattrs.ia_atime.tv_sec != times[0].tv_sec)
>                                 ...
>                         if (newattrs.ia_mtime.tv_sec != times[1].tv_sec)
>                                 ...
>
> Ben.
>
> > +                             newattrs.ia_mtime.tv_nsec = 0;
> > +                     else
> > +                             newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
> >                       newattrs.ia_valid |= ATTR_MTIME_SET;
> >               }

Darrick pointed out that maybe we could use timestamp_truncate() here to clamp.
I think it is ok to truncate to the right granularity also here.
setattr callbacks do it already. So the diff here looks like below:

-                       newattrs.ia_atime.tv_sec =
-                               clamp(times[0].tv_sec, sb->s_time_min,
sb->s_time_max);
-                       if (times[0].tv_sec == sb->s_time_max ||
times[0].tv_sec == sb->s_time_min)
-                               newattrs.ia_atime.tv_nsec = 0;
-                       else
-                               newattrs.ia_atime.tv_nsec = times[0].tv_nsec;
+                       newattrs.ia_atime = timestamp_truncate(times[0], inode);

Thanks,
Deepa

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-05 14:14   ` Ben Hutchings
@ 2019-08-10 20:44     ` Deepa Dinamani
  2019-08-12 13:25       ` Ben Hutchings
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-10 20:44 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Arnd Bergmann

On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > The warning reuses the uptime max of 30 years used by the
> > setitimeofday().
> >
> > Note that the warning is only added for new filesystem mounts
> > through the mount syscall. Automounts do not have the same warning.
> [...]
>
> Another thing - perhaps this warning should be suppressed for read-only
> mounts?

Many filesystems support read only mounts only. We do fill in right
granularities and limits for these filesystems as well. In keeping
with the trend, I have added the warning accordingly. I don't think I
have a preference either way. But, not warning for the red only mounts
adds another if case. If you have a strong preference, I could add it
in.

-Deepa

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-05 14:12   ` [Y2038] " Ben Hutchings
  2019-08-05 14:40     ` Arnd Bergmann
@ 2019-08-10 20:47     ` Deepa Dinamani
  1 sibling, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-10 20:47 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Arnd Bergmann

> This doesn't seem like a helpful way to log the time.  Maybe use
> time64_to_tm() to convert to "broken down" time and then print it with
> "%ptR"... but that wants struct rtc_time.  If you apply the attached
> patch, however, you should then be able to print struct tm with
> "%ptT".

OK. Will print a more user friendly date string here.

Thanks,
-Deepa

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-10 20:44     ` Deepa Dinamani
@ 2019-08-12 13:25       ` Ben Hutchings
  2019-08-12 14:11         ` Arnd Bergmann
  0 siblings, 1 reply; 62+ messages in thread
From: Ben Hutchings @ 2019-08-12 13:25 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Arnd Bergmann

On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > The warning reuses the uptime max of 30 years used by the
> > > setitimeofday().
> > > 
> > > Note that the warning is only added for new filesystem mounts
> > > through the mount syscall. Automounts do not have the same warning.
> > [...]
> > 
> > Another thing - perhaps this warning should be suppressed for read-only
> > mounts?
> 
> Many filesystems support read only mounts only. We do fill in right
> granularities and limits for these filesystems as well. In keeping
> with the trend, I have added the warning accordingly. I don't think I
> have a preference either way. But, not warning for the red only mounts
> adds another if case. If you have a strong preference, I could add it
> in.

It seems to me that the warning is needed if there is a possibility of
data loss (incorrect timestamps, potentially leading to incorrect
decisions about which files are newer).  This can happen only when a
filesystem is mounted read-write, or when a filesystem image is
created.

I think that warning for read-only mounts would be an annoyance to
users retrieving files from old filesystems.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom


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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-12 13:25       ` Ben Hutchings
@ 2019-08-12 14:11         ` Arnd Bergmann
  2019-08-12 16:09           ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Arnd Bergmann @ 2019-08-12 14:11 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Deepa Dinamani, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List

On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > <ben.hutchings@codethink.co.uk> wrote:
> > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > The warning reuses the uptime max of 30 years used by the
> > > > setitimeofday().
> > > >
> > > > Note that the warning is only added for new filesystem mounts
> > > > through the mount syscall. Automounts do not have the same warning.
> > > [...]
> > >
> > > Another thing - perhaps this warning should be suppressed for read-only
> > > mounts?
> >
> > Many filesystems support read only mounts only. We do fill in right
> > granularities and limits for these filesystems as well. In keeping
> > with the trend, I have added the warning accordingly. I don't think I
> > have a preference either way. But, not warning for the red only mounts
> > adds another if case. If you have a strong preference, I could add it
> > in.
>
> It seems to me that the warning is needed if there is a possibility of
> data loss (incorrect timestamps, potentially leading to incorrect
> decisions about which files are newer).  This can happen only when a
> filesystem is mounted read-write, or when a filesystem image is
> created.
>
> I think that warning for read-only mounts would be an annoyance to
> users retrieving files from old filesystems.

I agree, the warning is not helpful for read-only mounts. An earlier
plan was to completely disallow writable mounts that might risk an
overflow (in some configurations at least). The warning replaces that
now, and I think it should also just warn for the cases that would
otherwise have been dangerous.

       Arnd

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-12 14:11         ` Arnd Bergmann
@ 2019-08-12 16:09           ` Deepa Dinamani
  2019-08-12 16:15             ` Deepa Dinamani
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-12 16:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ben Hutchings, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List

On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > <ben.hutchings@codethink.co.uk> wrote:
> > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > The warning reuses the uptime max of 30 years used by the
> > > > > setitimeofday().
> > > > >
> > > > > Note that the warning is only added for new filesystem mounts
> > > > > through the mount syscall. Automounts do not have the same warning.
> > > > [...]
> > > >
> > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > mounts?
> > >
> > > Many filesystems support read only mounts only. We do fill in right
> > > granularities and limits for these filesystems as well. In keeping
> > > with the trend, I have added the warning accordingly. I don't think I
> > > have a preference either way. But, not warning for the red only mounts
> > > adds another if case. If you have a strong preference, I could add it
> > > in.
> >
> > It seems to me that the warning is needed if there is a possibility of
> > data loss (incorrect timestamps, potentially leading to incorrect
> > decisions about which files are newer).  This can happen only when a
> > filesystem is mounted read-write, or when a filesystem image is
> > created.
> >
> > I think that warning for read-only mounts would be an annoyance to
> > users retrieving files from old filesystems.
>
> I agree, the warning is not helpful for read-only mounts. An earlier
> plan was to completely disallow writable mounts that might risk an
> overflow (in some configurations at least). The warning replaces that
> now, and I think it should also just warn for the cases that would
> otherwise have been dangerous.

Ok, I will make the change to exclude new read only mounts. I will use
__mnt_is_readonly() so that it also exculdes filesystems that are
readonly also.
The diff looks like below:

-       if (!error && sb->s_time_max &&
+       if (!error && !__mnt_is_readonly(mnt) &&
            (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {

Note that we can get rid of checking for non zero sb->s_time_max now.

-Deepa

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-12 16:09           ` Deepa Dinamani
@ 2019-08-12 16:15             ` Deepa Dinamani
  2019-08-12 17:43               ` Ben Hutchings
  0 siblings, 1 reply; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-12 16:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ben Hutchings, Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List

On Mon, Aug 12, 2019 at 9:09 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> > <ben.hutchings@codethink.co.uk> wrote:
> > > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > > <ben.hutchings@codethink.co.uk> wrote:
> > > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > > The warning reuses the uptime max of 30 years used by the
> > > > > > setitimeofday().
> > > > > >
> > > > > > Note that the warning is only added for new filesystem mounts
> > > > > > through the mount syscall. Automounts do not have the same warning.
> > > > > [...]
> > > > >
> > > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > > mounts?
> > > >
> > > > Many filesystems support read only mounts only. We do fill in right
> > > > granularities and limits for these filesystems as well. In keeping
> > > > with the trend, I have added the warning accordingly. I don't think I
> > > > have a preference either way. But, not warning for the red only mounts
> > > > adds another if case. If you have a strong preference, I could add it
> > > > in.
> > >
> > > It seems to me that the warning is needed if there is a possibility of
> > > data loss (incorrect timestamps, potentially leading to incorrect
> > > decisions about which files are newer).  This can happen only when a
> > > filesystem is mounted read-write, or when a filesystem image is
> > > created.
> > >
> > > I think that warning for read-only mounts would be an annoyance to
> > > users retrieving files from old filesystems.
> >
> > I agree, the warning is not helpful for read-only mounts. An earlier
> > plan was to completely disallow writable mounts that might risk an
> > overflow (in some configurations at least). The warning replaces that
> > now, and I think it should also just warn for the cases that would
> > otherwise have been dangerous.
>
> Ok, I will make the change to exclude new read only mounts. I will use
> __mnt_is_readonly() so that it also exculdes filesystems that are
> readonly also.
> The diff looks like below:
>
> -       if (!error && sb->s_time_max &&
> +       if (!error && !__mnt_is_readonly(mnt) &&
>             (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
>
> Note that we can get rid of checking for non zero sb->s_time_max now.

One more thing, we will probably have to add a second warning for when
the filesystem is re-mounted rw after the initial readonly mount.

-Deepa

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

* Re: [Y2038] [PATCH 04/20] mount: Add mount warning for impending timestamp expiry
  2019-08-12 16:15             ` Deepa Dinamani
@ 2019-08-12 17:43               ` Ben Hutchings
  0 siblings, 0 replies; 62+ messages in thread
From: Ben Hutchings @ 2019-08-12 17:43 UTC (permalink / raw)
  To: Deepa Dinamani, Arnd Bergmann
  Cc: Alexander Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List

On Mon, 2019-08-12 at 09:15 -0700, Deepa Dinamani wrote:
> On Mon, Aug 12, 2019 at 9:09 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> > > <ben.hutchings@codethink.co.uk> wrote:
> > > > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > > > <ben.hutchings@codethink.co.uk> wrote:
> > > > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > > > The warning reuses the uptime max of 30 years used by the
> > > > > > > setitimeofday().
> > > > > > > 
> > > > > > > Note that the warning is only added for new filesystem mounts
> > > > > > > through the mount syscall. Automounts do not have the same warning.
> > > > > > [...]
> > > > > > 
> > > > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > > > mounts?
> > > > > 
> > > > > Many filesystems support read only mounts only. We do fill in right
> > > > > granularities and limits for these filesystems as well. In keeping
> > > > > with the trend, I have added the warning accordingly. I don't think I
> > > > > have a preference either way. But, not warning for the red only mounts
> > > > > adds another if case. If you have a strong preference, I could add it
> > > > > in.
> > > > 
> > > > It seems to me that the warning is needed if there is a possibility of
> > > > data loss (incorrect timestamps, potentially leading to incorrect
> > > > decisions about which files are newer).  This can happen only when a
> > > > filesystem is mounted read-write, or when a filesystem image is
> > > > created.
> > > > 
> > > > I think that warning for read-only mounts would be an annoyance to
> > > > users retrieving files from old filesystems.
> > > 
> > > I agree, the warning is not helpful for read-only mounts. An earlier
> > > plan was to completely disallow writable mounts that might risk an
> > > overflow (in some configurations at least). The warning replaces that
> > > now, and I think it should also just warn for the cases that would
> > > otherwise have been dangerous.
> > 
> > Ok, I will make the change to exclude new read only mounts. I will use
> > __mnt_is_readonly() so that it also exculdes filesystems that are
> > readonly also.
> > The diff looks like below:
> > 
> > -       if (!error && sb->s_time_max &&
> > +       if (!error && !__mnt_is_readonly(mnt) &&
> >             (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
> > 
> > Note that we can get rid of checking for non zero sb->s_time_max now.
> 
> One more thing, we will probably have to add a second warning for when
> the filesystem is re-mounted rw after the initial readonly mount.

Indeed, there would need to be a check for remount-read-write.  I
didn't check whether remount uses this same code path.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom


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

* Re: [PATCH 19/20] pstore: fs superblock limits
  2019-08-02  7:15         ` Arnd Bergmann
@ 2019-08-18 14:00           ` Deepa Dinamani
  0 siblings, 0 replies; 62+ messages in thread
From: Deepa Dinamani @ 2019-08-18 14:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Kees Cook, Al Viro, Linux Kernel Mailing List,
	Linux FS-devel Mailing List, y2038 Mailman List, Anton Vorontsov,
	Colin Cross, Tony Luck

On Fri, Aug 2, 2019 at 12:15 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Aug 2, 2019 at 4:26 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > On Tue, Jul 30, 2019 at 12:36 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Tue, Jul 30, 2019 at 6:31 AM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > On Mon, Jul 29, 2019 at 06:49:23PM -0700, Deepa Dinamani wrote:
> > > > > Also update the gran since pstore has microsecond granularity.
> > > >
> > > > So, I'm fine with this, but technically the granularity depends on the
> > > > backend storage... many have no actual time keeping, though. My point is,
> > > > pstore's timestamps are really mostly a lie, but the most common backend
> > > > (ramoops) is seconds-granularity.
> > > >
> > > > So, I'm fine with this, but it's a lie but it's a lie that doesn't
> > > > matter, so ...
> > > >
> > > > Acked-by: Kees Cook <keescook@chromium.org>
> > > >
> > > > I'm open to suggestions to improve it...
> > >
> > > If we don't care about using sub-second granularity, then setting it
> > > to one second unconditionally here will make it always use that and
> > > report it correctly.
> >
> > Should this printf in ramoops_write_kmsg_hdr() also be fixed then?
> >
> >         RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n",
> >         (time64_t)record->time.tv_sec,
> >         record->time.tv_nsec / 1000,
> >         record->compressed ? 'C' : 'D');
> >     persistent_ram_write(prz, hdr, len);
> >
> > ramoops_read_kmsg_hdr() doesn't read this as microseconds. Seems like
> > a mismatch from above.
>
> Good catch. This seems to go back to commit 3f8f80f0cfeb ("pstore/ram:
> Read and write to the 'compressed' flag of pstore"), which introduced the
> nanosecond read. The write function however has always used
> microseconds, and that was kept when the implementation changed
> from timeval to timespec in commit 1e817fb62cd1 ("time: create
> __getnstimeofday for WARNless calls").
>
> > If we want to agree that we just want seconds granularity for pstore,
> > we could replace the tv_nsec part to be all 0's if anybody else is
> > depending on this format.
> > I could drop this patch from the series and post that patch seperately.
>
> We should definitely fix it to not produce a bogus nanosecond value.
> Whether using full seconds or microsecond resolution is better here,
> I don't know. It seems that pstore records generally get created
> with a nanosecond nanosecond accurate timestamp from
> ktime_get_real_fast_ns() and then truncated to the resolution of the
> backend, rather than the normal jiffies-accurate inode timestamps that
> we have for regular file systems.
>
> This might mean that we do want the highest possible resolution
> and not further truncate here, in case that information ends
> up being useful afterwards.

I made a list of granularities used by pstore drivers using pstore_register():

1. efi - seconds
2. ramoops - microsecond
3. erst - seconds
4. powerpc/nvram64 - seconds

I will leave pstore granularity at nanoseconds and fix the ramoops read.

-Deepa

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

end of thread, other threads:[~2019-08-18 14:00 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30  1:49 [PATCH 00/20] vfs: Add support for timestamp limits Deepa Dinamani
2019-07-30  1:49 ` [PATCH 01/20] vfs: Add file timestamp range support Deepa Dinamani
2019-07-30  1:49 ` [PATCH 02/20] vfs: Add timestamp_truncate() api Deepa Dinamani
2019-07-30  1:49 ` [PATCH 03/20] timestamp_truncate: Replace users of timespec64_trunc Deepa Dinamani
2019-07-30  8:27   ` OGAWA Hirofumi
2019-07-30 17:26     ` Deepa Dinamani
2019-07-30 22:28       ` Anton Altaparmakov
2019-07-31  0:08         ` Deepa Dinamani
2019-07-30  1:49 ` [PATCH 04/20] mount: Add mount warning for impending timestamp expiry Deepa Dinamani
2019-08-05 14:12   ` [Y2038] " Ben Hutchings
2019-08-05 14:40     ` Arnd Bergmann
2019-08-10 20:47     ` Deepa Dinamani
2019-08-05 14:14   ` Ben Hutchings
2019-08-10 20:44     ` Deepa Dinamani
2019-08-12 13:25       ` Ben Hutchings
2019-08-12 14:11         ` Arnd Bergmann
2019-08-12 16:09           ` Deepa Dinamani
2019-08-12 16:15             ` Deepa Dinamani
2019-08-12 17:43               ` Ben Hutchings
2019-07-30  1:49 ` [PATCH 05/20] utimes: Clamp the timestamps before update Deepa Dinamani
2019-07-31 15:14   ` Darrick J. Wong
2019-07-31 15:33     ` Deepa Dinamani
2019-08-05 13:30   ` [Y2038] " Ben Hutchings
2019-08-10 20:36     ` Deepa Dinamani
2019-07-30  1:49 ` [PATCH 06/20] fs: Fill in max and min timestamps in superblock Deepa Dinamani
2019-07-31 15:28   ` Darrick J. Wong
2019-07-30  1:49 ` [PATCH 07/20] 9p: Fill min and max timestamps in sb Deepa Dinamani
2019-07-30  1:49 ` [PATCH 08/20] adfs: Fill in max and min " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 09/20] ext4: Initialize timestamps limits Deepa Dinamani
2019-07-31 15:26   ` Darrick J. Wong
2019-08-01 19:18     ` Deepa Dinamani
2019-08-01 22:43       ` Theodore Y. Ts'o
2019-08-02 10:39         ` Arnd Bergmann
2019-08-02 15:43           ` Theodore Y. Ts'o
2019-08-02 19:00             ` Arnd Bergmann
2019-08-02 21:39               ` Theodore Y. Ts'o
2019-08-03  9:30                 ` Arnd Bergmann
2019-08-03 16:02                   ` Theodore Y. Ts'o
2019-08-03 20:24                     ` Arnd Bergmann
2019-08-07 18:04                       ` Andreas Dilger
2019-08-08 18:27                         ` Deepa Dinamani
2019-07-30  1:49 ` [PATCH 10/20] fs: nfs: Initialize filesystem timestamp ranges Deepa Dinamani
2019-07-30  1:49 ` [PATCH 11/20] fs: cifs: " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 12/20] fs: fat: " Deepa Dinamani
2019-07-30  9:31   ` OGAWA Hirofumi
2019-07-30 17:39     ` Deepa Dinamani
2019-07-31  0:48       ` OGAWA Hirofumi
2019-07-30  1:49 ` [PATCH 13/20] fs: affs: " Deepa Dinamani
2019-08-01 11:28   ` David Sterba
2019-07-30  1:49 ` [PATCH 14/20] fs: sysv: " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 15/20] fs: ceph: " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 16/20] fs: orangefs: " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 17/20] fs: hpfs: " Deepa Dinamani
2019-07-30  1:49 ` [PATCH 18/20] fs: omfs: " Deepa Dinamani
2019-07-30 14:25   ` Bob Copeland
2019-07-30  1:49 ` [PATCH 19/20] pstore: fs superblock limits Deepa Dinamani
2019-07-30  4:31   ` Kees Cook
2019-07-30  7:36     ` Arnd Bergmann
2019-08-02  2:26       ` Deepa Dinamani
2019-08-02  7:15         ` Arnd Bergmann
2019-08-18 14:00           ` Deepa Dinamani
2019-07-30  1:49 ` [PATCH 20/20] isofs: Initialize filesystem timestamp ranges 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).