From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752653AbdHNJlu (ORCPT ); Mon, 14 Aug 2017 05:41:50 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:12244 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752521AbdHNJls (ORCPT ); Mon, 14 Aug 2017 05:41:48 -0400 Date: Mon, 14 Aug 2017 10:41:45 +0100 From: James Hogan To: Kees Cook CC: Linux MIPS Mailing List , LKML , Ralf Baechle , Oleg Nesterov , Andy Lutomirski , Will Drewry Subject: Re: [PATCH 4/4] MIPS/ptrace: Add PTRACE_SET_SYSCALL operation Message-ID: <20170814094145.GP6973@jhogan-linux.le.imgtec.org> References: <20170811205653.21873-1-james.hogan@imgtec.com> <20170811205653.21873-5-james.hogan@imgtec.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Bl8CjiHBfOoF9JLC" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [192.168.154.110] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --Bl8CjiHBfOoF9JLC Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 11, 2017 at 03:23:34PM -0700, Kees Cook wrote: > On Fri, Aug 11, 2017 at 1:56 PM, James Hogan wro= te: > > Add a PTRACE_SET_SYSCALL ptrace operation to allow the system call to be > > cancelled independently to the value of the v0 system call number > > register. > > > > This is needed for SECCOMP_RET_TRACE when the tracer wants to cancel the > > system call, since it has to set both the system call number to -1 and > > the chosen return value, both of which reside in the same register (v0). > > The tracer should set the return value first, followed by > > PTRACE_SET_SYSCALL to set the system call number to -1. > > > > That is in contrast to the normal ptrace syscall hook which triggers the > > tracer on both entry and exit, allowing the system call to be cancelled > > during the entry hook (setting system call number register to -1, or > > optionally using PTRACE_SET_SYSCALL), separately to setting the return > > value during the exit hook. > > > > Positive values (to change the syscall that should be executed instead > > of cancelling it entirely) are explicitly disallowed at the moment. The > > same thing can be done safely already by writing the v0 system call > > number register and the argument registers, and allowing > > thread_info::syscall to be changed to a different value independently of > > the v0 register would potentially allow seccomp or the syscall trace > > events to be fooled into thinking a different system call was being > > executed. >=20 > Wouldn't the sycall be reloaded, so no spoofing could occur? The case I was thinking of was: - PTRACE_POKEUSR v0 =3D __NR_some_disallowed_syscall - PTRACE_SET_SYSCALL __NR_some_allowed_syscall syscall_get_nr() will return __NR_some_allowed_syscall, so seccomp will allow, but when syscall_trace_enter() returns to syscall_trace_entry in arch/mips/kernel/scall32-o32.S, it will reload the syscall number from v0 (i.e. __NR_some_disallowed_syscall). >=20 > Regardless, can you update > tools/testing/selftests/seccomp/seccomp_bpf.c to update or eliminate > the MIPS-only SYSCALL_NUM_RET_SHARE_REG special-case? (Or maybe it > needs to be further special-cased to split syscall-changing from > syscall-cancelling?) Sure, i'll look into that, Thanks for reviewing, Cheers James >=20 > -Kees >=20 > > > > Signed-off-by: James Hogan > > Cc: Ralf Baechle > > Cc: Oleg Nesterov > > Cc: Kees Cook > > Cc: Andy Lutomirski > > Cc: Will Drewry > > Cc: linux-mips@linux-mips.org > > --- > > arch/mips/include/uapi/asm/ptrace.h | 1 + > > arch/mips/kernel/ptrace.c | 11 +++++++++++ > > arch/mips/kernel/ptrace32.c | 11 +++++++++++ > > 3 files changed, 23 insertions(+) > > > > diff --git a/arch/mips/include/uapi/asm/ptrace.h b/arch/mips/include/ua= pi/asm/ptrace.h > > index 91a3d197ede3..23af103c4e8d 100644 > > --- a/arch/mips/include/uapi/asm/ptrace.h > > +++ b/arch/mips/include/uapi/asm/ptrace.h > > @@ -58,6 +58,7 @@ struct pt_regs { > > > > #define PTRACE_GET_THREAD_AREA 25 > > #define PTRACE_SET_THREAD_AREA 26 > > +#define PTRACE_SET_SYSCALL 27 > > > > /* Calls to trace a 64bit program from a 32bit program. */ > > #define PTRACE_PEEKTEXT_3264 0xc0 > > diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c > > index 465fc5633e61..9bf31a990c6e 100644 > > --- a/arch/mips/kernel/ptrace.c > > +++ b/arch/mips/kernel/ptrace.c > > @@ -853,6 +853,17 @@ long arch_ptrace(struct task_struct *child, long r= equest, > > ret =3D put_user(task_thread_info(child)->tp_value, dat= alp); > > break; > > > > + case PTRACE_SET_SYSCALL: > > + /* > > + * This is currently only useful to cancel the syscall = =66rom a > > + * seccomp RET_TRACE tracer. > > + */ > > + if ((long)data >=3D 0) > > + return -EINVAL; > > + task_thread_info(child)->syscall =3D -1; > > + ret =3D 0; > > + break; > > + > > case PTRACE_GET_WATCH_REGS: > > ret =3D ptrace_get_watch_regs(child, addrp); > > break; > > diff --git a/arch/mips/kernel/ptrace32.c b/arch/mips/kernel/ptrace32.c > > index 2b9260f92ccd..cca76aec9c10 100644 > > --- a/arch/mips/kernel/ptrace32.c > > +++ b/arch/mips/kernel/ptrace32.c > > @@ -287,6 +287,17 @@ long compat_arch_ptrace(struct task_struct *child,= compat_long_t request, > > (unsigned int __user *) (unsigned long)= data); > > break; > > > > + case PTRACE_SET_SYSCALL: > > + /* > > + * This is currently only useful to cancel the syscall = =66rom a > > + * seccomp RET_TRACE tracer. > > + */ > > + if ((long)data >=3D 0) > > + return -EINVAL; > > + task_thread_info(child)->syscall =3D -1; > > + ret =3D 0; > > + break; > > + > > case PTRACE_GET_THREAD_AREA_3264: > > ret =3D put_user(task_thread_info(child)->tp_value, > > (unsigned long __user *) (unsigned long= ) data); > > -- > > 2.13.2 > > >=20 >=20 >=20 > --=20 > Kees Cook > Pixel Security --Bl8CjiHBfOoF9JLC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEd80NauSabkiESfLYbAtpk944dnoFAlmRcEcACgkQbAtpk944 dnpLRw//YmbX0+UEBZF6f39Nhrsr+Tp/WqIKu+kOGT8duf3wQbB/MnKlusAsvHh9 nOD1ebs/7KGj5t22krBOrOZEAIbbo/W22BxwFcXzRdiyoXCLFI7arbr41oHOfW/7 QawLBHAUPqcH9cut07NjIuBQ7JhO1ecfBYLtkmeJtp6eXXeHWYBR597waCJq5LBM 07a3n2I/neOX7MsKDg3b4G425XFjZhAxj5a6vwep5xfY3RUSkXOXxwO/1qlIuw2s +z0S+onYY4r7aLyGrWmen3RnoEJ60jSNBLPHuQr92O/mg3bYwBjHtbqYi4+3LkEA lk5zhAMkWJnW0sixGx+q9BJ4PVhpvxQwaOTbQfdyhqlQP8rmhvM7pfLQAMiWj7kr aap0tpy0kzJdMTrw/qWeQEnxHc3lM+x9kW7gD/lVr5fnrXRkXhK6CqXAzNymlIPx 2ZFyQOBgXP8z5dlnbdlfhGuB8fZBLrfA6v59cJQGZi57CXm+07TUsk48mcG5siMx 9MdzQ5wejiG+Dw7eD3dLQ12GWJhzfOE+0ZkvnUUFs8qcT6jeG3ipZ89it7Y+FqaP T4gp0j04VDeu0FEx+kZ8LUAUw/cATyNd6ocEQ4QYroGL5ZywodRp4yhFaMrpp0My ADbx86lQKwpWNUncAGZOMwqip+ZAdBtUqDUv/ZkWlEmbJO/2ygc= =vJpe -----END PGP SIGNATURE----- --Bl8CjiHBfOoF9JLC--