From: Philippe Waroquiers Date: Thu, 9 May 2013 21:29:23 +0000 (+0000) Subject: fix 319235 --db-attach=yes is broken with Yama ptrace scoping enabled X-Git-Tag: svn/VALGRIND_3_9_0~300 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ce4aedfab12b7f41e1a9b673a4e348cebee0c22;p=thirdparty%2Fvalgrind.git fix 319235 --db-attach=yes is broken with Yama ptrace scoping enabled On Ubuntu systems, ptrace_scoping could forbid a process to ptrace another. This ptrace scoping was already handled for vgdb by using SET_PTRACER (the valgrind process must be ptraced by vgdb when it is blocked in a syscall). set_ptracer is however also needed when the old mechanism --db-attach=yes is used. The following changes are done: * make the set_ptracer logic callable outside gdbserver * make set_ptracer less restrictive (i.e. allow all processes of the user to ptrace). This removes a limitation for vgdb. * call the set_ptracer in the child launched for --db-attach=yes * cleaned up the ptrace scope restriction message and doc as vgdb is now working properly by default, even with ptrace_scope enabled. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13384 --- diff --git a/NEWS b/NEWS index 7de23c4dcf..e4f6768e3e 100644 --- a/NEWS +++ b/NEWS @@ -222,6 +222,7 @@ m = merged into 3_8_BRANCH 315959 [390] valgrind man page has bogus SGCHECK (and no BBV) OPTIONS section 316144 [390] valgrind.1 manpage contains unknown ??? strings for some core option references 316145 [390] callgrind command line options in manpage reference (unknown) callgrind manual +319235 [390] --db-attach=yes is broken with Yama ptrace scoping enabled n-i-bz [390] report error for vgdb snapshot requested before execution n-i-bz [390] Some wrong command line options could be ignored n-i-bz [390] same as 303624 (fixed in 3.8.0), but for x86 android diff --git a/coregrind/m_debugger.c b/coregrind/m_debugger.c index e53094be7c..1c967ef8f9 100644 --- a/coregrind/m_debugger.c +++ b/coregrind/m_debugger.c @@ -35,6 +35,7 @@ #include "pub_core_xarray.h" #include "pub_core_clientstate.h" #include "pub_core_debugger.h" +#include "pub_core_gdbserver.h" #include "pub_core_libcbase.h" #include "pub_core_libcprint.h" #include "pub_core_libcproc.h" @@ -363,6 +364,7 @@ void VG_(start_debugger) ( ThreadId tid ) if (pid == 0) { /* child */ + VG_(set_ptracer)(); rc = VG_(ptrace)(VKI_PTRACE_TRACEME, 0, NULL, NULL); vg_assert(rc == 0); rc = VG_(kill)(VG_(getpid)(), VKI_SIGSTOP); diff --git a/coregrind/m_gdbserver/remote-utils.c b/coregrind/m_gdbserver/remote-utils.c index 8a9c9697a3..59a698effa 100644 --- a/coregrind/m_gdbserver/remote-utils.c +++ b/coregrind/m_gdbserver/remote-utils.c @@ -105,12 +105,7 @@ int vgdb_state_looks_bad(const char* where) return 0; // all is ok. } -/* On systems that defines PR_SET_PTRACER, verify if ptrace_scope is - is permissive enough for vgdb. Otherwise, call set_ptracer. - This is especially aimed at Ubuntu >= 10.10 which has added - the ptrace_scope context. */ -static -void set_ptracer(void) +void VG_(set_ptracer)(void) { #ifdef PR_SET_PTRACER SysRes o; @@ -132,11 +127,16 @@ void set_ptracer(void) dlog(1, "ptrace_scope %c\n", ptrace_scope); if (ptrace_scope != '0') { /* insufficient default ptrace_scope. - Indicate to the kernel that we accept to be - ptraced by our vgdb. */ - ret = VG_(prctl) (PR_SET_PTRACER, shared->vgdb_pid, 0, 0, 0); - dlog(1, "set_ptracer to vgdb_pid %d result %d\n", - shared->vgdb_pid, ret); + Indicate to the kernel that we accept to be ptraced. */ +#ifdef PR_SET_PTRACER_ANY + ret = VG_(prctl) (PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0); + dlog(1, "set_ptracer to PR_SET_PTRACER_ANY result %d\n", ret); +#else + ret = VG_(prctl) (PR_SET_PTRACER, 1, 0, 0, 0); + dlog(1, "set_ptracer to 1 result %d\n", ret); +#endif + if (ret) + VG_(umsg)("error calling PR_SET_PTRACER, vgdb might block\n"); } } else { dlog(0, "Could not read the ptrace_scope setting from %s\n", @@ -181,7 +181,6 @@ int ensure_write_remote_desc(void) be reasonably sure someone is reading on the other side of the fifo. */ if (!vgdb_state_looks_bad("bad?@ensure_write_remote_desc")) { - set_ptracer(); write_remote_desc = open_fifo ("write", to_gdb, VKI_O_WRONLY); } } @@ -293,7 +292,7 @@ void remote_open (const HChar *name) if (!mknod_done) { mknod_done++; - + VG_(set_ptracer)(); /* * Unlink just in case a previous process with the same PID had been * killed and hence Valgrind hasn't had the chance yet to remove these. diff --git a/coregrind/pub_core_gdbserver.h b/coregrind/pub_core_gdbserver.h index f4286c36df..9cb06c9905 100644 --- a/coregrind/pub_core_gdbserver.h +++ b/coregrind/pub_core_gdbserver.h @@ -53,6 +53,12 @@ extern Bool VG_(gdbserver_activity) (ThreadId tid); // gdbserver is then stopped (using VG_(gdbserver) (0)) void VG_(gdbserver_exit) (ThreadId tid, VgSchedReturnCode tids_schedretcode); +/* On systems that defines PR_SET_PTRACER, verify if ptrace_scope is + is permissive enough for vgdb or --db-attach=yes. + Otherwise, call set_ptracer. + This is especially aimed at Ubuntu >= 10.10 which has added + the ptrace_scope context. */ +void VG_(set_ptracer)(void); /* Called by low level to insert or remove a break or watch point. Break or watch point implementation is done using help from the tool. diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index fed0d9217f..dc3bedae1c 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -101,14 +101,12 @@ I_die_here : (PTRACEINVOKER) architecture missing in vgdb.c #if defined(PTRACEINVOKER) #include #if defined(VGO_linux) -# include # include #endif #endif -// Outputs information for the user about ptrace_scope protection -// or ptrace not working. +// Outputs information for the user about ptrace not working. static void ptrace_restrictions_msg(void); static int debuglevel; @@ -869,11 +867,7 @@ Bool invoke_gdbserver (int pid) // address pushed on the stack should ensure this is detected. /* Not yet attached. If problem, vgdb can abort, - no cleanup needed. - - On Ubuntu>= 10.10, a /proc setting can disable ptrace. - So, Valgrind has to SET_PTRACER this vgdb. Once this - is done, this vgdb can ptrace the valgrind process. */ + no cleanup needed. */ DEBUG(1, "attach to 'main' pid %d\n", pid); if (!attach(pid, "attach main pid")) { @@ -2049,27 +2043,6 @@ void report_pid (int pid, Bool on_stdout) static void ptrace_restrictions_msg(void) { -# ifdef PR_SET_PTRACER - const char *ptrace_scope_setting_file = "/proc/sys/kernel/yama/ptrace_scope"; - int fd = -1; - char ptrace_scope = 'X'; - fd = open (ptrace_scope_setting_file, O_RDONLY, 0); - if (fd >= 0 && (read (fd, &ptrace_scope, 1) == 1) && (ptrace_scope != '0')) { - fprintf (stderr, - "Note: your kernel restricts ptrace invoker using %s\n" - "vgdb will only be able to attach to a Valgrind process\n" - "blocked in a system call *after* an initial successful attach\n", - ptrace_scope_setting_file); - } else if (ptrace_scope == 'X') { - DEBUG (1, - "PR_SET_PTRACER defined" - " but could not determine ptrace scope from %s\n", - ptrace_scope_setting_file); - } - if (fd >= 0) - close (fd); -# endif - # ifndef PTRACEINVOKER fprintf(stderr, "Note: ptrace invoker not implemented\n" diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml index ff17351043..870996fc71 100644 --- a/docs/xml/manual-core-adv.xml +++ b/docs/xml/manual-core-adv.xml @@ -995,7 +995,8 @@ $5 = 36 Connecting to or interrupting a Valgrind process blocked in a system call requires the "ptrace" system call to be usable. - This may be disabled in your kernel for security reasons. + This may be disabled in your kernel for security reasons. + When running your program, Valgrind's scheduler periodically checks whether there is any work to be handled by @@ -1043,24 +1044,9 @@ $5 = 36 Ubuntu versions 10.10 and later may restrict the scope of ptrace to the children of the process calling ptrace. As the Valgrind process is not a child of vgdb, such restricted scoping - causes the ptrace calls to fail. To avoid that, when Valgrind - gdbserver receives the first packet from a vgdb, it calls - prctl(PR_SET_PTRACER, vgdb_pid, 0, 0, - 0) to ensure vgdb can reliably use ptrace. - Once vgdb_pid has been marked as - a ptracer, vgdb can then properly force the invocation of - Valgrind gdbserver when needed. To ensure the vgdb is set as a - ptracer before the Valgrind process gets blocked in a system - call, connect your GDB to the Valgrind gdbserver at startup by - passing to Valgrind. - - Note that - this "set ptracer" technique does not solve the problem in the - case where a standalone vgdb process wants to connect to the - gdbserver, since the first command to be sent by a standalone - vgdb must wake up the Valgrind process before Valgrind gdbserver - will mark vgdb as a ptracer. - + causes the ptrace calls to fail. To avoid that, Valgrind will + automatically allow all processes belonging to the same userid to + "ptrace" a Valgrind process, by using PR_SET_PTRACER. Unblocking processes blocked in system calls is not currently implemented on Mac OS X and Android. So you cannot diff --git a/gdbserver_tests/make_local_links b/gdbserver_tests/make_local_links index 8e50093b2e..73a2f0017d 100755 --- a/gdbserver_tests/make_local_links +++ b/gdbserver_tests/make_local_links @@ -99,8 +99,7 @@ ln -f -s ../coregrind/vgdb gdbserver_tests/vgdb # if ptrace not implemented in vgdb or OS restricts the initial attach, # some tests would block for a loooonnnng time. if gdbserver_tests/vgdb --help 2>&1 | - grep -e 'ptrace invoker not implemented' \ - -e 'kernel restricts ptrace invoker' > /dev/null + grep -e 'ptrace invoker not implemented' > /dev/null then rm -f gdbserver_tests/vgdb.ptraceinvoker else