Date:   Thu, 21 Jul 2022 16:16:17 +0800
From:   Zhang Yuchen <zhangyuchen.lcr@...edance.com>
To:     mcgrof@...nel.org, keescook@...omium.org, yzaikin@...gle.com,
        songmuchun@...edance.com
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        Zhang Yuchen <zhangyuchen.lcr@...edance.com>
Subject: [RFC] proc: fix create timestamp of files in proc

A user has reported a problem that the /proc/{pid} directory
creation timestamp is incorrect.
He believes that the directory was created when the process was
started, so the timestamp of the directory creation should also
be when the process was created.

The file timestamp in procfs is the timestamp when the inode was
created. If the inode of a file in procfs is reclaimed, the inode
will be recreated when it is opened again, and the timestamp will
be changed to the time when it was recreated.
In other file systems, this timestamp is typically recorded in
the file system and assigned to the inode when the inode is created.

This mechanism can be confusing to users who are not familiar with
it.
For users who know this mechanism, they will choose not to trust
this time.
So the timestamp now has no meaning other than to confuse the user.
It needs fixing.

There are three solutions. We don't have to make the timestamp
meaningful, as long as the user doesn't trust the timestamp.

1. Add to the kernel documentation that the timestamp in PROC is
   not reliable and do not use this timestamp.
   The problem with this solution is that most users don't read
   the kernel documentation and it can still be misleading.

2. Fix it, change the timestamp of /proc/pid to the timestamp of
   process creation.

   This raises new questions.

   a. Users don't know which kernel version is correct.

   b. This problem exists not only in the pid directory, but also
      in other directories under procfs. It would take a lot of
      extra work to fix them all. There are also easier ways for
      users to get the creation time information better than this.

   c. We need to describe the specific meaning of each file under
      proc in the kernel documentation for the creation time.
      Because the creation time of various directories has different
      meanings. For example, PID directory is the process creation
      time, FD directory is the FD creation time and so on.

   d. Some files have no associated entity, such as iomem.
      Unable to give a meaningful time.

3. Instead of fixing it, set the timestamp in all procfs to 0.
   Users will see it as an error and will not use it.

I think 3 is better. Any other suggestions?

Signed-off-by: Zhang Yuchen <zhangyuchen.lcr@...edance.com>
---
 fs/proc/base.c        | 4 +++-
 fs/proc/inode.c       | 3 ++-
 fs/proc/proc_sysctl.c | 3 ++-
 fs/proc/self.c        | 3 ++-
 fs/proc/thread_self.c | 3 ++-
 5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 0b72a6d8aac3..af440ef13091 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1892,6 +1892,8 @@ struct inode *proc_pid_make_inode(struct super_block *sb,
 	struct proc_inode *ei;
 	struct pid *pid;
 
+	struct timespec64 ts_zero = {0, 0};
+
 	/* We need a new inode */
 
 	inode = new_inode(sb);
@@ -1902,7 +1904,7 @@ struct inode *proc_pid_make_inode(struct super_block *sb,
 	ei = PROC_I(inode);
 	inode->i_mode = mode;
 	inode->i_ino = get_next_ino();
-	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = ts_zero;
 	inode->i_op = &proc_def_inode_operations;
 
 	/*
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index fd40d60169b5..efb1c935fa8d 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -642,6 +642,7 @@ const struct inode_operations proc_link_inode_operations = {
 struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
 {
 	struct inode *inode = new_inode(sb);
+	struct timespec64 ts_zero = {0, 0};
 
 	if (!inode) {
 		pde_put(de);
@@ -650,7 +651,7 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
 
 	inode->i_private = de->data;
 	inode->i_ino = de->low_ino;
-	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = ts_zero;
 	PROC_I(inode)->pde = de;
 	if (is_empty_pde(de)) {
 		make_empty_dir_inode(inode);
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 021e83fe831f..c670f9d3b871 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -455,6 +455,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
 	struct ctl_table_root *root = head->root;
 	struct inode *inode;
 	struct proc_inode *ei;
+	struct timespec64 ts_zero = {0, 0};
 
 	inode = new_inode(sb);
 	if (!inode)
@@ -476,7 +477,7 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
 	head->count++;
 	spin_unlock(&sysctl_lock);
 
-	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = ts_zero;
 	inode->i_mode = table->mode;
 	if (!S_ISDIR(table->mode)) {
 		inode->i_mode |= S_IFREG;
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 72cd69bcaf4a..b9e572fdc27c 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -44,9 +44,10 @@ int proc_setup_self(struct super_block *s)
 	self = d_alloc_name(s->s_root, "self");
 	if (self) {
 		struct inode *inode = new_inode(s);
+		struct timespec64 ts_zero = {0, 0};
 		if (inode) {
 			inode->i_ino = self_inum;
-			inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+			inode->i_mtime = inode->i_atime = inode->i_ctime = ts_zero;
 			inode->i_mode = S_IFLNK | S_IRWXUGO;
 			inode->i_uid = GLOBAL_ROOT_UID;
 			inode->i_gid = GLOBAL_ROOT_GID;
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index a553273fbd41..964966387da2 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -44,9 +44,10 @@ int proc_setup_thread_self(struct super_block *s)
 	thread_self = d_alloc_name(s->s_root, "thread-self");
 	if (thread_self) {
 		struct inode *inode = new_inode(s);
+		struct timespec64 ts_zero = {0, 0};
 		if (inode) {
 			inode->i_ino = thread_self_inum;
-			inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+			inode->i_mtime = inode->i_atime = inode->i_ctime = ts_zero;
 			inode->i_mode = S_IFLNK | S_IRWXUGO;
 			inode->i_uid = GLOBAL_ROOT_UID;
 			inode->i_gid = GLOBAL_ROOT_GID;
-- 
2.30.2

Powered by blists - more mailing lists