All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ryan Houdek <houdek.ryan@fex-emu.org>
To: Arnd Bergmann <arnd@kernel.org>
Cc: "Amanieu d'Antras" <amanieu@gmail.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Steven Price <steven.price@arm.com>,
	David Laight <David.Laight@aculab.com>,
	Mark Brown <broonie@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v4 8/8] arm64: Allow 64-bit tasks to invoke compat syscalls
Date: Tue, 18 May 2021 16:52:10 -0700	[thread overview]
Message-ID: <CAPpY1unuaw_GzbAn=Qugv==2zyG71g+xdxD-6HdsTeAoNSL6og@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a0=iSUBu5GnuWoxEjB0Hpd3iHeVwe2Njfj6x64hoJA5oA@mail.gmail.com>

On Tue, May 18, 2021 at 6:03 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, May 18, 2021 at 11:06 AM Amanieu d'Antras <amanieu@gmail.com> wrote:
> >
> > Setting bit 31 in x8 when performing a syscall will do the following:
> > - The remainder of x8 is treated as a compat syscall number and is used
> >   to index the compat syscall table.
> > - in_compat_syscall will return true for the duration of the syscall.
> > - VM allocations performed by the syscall will be located in the lower
> >   4G of the address space.
> > - Interrupted syscalls are properly restarted as compat syscalls.
> > - Seccomp will treats the syscall as having AUDIT_ARCH_ARM instead of
> >   AUDIT_ARCH_AARCH64. This affects the arch value seen by seccomp
> >   filters and reported by SIGSYS.
> > - PTRACE_GET_SYSCALL_INFO also treats the syscall as having
> >   AUDIT_ARCH_ARM. Recent versions of strace will correctly report the
> >   system call name and parameters when an AArch64 task mixes 32-bit and
> >   64-bit syscalls.
> >
> > Previously, setting bit 31 of the syscall number would always cause the
> > sygscall to return ENOSYS. This allows user programs to reliably detect
> > kernel support for compat syscall by trying a simple syscall such as
> > getpid.
> >
> > The AArch32-private compat syscalls (__ARM_NR_compat_*) are not exposed
> > through this interface. These syscalls do not make sense in the context
> > of an AArch64 task.
> >
> > Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
> > Co-developed-by: Ryan Houdek <Houdek.Ryan@fex-emu.org>
> > Signed-off-by: Ryan Houdek <Houdek.Ryan@fex-emu.org>
>
> I'm still undecided about this approach. It is an easy way to expose the 32-bit
> ABIs, it mostly copies what x86-64 already does with 32-bit syscalls and
> it doesn't expose a lot of attack surface that isn't already exposed to normal
> 32-bit tasks running compat mode.
>
> On the other hand, exposing the entire aarch32 syscall set seems both
> too broad and not broad enough: Half of the system calls behave the
> exact same way in native and compat mode, so they wouldn't need to
> be exposed like this, a lot of others are trivially emulated in user space
> by calling the native versions. The syscalls that are actually hard to do
> such as ioctl() or the signal handling will work for aarch32 emulation, but
> they are still insufficient to correctly emulate other 32-bit architectures
> that have a slightly different ABI. This means the interface is a fairly good
> fit for Tango, but much less so for FEX.

You are correct here. This meshes perfectly for Tango's use case. Where
the syscalls will match perfectly for their aarch32->aarch64->compat syscall
path.

For FEX's use case, we still need to deal with any data structure that
doesn't match between the 32-bit x86 to compat syscall boundary. While
x86->compat will require significantly less fixups than x86->aarch64, it
is still likely to have some structure differences that need fixing.

>
> It's also worth pointing out that this approach has a few things in common
> with Yury's ilp32 tree at https://github.com/norov/linux/tree/ilp32-5.2
> Unlike the x86 x32 mode, that port however does not allow calling compat
> syscalls from normal 64-bit tasks but rather keys the syscall entry point
> off the executable format., which wouldn't work here. It also uses the
> asm-generic system call numbers instead of the arm32 syscall numbers.
>
> I assume you have already considered or tried the alternative approach of
> only adding a minimal set of syscalls that are needed for the emulation.
> Having a way to limit the address space for mmap() and similar
> system calls sounds like a generally useful addition, and having an
> extended variant of ioctl() that lets you pick the target ABI (arm32, x86-32,
> ...) on supported drivers would probably be better for FEX. Can you
> explain the tradeoffs that led you towards duplicating the syscall
> entry points instead?

I'm likely to not be very concise here. There are many paper cuts for
any route taken here.
For me, this one is the best route because of its ability to future proof
for any upcoming additions to syscalls.

If we were wanting to take a path of duplicating a bunch of compat
syscalls to work from the 64-bit side. We would first need to start with
around nine syscalls that are causing immediate problems.
mmap/mmap2, mremap, shmat, ioctl, recvmsg, recvmmsg, getdents,
and getdents64.
So we could carve those out, adding effectively the same memory
handling code that is being added here[1]. Do the ML dance to upstream.
We now have nine-ish syscalls that are added specifically for userspace
compatibility layers.
That's already beginning to have a bad smell.

Next step is a couple months down the line, someone adds a super cool
syscall that say, allocates memory that is secure over infiniband and flushes
to persistence on hibernate. Neato. Oops, this is allocating memory, and
since FEX is tracking very close to upstream kernel syscall support, we now
need to add yet another syscall that handles the compat version in a
64-bit space.
Or maybe it appends to a linked list of secure memory regions. Only visible as
the head of the list (Hello robust futexes).

See what I mean? Exposing the 32-bit compat syscalls removes the burden of
now needing to think about every syscall in a context of 32-bit,
64-bit, 32-bit on 64-bit.
Also removes the burden that I then need to come back and pester the ML
every single time with new patchsets adding syscalls only for compat layers.

And I'm all about removing unnecessary burden

[1]Side grade, personality flags won't be pretty here, FEX lives in a
mixed syscall world and doesn't want only one or the other working.
FEX does a bunch of stuff in the background and a personality flag
would be hard to work around whenever we need to do some
memory allocations, or file system handling, or its own 64-bit ioctl
handling. Just not very versatile.
FEX is already allocating all 48/52-bit VA, breaking ASLR and stack
growing, as a partial workaround here.

>
>          Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Ryan Houdek <houdek.ryan@fex-emu.org>
To: Arnd Bergmann <arnd@kernel.org>
Cc: "Amanieu d'Antras" <amanieu@gmail.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	 Steven Price <steven.price@arm.com>,
	David Laight <David.Laight@aculab.com>,
	 Mark Brown <broonie@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v4 8/8] arm64: Allow 64-bit tasks to invoke compat syscalls
Date: Tue, 18 May 2021 16:52:10 -0700	[thread overview]
Message-ID: <CAPpY1unuaw_GzbAn=Qugv==2zyG71g+xdxD-6HdsTeAoNSL6og@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a0=iSUBu5GnuWoxEjB0Hpd3iHeVwe2Njfj6x64hoJA5oA@mail.gmail.com>

On Tue, May 18, 2021 at 6:03 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, May 18, 2021 at 11:06 AM Amanieu d'Antras <amanieu@gmail.com> wrote:
> >
> > Setting bit 31 in x8 when performing a syscall will do the following:
> > - The remainder of x8 is treated as a compat syscall number and is used
> >   to index the compat syscall table.
> > - in_compat_syscall will return true for the duration of the syscall.
> > - VM allocations performed by the syscall will be located in the lower
> >   4G of the address space.
> > - Interrupted syscalls are properly restarted as compat syscalls.
> > - Seccomp will treats the syscall as having AUDIT_ARCH_ARM instead of
> >   AUDIT_ARCH_AARCH64. This affects the arch value seen by seccomp
> >   filters and reported by SIGSYS.
> > - PTRACE_GET_SYSCALL_INFO also treats the syscall as having
> >   AUDIT_ARCH_ARM. Recent versions of strace will correctly report the
> >   system call name and parameters when an AArch64 task mixes 32-bit and
> >   64-bit syscalls.
> >
> > Previously, setting bit 31 of the syscall number would always cause the
> > sygscall to return ENOSYS. This allows user programs to reliably detect
> > kernel support for compat syscall by trying a simple syscall such as
> > getpid.
> >
> > The AArch32-private compat syscalls (__ARM_NR_compat_*) are not exposed
> > through this interface. These syscalls do not make sense in the context
> > of an AArch64 task.
> >
> > Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
> > Co-developed-by: Ryan Houdek <Houdek.Ryan@fex-emu.org>
> > Signed-off-by: Ryan Houdek <Houdek.Ryan@fex-emu.org>
>
> I'm still undecided about this approach. It is an easy way to expose the 32-bit
> ABIs, it mostly copies what x86-64 already does with 32-bit syscalls and
> it doesn't expose a lot of attack surface that isn't already exposed to normal
> 32-bit tasks running compat mode.
>
> On the other hand, exposing the entire aarch32 syscall set seems both
> too broad and not broad enough: Half of the system calls behave the
> exact same way in native and compat mode, so they wouldn't need to
> be exposed like this, a lot of others are trivially emulated in user space
> by calling the native versions. The syscalls that are actually hard to do
> such as ioctl() or the signal handling will work for aarch32 emulation, but
> they are still insufficient to correctly emulate other 32-bit architectures
> that have a slightly different ABI. This means the interface is a fairly good
> fit for Tango, but much less so for FEX.

You are correct here. This meshes perfectly for Tango's use case. Where
the syscalls will match perfectly for their aarch32->aarch64->compat syscall
path.

For FEX's use case, we still need to deal with any data structure that
doesn't match between the 32-bit x86 to compat syscall boundary. While
x86->compat will require significantly less fixups than x86->aarch64, it
is still likely to have some structure differences that need fixing.

>
> It's also worth pointing out that this approach has a few things in common
> with Yury's ilp32 tree at https://github.com/norov/linux/tree/ilp32-5.2
> Unlike the x86 x32 mode, that port however does not allow calling compat
> syscalls from normal 64-bit tasks but rather keys the syscall entry point
> off the executable format., which wouldn't work here. It also uses the
> asm-generic system call numbers instead of the arm32 syscall numbers.
>
> I assume you have already considered or tried the alternative approach of
> only adding a minimal set of syscalls that are needed for the emulation.
> Having a way to limit the address space for mmap() and similar
> system calls sounds like a generally useful addition, and having an
> extended variant of ioctl() that lets you pick the target ABI (arm32, x86-32,
> ...) on supported drivers would probably be better for FEX. Can you
> explain the tradeoffs that led you towards duplicating the syscall
> entry points instead?

I'm likely to not be very concise here. There are many paper cuts for
any route taken here.
For me, this one is the best route because of its ability to future proof
for any upcoming additions to syscalls.

If we were wanting to take a path of duplicating a bunch of compat
syscalls to work from the 64-bit side. We would first need to start with
around nine syscalls that are causing immediate problems.
mmap/mmap2, mremap, shmat, ioctl, recvmsg, recvmmsg, getdents,
and getdents64.
So we could carve those out, adding effectively the same memory
handling code that is being added here[1]. Do the ML dance to upstream.
We now have nine-ish syscalls that are added specifically for userspace
compatibility layers.
That's already beginning to have a bad smell.

Next step is a couple months down the line, someone adds a super cool
syscall that say, allocates memory that is secure over infiniband and flushes
to persistence on hibernate. Neato. Oops, this is allocating memory, and
since FEX is tracking very close to upstream kernel syscall support, we now
need to add yet another syscall that handles the compat version in a
64-bit space.
Or maybe it appends to a linked list of secure memory regions. Only visible as
the head of the list (Hello robust futexes).

See what I mean? Exposing the 32-bit compat syscalls removes the burden of
now needing to think about every syscall in a context of 32-bit,
64-bit, 32-bit on 64-bit.
Also removes the burden that I then need to come back and pester the ML
every single time with new patchsets adding syscalls only for compat layers.

And I'm all about removing unnecessary burden

[1]Side grade, personality flags won't be pretty here, FEX lives in a
mixed syscall world and doesn't want only one or the other working.
FEX does a bunch of stuff in the background and a personality flag
would be hard to work around whenever we need to do some
memory allocations, or file system handling, or its own 64-bit ioctl
handling. Just not very versatile.
FEX is already allocating all 48/52-bit VA, breaking ASLR and stack
growing, as a partial workaround here.

>
>          Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-05-18 23:52 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18  9:06 [RESEND PATCH v4 0/8] arm64: Allow 64-bit tasks to invoke compat syscalls Amanieu d'Antras
2021-05-18  9:06 ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 1/8] mm: Add arch_get_mmap_base_topdown macro Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 2/8] hugetlbfs: Use arch_get_mmap_* macros Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 3/8] mm: Support mmap_compat_base with the generic layout Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 4/8] arm64: Separate in_compat_syscall from is_compat_task Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 5/8] arm64: mm: Use HAVE_ARCH_COMPAT_MMAP_BASES Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 6/8] arm64: Add a compat syscall flag to thread_info Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 7/8] arm64: Forbid calling compat sigreturn from 64-bit tasks Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18  9:06 ` [RESEND PATCH v4 8/8] arm64: Allow 64-bit tasks to invoke compat syscalls Amanieu d'Antras
2021-05-18  9:06   ` Amanieu d'Antras
2021-05-18 13:02   ` Arnd Bergmann
2021-05-18 13:02     ` Arnd Bergmann
2021-05-18 20:26     ` David Laight
2021-05-18 20:26       ` David Laight
2021-05-18 22:41       ` Ryan Houdek
2021-05-18 22:41         ` Ryan Houdek
2021-05-18 23:51     ` Amanieu d'Antras
2021-05-18 23:51       ` Amanieu d'Antras
2021-05-19 15:30       ` Steven Price
2021-05-19 15:30         ` Steven Price
2021-05-19 16:14         ` Amanieu d'Antras
2021-05-19 16:14           ` Amanieu d'Antras
2021-05-21  8:51           ` Steven Price
2021-05-21  8:51             ` Steven Price
2021-05-21 19:18             ` Amanieu d'Antras
2021-05-21 19:18               ` Amanieu d'Antras
2021-05-24 11:20               ` Steven Price
2021-05-24 11:20                 ` Steven Price
2021-05-24 12:38                 ` David Laight
2021-05-24 12:38                   ` David Laight
2021-05-18 23:52     ` Ryan Houdek [this message]
2021-05-18 23:52       ` Ryan Houdek

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='CAPpY1unuaw_GzbAn=Qugv==2zyG71g+xdxD-6HdsTeAoNSL6og@mail.gmail.com' \
    --to=houdek.ryan@fex-emu.org \
    --cc=David.Laight@aculab.com \
    --cc=amanieu@gmail.com \
    --cc=arnd@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=steven.price@arm.com \
    --cc=will@kernel.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.