linux-kernel - Re: [PATCH 4/4] MIPS/ptrace: Add PTRACE_SET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGXu5j+s3bBGX51Ufo9rJZ2RVEPk90J1KEEF-hVmwzR=_tuccA@mail.gmail.com>
Date:   Mon, 14 Aug 2017 10:55:20 -0700
From:   Kees Cook <keescook@...omium.org>
To:     James Hogan <james.hogan@...tec.com>
Cc:     Linux MIPS Mailing List <linux-mips@...ux-mips.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>
Subject: Re: [PATCH 4/4] MIPS/ptrace: Add PTRACE_SET_SYSCALL operation

On Mon, Aug 14, 2017 at 2:41 AM, James Hogan <james.hogan@...tec.com> wrote:
> On Fri, Aug 11, 2017 at 03:23:34PM -0700, Kees Cook wrote:
>> On Fri, Aug 11, 2017 at 1:56 PM, James Hogan <james.hogan@...tec.com> wrote:
>> > 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.
>>
>> Wouldn't the sycall be reloaded, so no spoofing could occur?
>
> The case I was thinking of was:
> - PTRACE_POKEUSR v0 = __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).

IIUC, the issue is that v0 holds syscall on entry and syscall return
on exit. Isn't it possible to rework all the entry logic to examine
only thread_info->syscall and ignore v0 during the ptrace and seccomp
events? i.e. SET_SYSCALL can modify ti->syscall, and only if it goes
to -1 only then will v0 be examined for a result? (If I'm reading
scall32-o32.S, I think this means loading the new syscall from
thread_info instead of registers after syscall_trace_enter.)

If that is possible, it doesn't have to happen in this patch,
obviously. Incremental is fine. :)

>> 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,

Sure thing, thanks!

-Kees

-- 
Kees Cook
Pixel Security