Bernd Edlinger bernd.edlinger@hotmail.de writes:
On 3/10/20 8:06 PM, Eric W. Biederman wrote:
Bernd Edlinger bernd.edlinger@hotmail.de writes:
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