Re: [PATCH RFC] Allow introspection to already attached ptracer in __ptrace_may_access

From: Stijn Volckaert
Date: Tue Jan 06 2015 - 04:08:14 EST

Casey Schaufler schreef op 6/01/2015 om 1:17:
On 1/5/2015 3:47 PM, Kees Cook wrote:
On Wed, Dec 24, 2014 at 5:28 AM, Stijn Volckaert
<Stijn.Volckaert@xxxxxxxxxxxxx> wrote:
Hello,

I ran across the following problem recently but I'm not entirely sure
whether this should be fixed in ptrace or in Yama. I'm working on a
ptrace-based monitor that forks off its own tracee during startup. The
monitor attaches to the tracee and then lets the tracee perform an execve
call. This is much like running a program in gdb.

My monitor is multi-threaded and uses one monitor thread for every tracee
thread so whenever the tracee forks/vforks/clones, I fire up a new monitor
thread, detach the old monitor thread from the tracee thread and attach the
new monitor thread to the tracee thread.

I have recently stumbled upon several applications in which the main process
A forks off process B and then immediately exits. Under normal circumstances
the following would happen:

Monitor[0] --- FORKS OFF ---> Monitor[0]'
Monitor[0] --- PTRACE_ATTACH ---> Monitor[0]'
Monitor[0]' --- EXECVE ---> Process A

Process A --- FORKS OFF ---> Process B
Monitor[0] --- PTRACE_DETACH ---> Process B
Monitor[1] --- PTRACE_ATTACH ---> Process B

With Yama enabled (and the scope set to YAMA_SCOPE_RELATIONAL) however, a
few interesting things can (and usually do) happen:

1) If Process A dies before Monitor[1] is attached to Process B, the attach
will fail since from Yama's point of view, Process B is no longer a
descendant of Monitor[1]. This problem is probably hard to fix
but I've circumvented it by delaying the death of Process A until Process B
is attached to Monitor[1].
Just to make sure I understand this better, "Monitor" is the initial
process, and [0] and [1] are separate threads within that process? I
would expect B to have Monitor as its parent after A died, but I must
be misunderstanding something.

Regardless, your "interesting thing 1" is certainly a side-effect of
YAMA_SCOPE_RELATIONAL trying to do its job.
Correct. Process B is obviously still descendant from the process that is ptracing it so you'd
think that this would work with YAMA_SCOPE_RELATIONAL. However, after process A dies,
/sbin/init becomes the new parent of Process B. If you want to see this yourselves, you can
reproduce this problem with this little test program:

$ cat ptrace_bug.c
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>

void daemon_process() {
printf("pre-open\n");
int fd = open("test.txt", O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
if (fd != -1) {
write(fd, "post-open\n", strlen("post-open\n"));
close(fd);
}
exit(0);
}

int main(int argc, char** argv) {
if (fork() == 0)
daemon_process();
return 0;
}
$ gcc -o ptrace_bug ptrace_bug.c

If I strace this on a machine which I've patched with my proposed patch below, I see the following
output for the "daemon_process":

$ strace -o strace_out -ff ./ptrace_bug
$ cat strace_out.2815
...
write(1, "pre-open\n", 9) = 9
open("test.txt", O_RDWR|O_CREAT, 0600) = 3
write(3, "post-open\n", 10) = 10
...

However, on a machine which I haven't patched and with YAMA_SCOPE_RELATIONAL, I see:
...
write(1, 0x7f74338c0000, 9) = 9
open(0x4007bd, O_RDWR|O_CREAT, 0600) = 3
write(3, 0x4007c6, 10) = 10
...

This is the exact same problem: you can still read the regs and stop/resume tracees but
you can't do process_vm_{read,write}v or the ptrace ops that depend on it.

If I now add a pause() call just before the exit of the daemon process, you'll see the following
after process A dies:

$ ps u
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
stijn 29772 0.0 0.0 4852 820 pts/0 S+ 09:41 0:00 strace -o strace_out -ff ./ptrace_bug
stijn 29778 0.0 0.0 4300 92 pts/0 S+ 09:41 0:00 ./ptrace_bug

$ cat /proc/29778/status
Name: ptrace_bug
State: S (sleeping)
Tgid: 29778
Ngid: 0
Pid: 29778
PPid: 1
TracerPid: 29772

PPid is 1 so /sbin/init is now the real parent. As far as I can tell, this whole reparenting goes on
in kernel/exit.c due to:

do_exit
-> exit_notify
-> forget_original_parent
-> find_new_reaper

From my viewpoint, it would make more sense to reparent Process B to the ptracer that originally
forked off Process A. However, looking at the exit.c code, what actually goes on does seem like
the intended behavior.

2) More interestingly though, even if Process B does get attached to
Monitor[1], as soon as Process A dies, all process_vm_readv and
process_vm_writev calls on Process B start failing. Any other ptrace
operations peformed on Process B do succeed.

process_vm_readv|writev use __ptrace_may_access to check whether the
operation is permitted, whereas other ptrace operations (with the exception
of PTRACE_ATTACH) use ptrace_check_attach.
Right, process_vm_{read,write}v use PTRACE_MODE_ATTACH (which is what
Yama interposes via the LSM entry point in __ptrace_may_access).

To fix this problem, __ptrace_may_access should be forced to return 0 if the
calling process is already attached to the target process.

The question now is whether or not it's the security module's responsibility
to check whether a tracee relationship is already in place or if ptrace
itself should do it. For the latter case, which seems more logical to me,
you could use the patch below.

What do you guys think?

Regards,
Stijn Volckaert

--

Signed-off-by: Stijn Volckaert <Stijn.Volckaert@xxxxxxxxxxxxx>

--- a/kernel/ptrace.c 2014-12-24 13:53:23.055346526 +0100
+++ b/kernel/ptrace.c 2014-12-24 14:17:20.617824840 +0100
@@ -232,6 +232,9 @@ static int __ptrace_may_access(struct ta
/* Don't let security modules deny introspection */
if (same_thread_group(task, current))
return 0;
+ /* Don't deny introspection to already attached ptracer */
+ if (!ptrace_check_attach(task, true))
+ return 0;
rcu_read_lock();
tcred = __task_cred(task);
if (uid_eq(cred->uid, tcred->euid) &&

I'm nervous to add this (or Oleg's suggestion) generally to
__ptrace_may_access, as it would mean other LSMs would stop seeing
access checks even when attached. It does seem silly to deny ptrace
checks when already attached, but it does change the behavior here.
An LSM may chose to do checks on a per access basis. Think in terms
of access checks on read/write instead of open. Smack and SELinux
do this for some network checks. It is reasonable to think that there
is a case where a security attribute (or access rule) could change
between the attach and the access.

Example: You allow the access when the developer mode switch is
set, but not when it isn't. Someone flips the switch.
That makes sense I guess. However, the problem in my case was not that the security policy
had change between the "open" and the "read/write" check. Instead, after the death of
Process A, the security check now lacks the context to see the relationship between
the tracer and the tracee.

If the other LSM folks don't see a problem here, then it should live
in the general case. Otherwise, I'm happy to add this check only in
Yama. The existing Yama scopes should ignore attach requests when
already attached.

-Kees


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/