On Fri, Sep 30, 2016 at 05:35:05PM +0200, Oleg Nesterov wrote: > On 09/23, Jann Horn wrote: > > > > This is a new per-threadgroup lock that can often be taken instead of > > cred_guard_mutex and has less deadlock potential. > > Oh, please don't. > > > I'm doing this because > > Oleg Nesterov mentioned the potential for deadlocks, in particular if a > > debugged task is stuck in execve, trying to get rid of a ptrace-stopped > > thread, and the debugger attempts to inspect procfs files of the debugged > > task. > > Yes, but we need to fix this anyway. And I am not sure the new mutex can > actually help. > > And I think that cred_guard_mutex is already over-used in fs/proc. Say, > I think lock_trace() must die, I simply can't understand why it is useful. IMO it's just about reducing potential (really small) information leaks. These leaks probably don't matter much in practice - they're racy and only disclose minimal amounts of data -, but in principle, they expose data. But since you're opposed to it and I don't see a significant benefit in having these checks, I'll just remove the lock_trace() stuff from my patch for now. > Suppose we modify, say, proc_pid_stack() to do > > save_stack_trace_tsk(task, &trace); > if (!ptrace_may_access(task, ...)) > goto return -EPERM; > > for (i = 0; i < trace.nr_entries; i++) > seq_printf(...); > > return 0; > > is there any problem if it shows some trace before setuid exec does > install_exec_creds() ? > > Oleg. >
Attachment:
signature.asc
Description: Digital signature
