All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
To: "Mickaël Salaün" <mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>,
	Jeff Dike <jdike-OPE4K8JWMJJBDgjK7y7TUQ@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"H . Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>,
	x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>,
	Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>,
	Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>,
	Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>,
	Thomas Meyer <thomas-VsYtu1Qij5c@public.gmane.org>,
	Nicolas Iooss <nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>,
	Anton Ivanov <aivanov-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	user-mode-linux-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	user-mode-linux-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Meredydd Luff <meredydd-zPN50pYk8eUaUu29zAJCuw@public.gmane.org>,
	David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH v1 1/4] um: Fix ptrace GETREGS/SETREGS bugs
Date: Mon, 21 Dec 2015 10:00:45 +0100	[thread overview]
Message-ID: <5677BFBD.3090200@nod.at> (raw)
In-Reply-To: <5677BD23.4060602-WFhQfpSGs3bR7s880joybQ@public.gmane.org>

[sending the mail again, Thunderbird crashed :-\]

Am 21.12.2015 um 09:49 schrieb Mickaël Salaün:
> 
> On 21/12/2015 01:20, Richard Weinberger wrote:
>> Am 21.12.2015 um 01:03 schrieb Mickaël Salaün:
>>> diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
>>> index 1683b8e..65f0d1a 100644
>>> --- a/arch/um/kernel/skas/syscall.c
>>> +++ b/arch/um/kernel/skas/syscall.c
>>> @@ -7,6 +7,7 @@
>>>  #include <linux/ptrace.h>
>>>  #include <kern_util.h>
>>>  #include <sysdep/ptrace.h>
>>> +#include <sysdep/ptrace_user.h>
>>>  #include <sysdep/syscalls.h>
>>>  #include <os.h>
>>>  
>>> @@ -16,12 +17,16 @@ void handle_syscall(struct uml_pt_regs *r)
>>>  	long result;
>>>  	int syscall;
>>>  
>>> +	/* Save the syscall register. */
>>> +	UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
>>> +
>>>  	if (syscall_trace_enter(regs)) {
>>>  		result = -ENOSYS;
>>>  		goto out;
>>>  	}
>>>  
>>> -	syscall = get_syscall(r);
>>> +	/* Get the syscall after being potentially updated with ptrace. */
>>> +	syscall = UPT_SYSCALL_NR(r);
>>
>> Doesn't this break the support for changing syscall numbers using PTRACE_SETREGS?
> 
> The logic is unchanged except updating the UPT_SYSCALL_NR before syscall_trace_enter(). I did my last tests with the x86_32 subarchitecture and all tests (from selftest/seccomp), including PTRACE_SETREGS for syscall numbers tests, passed. However, 2 of this tests still fail for x86_64 (only).

No, the logic is different.
syscall_trace_enter(regs) enters the ptrace() path and here registers can be changed.
Hence "syscall = UPT_SYSCALL_NR(r);" will see the old syscall number.
UPT_SYSCALL_NR() returns the syscall number before the ptrace() path...

Thanks,
//richard

WARNING: multiple messages have this Message-ID (diff)
From: Richard Weinberger <richard@nod.at>
To: "Mickaël Salaün" <mic@digikod.net>, linux-kernel@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>, Jeff Dike <jdike@addtoit.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Chris Metcalf <cmetcalf@ezchip.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Andrew Morton <akpm@linux-foundation.org>,
	James Hogan <james.hogan@imgtec.com>,
	Thomas Meyer <thomas@m3y3r.de>,
	Nicolas Iooss <nicolas.iooss_linux@m4x.org>,
	Anton Ivanov <aivanov@brocade.com>,
	linux-doc@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net,
	user-mode-linux-user@lists.sourceforge.net,
	linux-api@vger.kernel.org,
	Meredydd Luff <meredydd@senatehouse.org>,
	David Drysdale <drysdale@google.com>
Subject: Re: [PATCH v1 1/4] um: Fix ptrace GETREGS/SETREGS bugs
Date: Mon, 21 Dec 2015 10:00:45 +0100	[thread overview]
Message-ID: <5677BFBD.3090200@nod.at> (raw)
In-Reply-To: <5677BD23.4060602@digikod.net>

[sending the mail again, Thunderbird crashed :-\]

Am 21.12.2015 um 09:49 schrieb Mickaël Salaün:
> 
> On 21/12/2015 01:20, Richard Weinberger wrote:
>> Am 21.12.2015 um 01:03 schrieb Mickaël Salaün:
>>> diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
>>> index 1683b8e..65f0d1a 100644
>>> --- a/arch/um/kernel/skas/syscall.c
>>> +++ b/arch/um/kernel/skas/syscall.c
>>> @@ -7,6 +7,7 @@
>>>  #include <linux/ptrace.h>
>>>  #include <kern_util.h>
>>>  #include <sysdep/ptrace.h>
>>> +#include <sysdep/ptrace_user.h>
>>>  #include <sysdep/syscalls.h>
>>>  #include <os.h>
>>>  
>>> @@ -16,12 +17,16 @@ void handle_syscall(struct uml_pt_regs *r)
>>>  	long result;
>>>  	int syscall;
>>>  
>>> +	/* Save the syscall register. */
>>> +	UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
>>> +
>>>  	if (syscall_trace_enter(regs)) {
>>>  		result = -ENOSYS;
>>>  		goto out;
>>>  	}
>>>  
>>> -	syscall = get_syscall(r);
>>> +	/* Get the syscall after being potentially updated with ptrace. */
>>> +	syscall = UPT_SYSCALL_NR(r);
>>
>> Doesn't this break the support for changing syscall numbers using PTRACE_SETREGS?
> 
> The logic is unchanged except updating the UPT_SYSCALL_NR before syscall_trace_enter(). I did my last tests with the x86_32 subarchitecture and all tests (from selftest/seccomp), including PTRACE_SETREGS for syscall numbers tests, passed. However, 2 of this tests still fail for x86_64 (only).

No, the logic is different.
syscall_trace_enter(regs) enters the ptrace() path and here registers can be changed.
Hence "syscall = UPT_SYSCALL_NR(r);" will see the old syscall number.
UPT_SYSCALL_NR() returns the syscall number before the ptrace() path...

Thanks,
//richard
--
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/


WARNING: multiple messages have this Message-ID (diff)
From: Richard Weinberger <richard@nod.at>
To: "Mickaël Salaün" <mic@digikod.net>, linux-kernel@vger.kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>, Jeff Dike <jdike@addtoit.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Chris Metcalf <cmetcalf@ezchip.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Andrew Morton <akpm@linux-foundation.org>,
	James Hogan <james.hogan@imgtec.com>,
	Thomas Meyer <thomas@m3y3r.de>,
	Nicolas Iooss <nicolas.iooss_linux@m4x.org>,
	Anton Ivanov <aivanov@brocade.com>,
	linux-doc@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net,
	user-mode-linux-user@lists.sourceforge.net,
	linux-api@vger.kernel.org,
	Meredydd Luff <meredydd@senatehouse.org>,
	David Drysdale <drysdale@google.com>
Subject: Re: [PATCH v1 1/4] um: Fix ptrace GETREGS/SETREGS bugs
Date: Mon, 21 Dec 2015 10:00:45 +0100	[thread overview]
Message-ID: <5677BFBD.3090200@nod.at> (raw)
In-Reply-To: <5677BD23.4060602@digikod.net>

[sending the mail again, Thunderbird crashed :-\]

Am 21.12.2015 um 09:49 schrieb Mickaël Salaün:
> 
> On 21/12/2015 01:20, Richard Weinberger wrote:
>> Am 21.12.2015 um 01:03 schrieb Mickaël Salaün:
>>> diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
>>> index 1683b8e..65f0d1a 100644
>>> --- a/arch/um/kernel/skas/syscall.c
>>> +++ b/arch/um/kernel/skas/syscall.c
>>> @@ -7,6 +7,7 @@
>>>  #include <linux/ptrace.h>
>>>  #include <kern_util.h>
>>>  #include <sysdep/ptrace.h>
>>> +#include <sysdep/ptrace_user.h>
>>>  #include <sysdep/syscalls.h>
>>>  #include <os.h>
>>>  
>>> @@ -16,12 +17,16 @@ void handle_syscall(struct uml_pt_regs *r)
>>>  	long result;
>>>  	int syscall;
>>>  
>>> +	/* Save the syscall register. */
>>> +	UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
>>> +
>>>  	if (syscall_trace_enter(regs)) {
>>>  		result = -ENOSYS;
>>>  		goto out;
>>>  	}
>>>  
>>> -	syscall = get_syscall(r);
>>> +	/* Get the syscall after being potentially updated with ptrace. */
>>> +	syscall = UPT_SYSCALL_NR(r);
>>
>> Doesn't this break the support for changing syscall numbers using PTRACE_SETREGS?
> 
> The logic is unchanged except updating the UPT_SYSCALL_NR before syscall_trace_enter(). I did my last tests with the x86_32 subarchitecture and all tests (from selftest/seccomp), including PTRACE_SETREGS for syscall numbers tests, passed. However, 2 of this tests still fail for x86_64 (only).

No, the logic is different.
syscall_trace_enter(regs) enters the ptrace() path and here registers can be changed.
Hence "syscall = UPT_SYSCALL_NR(r);" will see the old syscall number.
UPT_SYSCALL_NR() returns the syscall number before the ptrace() path...

Thanks,
//richard

  parent reply	other threads:[~2015-12-21  9:00 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-21  0:03 [PATCH v1 0/4] um: Add seccomp support Mickaël Salaün
2015-12-21  0:03 ` Mickaël Salaün
2015-12-21  0:03 ` Mickaël Salaün
2015-12-21  0:03 ` [PATCH v1 1/4] um: Fix ptrace GETREGS/SETREGS bugs Mickaël Salaün
2015-12-21  0:03   ` Mickaël Salaün
2015-12-21  0:03   ` Mickaël Salaün
     [not found]   ` <1450656209-2676-2-git-send-email-mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
2015-12-21  0:20     ` Richard Weinberger
2015-12-21  0:20       ` Richard Weinberger
2015-12-21  0:20       ` Richard Weinberger
2015-12-21  8:49       ` Mickaël Salaün
2015-12-21  8:56         ` Richard Weinberger
2015-12-21  8:56           ` Richard Weinberger
     [not found]         ` <5677BD23.4060602-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
2015-12-21  9:00           ` Richard Weinberger [this message]
2015-12-21  9:00             ` Richard Weinberger
2015-12-21  9:00             ` Richard Weinberger
     [not found]             ` <5677BFBD.3090200-/L3Ra7n9ekc@public.gmane.org>
2015-12-21  9:23               ` Mickaël Salaün
2015-12-21  9:23                 ` Mickaël Salaün
2015-12-21 10:13                 ` Richard Weinberger
2015-12-21 19:10                   ` Mickaël Salaün
2015-12-21  0:03 ` [PATCH v1 3/4] um: Add full asm/syscall.h support Mickaël Salaün
2015-12-21  0:03   ` Mickaël Salaün
2015-12-21  0:03   ` Mickaël Salaün
     [not found] ` <1450656209-2676-1-git-send-email-mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
2015-12-21  0:03   ` [PATCH v1 2/4] selftests/seccomp: Remove the need for HAVE_ARCH_TRACEHOOK Mickaël Salaün
2015-12-21  0:03     ` Mickaël Salaün
2015-12-21  0:03     ` Mickaël Salaün
2015-12-21  0:03   ` [PATCH v1 4/4] um: Add seccomp support Mickaël Salaün
2015-12-21  0:03     ` Mickaël Salaün
2015-12-21  0:03     ` Mickaël Salaün

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=5677BFBD.3090200@nod.at \
    --to=richard-/l3ra7n9ekc@public.gmane.org \
    --cc=aivanov-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org \
    --cc=corbet-T1hC0tSOHrs@public.gmane.org \
    --cc=drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
    --cc=james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org \
    --cc=jdike-OPE4K8JWMJJBDgjK7y7TUQ@public.gmane.org \
    --cc=keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org \
    --cc=meredydd-zPN50pYk8eUaUu29zAJCuw@public.gmane.org \
    --cc=mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org \
    --cc=mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org \
    --cc=nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org \
    --cc=shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    --cc=thomas-VsYtu1Qij5c@public.gmane.org \
    --cc=user-mode-linux-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    --cc=user-mode-linux-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    --cc=wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.