lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220721081617.36103-1-zhangyuchen.lcr@bytedance.com>
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ