Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251910 > unrolled thread

[PATCH 0/2] wait/ptrace: always assume __WALL if the child is traced

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] wait/ptrace: always assume __WALL if the child is  traced Oleg Nesterov <oleg@redhat.com> - 2015-10-20 19:30 +0200
    Re: [PATCH 0/2] wait/ptrace: always assume __WALL if the child is  traced Oleg Nesterov <oleg@redhat.com> - 2015-10-20 19:50 +0200
    Re: [PATCH 0/2] wait/ptrace: always assume __WALL if the child is  traced Pedro Alves <palves@redhat.com> - 2015-10-22 16:50 +0200
      Re: [PATCH 0/2] wait/ptrace: always assume __WALL if the child is  traced Oleg Nesterov <oleg@redhat.com> - 2015-10-25 15:50 +0100

#1251910 — [PATCH 0/2] wait/ptrace: always assume __WALL if the child is traced

Damn. I simply do not know what should/can we do. From the change
log:

	And I can only hope that this won't break something.

yet this patch cc's -stable.


Please see the changelog, but in short: this is not a kernel bug
but unlikely we can fix all distributions, so I think we have to
change the kernel.

HOWEVER. With this change __WCLONE and __WALL have no effect for
debugger, do_wait() works as if __WALL is set if the child (natural
or not) is traced.


Jan, Pedro, could you please confirm this won't break gdb? I tried
to look into gdb-7.1, and at first glance gdb uses __WCLONE only
because __WALL doesn't work on older kernels, iow it seems to me
that gdb actually wants __WALL so this change should be fine.


Any other ideas?

Oleg.

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

[toc] | [next] | [standalone]

#1251922

Forgot to say...

Another question is why PTRACE_TRACEME succeeds in this case. I guess
it is to late to change (break) the rules, but I never understood the
security checks. The comment above cap_ptrace_traceme() says:

	Determine whether another process may trace the current

and "another process" is parent. To me this looks strange, imo we should
determine whether the current may abuse its parent. So perhaps we could
change ptrace_traceme() to fail if

	current->parent_exec_id != parent->self_exec_id

?

But this too can break something. Although I can't imagine why the
child reaper or a PR_SET_CHILD_SUBREAPER process may want to trace
the reparented tasks.

On 10/20, Oleg Nesterov wrote:
>
> Damn. I simply do not know what should/can we do. From the change
> log:
>
> 	And I can only hope that this won't break something.
>
> yet this patch cc's -stable.
>
>
> Please see the changelog, but in short: this is not a kernel bug
> but unlikely we can fix all distributions, so I think we have to
> change the kernel.
>
> HOWEVER. With this change __WCLONE and __WALL have no effect for
> debugger, do_wait() works as if __WALL is set if the child (natural
> or not) is traced.
>
>
> Jan, Pedro, could you please confirm this won't break gdb? I tried
> to look into gdb-7.1, and at first glance gdb uses __WCLONE only
> because __WALL doesn't work on older kernels, iow it seems to me
> that gdb actually wants __WALL so this change should be fine.
>
>
> Any other ideas?
>
> Oleg.

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

[toc] | [prev] | [next] | [standalone]

#1253887

On 10/20/2015 06:17 PM, Oleg Nesterov wrote:

> Jan, Pedro, could you please confirm this won't break gdb? I tried
> to look into gdb-7.1, and at first glance gdb uses __WCLONE only
> because __WALL doesn't work on older kernels, iow it seems to me
> that gdb actually wants __WALL so this change should be fine.

Right, gdb actually wants __WALL, but it doesn't use it to keep
compatibility with kernels that predate it.

gdb nowadays has an __WALL emulation waitpid wrapper
(alternates __WCLONE+WNOHANG with 0+WNOHANG, blocks
on sigsuspend/SIGCHLD):

 https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/nat/linux-waitpid.c;h=cbcdd95afa9c664993542b0b3851e79fbae4e1df;hb=HEAD#l77

Though it's not used everywhere.  Some older code in the
ptrace backend open codes the "try __WCLONE, then try !__WCLONE."
dance.

Seems like __WALL was added in Linux 2.4; gdb could probably
assume it's available nowadays...

In any case, to make sure existing gdb binaries would still work
with your kernel change, I ran GDB's testsuite with this:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index cbcdd95..864ba2e 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -149,3 +149,17 @@ my_waitpid (int pid, int *status, int flags)
   errno = out_errno;
   return ret;
 }
+
+#include <dlfcn.h>
+
+pid_t
+waitpid (pid_t pid, int *status, int options)
+{
+  static pid_t (*waitpid2) (pid_t pid, int *status, int options) = NULL;
+
+  if (waitpid2 == NULL)
+    waitpid2 = dlsym (RTLD_NEXT, "waitpid");
+
+  options |= __WALL;
+  return waitpid2 (pid, status, options);
+}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and got no regressions.  So seems like all would be well from
GDB's perspective.

Thanks,
Pedro Alves

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

[toc] | [prev] | [next] | [standalone]

#1255423

On 10/22, Pedro Alves wrote:
>
> In any case, to make sure existing gdb binaries would still work
> with your kernel change, I ran GDB's testsuite with this:
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
> index cbcdd95..864ba2e 100644
> --- a/gdb/nat/linux-waitpid.c
> +++ b/gdb/nat/linux-waitpid.c
> @@ -149,3 +149,17 @@ my_waitpid (int pid, int *status, int flags)
>    errno = out_errno;
>    return ret;
>  }
> +
> +#include <dlfcn.h>
> +
> +pid_t
> +waitpid (pid_t pid, int *status, int options)
> +{
> +  static pid_t (*waitpid2) (pid_t pid, int *status, int options) = NULL;
> +
> +  if (waitpid2 == NULL)
> +    waitpid2 = dlsym (RTLD_NEXT, "waitpid");
> +
> +  options |= __WALL;
> +  return waitpid2 (pid, status, options);
> +}
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Thanks a lot Pedro!

So gdb should be fine, strace too. Perhaps we should change the kernel
this way and forget about /sbin/init fixes.

Oleg.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web