linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: lineprinter@altlinux.org
Cc: Oleg Nesterov <oleg@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Dmitry V. Levin" <ldv@altlinux.org>,
	Eugene Syromiatnikov <esyr@redhat.com>,
	Andrew Lutomirski <luto@kernel.org>,
	strace-devel@lists.strace.io
Subject: Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request
Date: Wed, 7 Nov 2018 12:44:58 -0800	[thread overview]
Message-ID: <CALCETrV1v-DPRfDRwiH=xn29bxWxiHdZtAH1nw=dsmDtnT0YGQ@mail.gmail.com> (raw)
In-Reply-To: <20181107042751.3b519062@akathisia>

> On Nov 6, 2018, at 7:27 PM, Elvira Khabirova <lineprinter@altlinux.org> wrote:
>
> PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
> the tracee is blocked in. The request returns meaningful data only
> when the tracee is in a syscall-enter-stop or a syscall-exit-stop.
>
> There are two reasons for a special syscall-related ptrace request.
>
> Firstly, with the current ptrace API there are cases when ptracer cannot
> retrieve necessary information about syscalls. Some examples include:
> * The notorious int-0x80-from-64-bit-task issue. See [1] for details.
> In short, if a 64-bit task performs a syscall through int 0x80, its tracer
> has no reliable means to find out that the syscall was, in fact,
> a compat syscall, and misidentifies it.
> * Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
> Common practice is to keep track of the sequence of ptrace-stops in order
> not to mix the two syscall-stops up. But it is not as simple as it looks;
> for example, strace had a (just recently fixed) long-standing bug where
> attaching strace to a tracee that is performing the execve system call
> led to the tracer identifying the following syscall-exit-stop as
> syscall-enter-stop, which messed up all the state tracking.
> * Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
> ("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
> and process_vm_readv become unavailable when the process dumpable flag
> is cleared. On ia64 this results in all syscall arguments being unavailable.
>
> Secondly, ptracers also have to support a lot of arch-specific code for
> obtaining information about the tracee. For some architectures, this
> requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
> argument and return value.
>
> PTRACE_GET_SYSCALL_INFO returns the following structure:
>
> struct ptrace_syscall_info {
>    __u8 op; /* 0 for entry, 1 for exit */

Please consider adding another op for a seccomp stop.

>    __u8 __pad0[7];
>    union {
>        struct {
>            __u64 nr;
>            __u64 ip;
>            __u64 args[6];
>            __u8 is_compat;
>            __u8 __pad1[7];
>        } entry_info;
>        struct {
>            __s64 rval;
>            __u8 is_error;
>            __u8 __pad2[7];
>        } exit_info;
>    };
> };
>
> The structure was chosen according to [2], except for two changes.
> First: instead of an arch field with a value of AUDIT_ARCH_*, a boolean
> is_compat value is returned, because a) not all arches have an AUDIT_ARCH_*
> defined for them, b) the tracer already knows what *arch* it is running on,
> but it does not know whether the tracee/syscall is in compat mode or not.

I don’t like this for a few reasons:

1. A 32-bit tracer can’t readily tell what is_compat == 0 means.

2. There is no actual guarantee that there are only two syscall
architectures available.  In fact, I think that arm64 is seriously
considering adding a third.  x86 ought to have three, but, for
arguably dubious historical reasons, it only has two, and x32 is
distinguished only by nr.

3. Your patch will be a whole lot shorter if you use
syscall_get_arch().  You'd have to add syscall_get_arch()
implementations for the remaining architectures, but that's still less
code.

> Second: a boolean is_error value is added to rval. This way the tracer can
> more reliably distinguish a return value from an error value.

Sounds reasonable to me.

Also, maybe use the extra parameter to ptrace to have userspace pass
in the size of the structure so that more fields can be added later if
needed.

  parent reply	other threads:[~2018-11-07 20:45 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07  3:27 Elvira Khabirova
2018-11-07 11:21 ` Oleg Nesterov
2018-11-07 14:06   ` Andy Lutomirski
2018-11-07 16:44     ` Oleg Nesterov
2018-11-07 20:02       ` Elvira Khabirova
2018-11-08  9:16         ` Oleg Nesterov
2018-11-07 20:20       ` Andy Lutomirski
2018-11-08 14:33         ` Oleg Nesterov
2018-11-07 16:12   ` Dmitry V. Levin
2018-11-07 15:50 ` Dmitry V. Levin
2018-11-07 20:44 ` Andy Lutomirski [this message]
2018-11-09  3:13   ` [PATCH 00/13] Prepare for PTRACE_GET_SYSCALL_INFO Dmitry V. Levin
2018-11-09  3:15     ` [PATCH 01/13] Move EM_HEXAGON to uapi/linux/elf-em.h Dmitry V. Levin
2018-11-09  3:15     ` [PATCH 02/13] elf-em.h: add EM_ARC Dmitry V. Levin
2018-11-09 14:20       ` Alexey Brodkin
2018-11-09 16:41       ` Vineet Gupta
2018-11-09 21:44         ` Dmitry V. Levin
2018-11-09 23:33           ` [PATCH 02/13 v2] Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h Dmitry V. Levin
2018-11-09 23:42             ` Vineet Gupta
2018-11-09  3:15     ` [PATCH 03/13] elf-em.h: add EM_NDS32 Dmitry V. Levin
2018-11-09  3:15     ` [PATCH 04/13] elf-em.h: add EM_XTENSA Dmitry V. Levin
2018-11-09  6:45       ` Max Filippov
2018-11-09  3:15     ` [PATCH 05/13] m68k: define syscall_get_arch() Dmitry V. Levin
2018-11-09  3:16     ` [PATCH 06/13] arc: " Dmitry V. Levin
2018-11-09 14:22       ` Alexey Brodkin
2018-11-09 15:17         ` Andy Lutomirski
2018-11-09 15:27           ` Alexey Brodkin
2018-11-09 15:56             ` Andy Lutomirski
2018-11-09 16:11               ` Alexey Brodkin
2018-11-09 16:35                 ` Andy Lutomirski
2018-11-09 23:33                   ` [PATCH 06/13 v2] " Dmitry V. Levin
2018-11-09 23:39                     ` Vineet Gupta
2018-11-09 23:54                       ` [PATCH 06/13 v3] " Dmitry V. Levin
2018-11-10  0:06                         ` Vineet Gupta
2018-11-09 16:50       ` [PATCH 06/13] " Vineet Gupta
2018-11-09 19:03         ` Andy Lutomirski
2018-11-09 19:13           ` Vineet Gupta
2018-11-09  3:16     ` [PATCH 07/13] c6x: " Dmitry V. Levin
2018-11-10  2:01       ` [PATCH 07/13 v2] " Dmitry V. Levin
2018-11-09  3:16     ` [PATCH 08/13] h8300: " Dmitry V. Levin
2018-11-09  3:16     ` [PATCH 09/13] hexagon: " Dmitry V. Levin
2018-11-09  3:16     ` [PATCH 10/13] nds32: " Dmitry V. Levin
2018-11-10  2:01       ` [PATCH 10/13 v2] " Dmitry V. Levin
2018-11-09  3:17     ` [PATCH 11/13] nios2: " Dmitry V. Levin
2018-11-09  3:17     ` [PATCH 12/13] riscv: " Dmitry V. Levin
2018-11-09  6:59       ` David Abdurachmanov
2018-11-09 22:28         ` Dmitry V. Levin
2018-11-10  5:12           ` David Abdurachmanov
2018-11-10  9:27           ` Andreas Schwab
2018-11-09 18:45       ` Palmer Dabbelt
2018-11-09 21:31         ` Dmitry V. Levin
2018-11-09 22:48           ` [PATCH 12/13 v2] " Dmitry V. Levin
2018-11-11 21:21             ` Palmer Dabbelt
2018-11-09  3:17     ` [PATCH 13/13] xtensa: " Dmitry V. Levin
2018-11-09  6:48       ` Max Filippov
2018-11-09  6:06     ` [PATCH 00/13] Prepare for PTRACE_GET_SYSCALL_INFO Andy Lutomirski
2018-11-10 14:09     ` [PATCH 14/13] Move EM_UNICORE to uapi/linux/elf-em.h Dmitry V. Levin
2018-11-10 14:10     ` [PATCH 15/13] unicore32: define syscall_get_arch() Dmitry V. Levin
2018-11-13  3:38   ` [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request Dmitry V. Levin
2018-11-20  0:11   ` [PATCH v2 00/15] Prepare for PTRACE_GET_SYSCALL_INFO Dmitry V. Levin
2018-11-20  0:14     ` [PATCH v2 01/15] Move EM_HEXAGON to uapi/linux/elf-em.h Dmitry V. Levin
2018-11-20  0:14     ` [PATCH v2 02/15] Move EM_ARCOMPACT and EM_ARCV2 " Dmitry V. Levin
2018-11-20  0:14     ` [PATCH v2 03/15] Move EM_UNICORE " Dmitry V. Levin
2018-11-20  0:15     ` [PATCH v2 04/15] elf-em.h: add EM_NDS32 Dmitry V. Levin
2018-11-20  0:15     ` [PATCH v2 05/15] elf-em.h: add EM_XTENSA Dmitry V. Levin
2018-11-20  0:15     ` [PATCH v2 06/15] m68k: define syscall_get_arch() Dmitry V. Levin
2018-12-02 10:29       ` Geert Uytterhoeven
2018-12-03  0:24         ` Dmitry V. Levin
2018-12-03  7:36           ` Geert Uytterhoeven
2018-11-20  0:15     ` [PATCH v2 07/15] arc: " Dmitry V. Levin
2018-11-20  0:15     ` [PATCH v2 08/15] c6x: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 09/15] h8300: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 10/15] hexagon: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 11/15] nds32: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 12/15] nios2: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 13/15] riscv: " Dmitry V. Levin
2018-11-20  0:16     ` [PATCH v2 14/15] unicore32: " Dmitry V. Levin
2018-11-20  0:17     ` [PATCH v2 15/15] xtensa: " Dmitry V. Levin
2018-11-20 20:26     ` [PATCH v2 00/15] Prepare for PTRACE_GET_SYSCALL_INFO Paul Moore
     [not found]     ` <20181121004422.GA29053@altlinux.org>
     [not found]       ` <20181121184004.jro532jopnbmru2m@pburton-laptop>
2018-11-21 19:00         ` [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument Dmitry V. Levin
2018-11-21 19:14           ` [PATCH] mips: fix mips_get_syscall_arg o32 check Dmitry V. Levin
2018-11-21 19:23             ` Paul Burton
2018-11-21 19:35           ` [PATCH v2 16/15 v2] syscall_get_arch: add "struct task_struct *" argument Dmitry V. Levin
2018-11-21 19:45             ` Paul Burton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrV1v-DPRfDRwiH=xn29bxWxiHdZtAH1nw=dsmDtnT0YGQ@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=esyr@redhat.com \
    --cc=ldv@altlinux.org \
    --cc=lineprinter@altlinux.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=strace-devel@lists.strace.io \
    --subject='Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).