Re: [RFC] proc: fix create timestamp of files in proc

On Thu, Jul 21, 2022 at 12:45:42PM -0500, Eric W. Biederman wrote:

Luis Chamberlain [off-list ref] writes:

quoted

On Thu, Jul 21, 2022 at 04:16:17PM +0800, Zhang Yuchen wrote:

quoted

A user has reported a problem that the /proc/{pid} directory
creation timestamp is incorrect.

The directory?

A bit of history that I don't think made it to the git log is that
procps uses the /proc/<pid> directory, to discover the uid and gid of the
process.

Oy vei.

In that sense, if *really* the directory for a PID all of a sudden
disappear and reapper with another time stamp, I wonder if its
possible to change the uid/gid.

I have memories of Albert Cahalan reporting regressions because I
had tweaked the attributes of proc in ways that I expected no
one would care about and caused a regression in procps.

Thanks for bringing this little bit of history to light.

So it is not unreasonable for people to have used proc in surprising
ways.

I took a quick read through procps and it looks like procps reads
/proc/<pid>/stat to get the start_time of the process.

OK so at least for that world of userspace *if* indeed the pid directory
is changing time somehow (the exact way in which this can happen as
reported is yet to be determined) the existing procps would *not* have
been affected.

If *procps* is not being used and someone is trying to re-implement it,
and if indeed it is using the time of the inode, and *if* this really
can change, then we have an explanation to the current situation.

Which leads us to this quality of implementation issue that the time
on the inode of a proc directory is the first time that someone read
the directory and observed the file.  Which does not need to be anything
at all related to the start time.

Best I think we can do, is document this, and if we *want* to accept
a *new mechanism*, add a kconfig entry so to distinguish this, so to
not break old reliance on prior behaviour.

I think except for the symlinks and files under /proc/pid/fd and
/proc/pid/fdinfo there is a very good case for making all of the files
/proc/pid have a creation time of equal to the creation of the process
in question.

A new kconfig entry could enable this. But that still would not
prevent userspace from modifying file's creation time and I'm not
sure why we'd want to change things.

Although the files under /proc/pid/task/ need to have
a time equal to the creation time of the thread in question.

Improving the quality of implementation requires caring enough to make
that change, and right now I don't.

My biggest fear is breaking things.

If we *really* are somehow modifying the timestamp of the directory
inode though, I'd like to understand how that happened.

At the same time I would say the suggested patch is a bad idea.
Any application that breaks because we hard set the timestamp on a proc
file or directory to the beginning of time is automatically counts as a
regression.

It is why I was seriously confused, why would someone purposely try to
create a regression.

Since the entire point of the patch is to break applications that are
doing things wrong, aka cause regressions I don't think the patch
make sense.

And hence my serious distaste for it.

So I would vote for understanding what the problem user is doing.  Then
either proc can be improved to better support users, or we can do
nothing.

Except for explaining the history and how people have legitimately used
implementation details of proc before, I am not really interested.  But
I do think we can do better.

Thanks for the feedback.

  Luis