Bernd Edlinger [off-list ref] writes:

On 3/10/20 8:06 PM, Eric W. Biederman wrote:

quoted

Bernd Edlinger [off-list ref] writes:

quoted

This changes do_io_accounting to use the new exec_update_mutex
instead of cred_guard_mutex.

This fixes possible deadlocks when the trace is accessing
/proc/$pid/io for instance.

This should be safe, as the credentials are only used for reading.

This is an improvement.

We probably want to do this just as an incremental step in making things
better but perhaps I am blind but I am not finding the reason for
guarding this with the cred_guard_mutex to be at all persuasive.

I think moving the ptrace_may_access check down to after the
unlock_task_sighand would be just as effective at addressing the
concerns raised in the original commit.  I think the task_lock provides
all of the barrier we need to make it safe to move the ptrace_may_access
checks safe.

The reason I say this is I don't see exec changing ->ioac.  Just
performing some I/O which would update the io accounting statistics.

Maybe the suid executable is starting up and doing io or not,
and what the program does immediately at startup is a secret,
that we want to keep secret but evil eve want to find out.
eve is using /proc/alice/io to do that.

It is a bit constructed, but seems like a security concern.
when we keep the exec_update_mutex while collecting the data, we
cannot see any io of the new process when the new credentials
don't allow that.

Jann Horn has convinced me we should just convert these to the
exec_change_mutex today.  Because while not 100% correct in theory, the
only really interesting case is exec.  So the code does something
interesting and worth while, and mostly correct.  The last thing I want
to do is to cause an unnecessary regression.

Eric