Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CABV8kRxA9mXPZwtYrjbAfOfFewhABHddipccgk-LQJO+ZYu4Xg@mail.gmail.com>
 <20200703083914.GA18516@willie-the-truck>
In-Reply-To: <20200703083914.GA18516@willie-the-truck>
From:   Keno Fischer <keno@juliacomputing.com>
Date:   Fri, 3 Jul 2020 16:27:37 -0400
Message-ID: <CABV8kRwYp9cvu1b4-fhHktyX_r2QiObhadJ4pqscvuLt1BtRvg@mail.gmail.com>
Subject: Re: ptrace: seccomp: Return value when the call was already invalid
To:     Will Deacon <will@kernel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Oleg Nesterov <oleg@redhat.com>,
        Kees Cook <keescook@chromium.org>,
        Andy Lutomirski <luto@amacapital.net>,
        Will Drewry <wad@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> > Now, if we have a seccomp filter that simply does
> > SECCOMP_RET_TRACE, and a ptracer that simply
> > does PTRACE_CONT
>
> Ok, so this means that we're _skipping_ the system call, right?

If the system call were positive this would result in the system call
being executed. The notion of "skipping" the syscall is a bit odd in
this situation. Having the ptracer set the syscallno to -1 is generally
accepted as the way to do it, but what happens if the syscallno is
already -1 or negative is underspecified.

> > then the assert will fire/fail on arm64, but not on x86_64.
>
> It feels weird to me that skipping the system call has any effect on the
> tracee registers...

I think the correct way to frame it is to ask whether the behavior
matches that of the tracee in absence of the ptracer. I would argue
that if the ptracer doesn't explicitly modify register contents, then
the tracee shouldn't observe any behavior difference.

> > Interestingly, arm64 does do something different
> > if the syscall is -1 rather than -10, where early
> > in the ptrace stop it does.
> > ```
> > /* set default errno for user-issued syscall(-1) */
> > if (scno == NO_SYSCALL)
> >     regs->regs[0] = -ENOSYS;
>
> ... so I think this should be fixed too. How about the diff below?

I think the patch behavior is better overall, but I'm not sure it's ideal.
I think the biggest question is what the behavior should be here and
if we want a behavioral difference between *the syscall was -1 at entry*
and *the syscall was -1 because the ptracer wanted to skip the syscall*.
I think there is a bit of a semantic disconnect because "skipping" the
syscall is not really an operation that the ptracer has at its disposal
(unless it's using SYSEMU of course). The only thing it can do is set
the syscall to -1. However, arguably that already has semantics
(of returning -ENOSYS), so it's not at all clear to me that we should
deviate from that. Unfortunately, none of this is currently consistent
across architectures, so I think before we go changing arm64, we
should decide what we'd like to happen in theory and then see
what we can do to improve the situation without being too breaking.

Keno