Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i36794607:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-624-g7714e4406d-fm-20230801.001-g7714e440
Mime-Version: 1.0
Message-Id: <786b2d02-f649-4c5f-ae9a-ed2228e4a3fb@app.fastmail.com>
In-Reply-To: <202308101209.45CF7C6F80@keescook>
References: <20230804071045.never.134-kees@kernel.org>
 <f34c11eb-89b5-48a5-bd24-c215083575a5@app.fastmail.com>
 <202308101209.45CF7C6F80@keescook>
Date:   Thu, 10 Aug 2023 22:10:08 +0200
From:   "Arnd Bergmann" <arnd@kernel.org>
To:     "Kees Cook" <keescook@chromium.org>
Cc:     "Russell King" <linux@armlinux.org.uk>,
        "Lecopzer Chen" <lecopzer.chen@mediatek.com>,
        "Oleg Nesterov" <oleg@redhat.com>,
        linux-arm-kernel@lists.infradead.org,
        "Linus Walleij" <linus.walleij@linaro.org>,
        "Russell King" <rmk+kernel@armlinux.org.uk>,
        linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH] ARM: ptrace: Restore syscall skipping and restart while tracing
Content-Type: text/plain
Precedence: bulk

On Thu, Aug 10, 2023, at 21:32, Kees Cook wrote:
> On Wed, Aug 09, 2023 at 09:47:24PM +0200, Arnd Bergmann wrote:
>
>> If the local_restart code has to store the syscall number
>> for an EABI-only kernel, wouldn't it have to also do this
>> for a kernel with OABI-only or OABI_COMPAT support?
>
> This is the part I wasn't sure about. Initially I was thinking it didn't
> matter because it's only a problem for a seccomp tracer, but I realize
> it might be exposed to a PTRACE tracer too. I was only able to test with
> EABI since seccomp is disabled for OABI_COMPAT.
>
> Anyway, syscall restart is done this way:
>
>         movlt   scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE)
>
> Can a EABI call restart an OABI syscall? I think so?

There are very few differences between oabi and eabi syscalls, I
think it basically comes down to 

 - the syscall number, and register in which it is passed to the kernel
 - a few syscalls that exist for OABI backward compatibility and were
   deprecated before EABI was added
 - a few syscalls that pass a struct with different alignment rules
 - epoll_wait() uses a runtime check for the output format

It also seems like the __NR_restart_syscall path is only relevant
for syscalls using restart_block for restarting, and that means
it's only poll(), futex(), nanosleep(), clock_nanosleep() and their
time64 counterparts. All of these are handled by the same entry
points for OABI and EABI, i.e. there is no overlap with the
exceptions above. Crucially, epoll does not use restart_block,
unlike poll().

> So maybe we just need to add:
>
> 	str     scno, [tsk, #TI_ABI_SYSCALL]    @ store scno for syscall restart
>
> after that instead of moving it like I did originally?

Yes, I think that works!

For pure EABI and pure OABI kernels, this just does the right thing,
storing a plain __NR_restart_syscall in the field without an ABI
marker. For an OABI compat task running on an EABI kernel, it will
call the EABI version of restart_syscall(), but that is exactly
the same as the OABI version, as shown above.

    Arnd