[PATCH v1 4/4] arm64/mte: Add userspace interface for enabling asymmetric mode

[Peter Collingbourne]

On Thu, Mar 3, 2022 at 2:41 AM Catalin Marinas <catalin.marinas at arm.com> wrote:
>
> On Wed, Mar 02, 2022 at 12:58:48PM -0800, Evgenii Stepanov wrote:
> > On Wed, Mar 2, 2022 at 11:33 AM Mark Brown <broonie at kernel.org> wrote:
> > > On Wed, Mar 02, 2022 at 10:44:31AM -0800, Evgenii Stepanov wrote:
> > > > On Wed, Mar 2, 2022 at 5:10 AM Mark Brown <broonie at kernel.org> wrote:
> > > > > On Wed, Mar 02, 2022 at 11:44:53AM +0000, Catalin Marinas wrote:
> > > > > > On Tue, Mar 01, 2022 at 04:52:01PM -0800, Evgenii Stepanov wrote:
> > >
> > > > > > > Extending PR_MTE_TCF_MASK seems bad for backward compatibility. User
> > > > > > > code may do "flags =& ~PR_MTE_TCF_MASK" to disable MTE; when compiled
> > > > > > > against an old version of the header this would fail to remove the ASYMM
> > > > > > > bit.
> > >
> > > > > > But if the app is compiled against an old version, it wouldn't set
> > > > > > MTE_CTRL_TCF_ASYMM either as it doesn't have the definition.
> > >
> > > > Libraries within a single process can be built against different
> > > > header versions. In our case, this is libc vs the app: we expect to
> > > > set all 3 mode bits when an app asks for "async" to enable the
> > > > mte_tcf_preferred logic. Even if the app is built against an older NDK
> > > > and unaware of the Asymm mode existence!
> > >
> > > I can't see how we can resolve that case in the kernel except by adding
> > > a specific call to disable all MTE modes which would obviously only be
> > > useful for future proofing given that no existing applications would
> > > support it.
> >
> > One option would be to introduce a new, future-proofed prctl with a
> > wider mask, and throw in a few extra reserved bits just in case. Then
> > have the legacy prctl always clear MTE_CTRL_TCF_ASYMM.
>
> If this problem is real, we can easily tweak the current proposal so the
> that ASYMM can only be set *if* both ASYNC and SYNC are set. IOW, it's
> not a specific mode an app requests on its own but rather something the
> kernel may choose via the preferred mode if the app opted in. We still
> introduce a new bit for ASYMM but not change the mask.

As discussed out-of-band, I've never really liked this API style of
trying to cram everything into a single prctl(), not least because of
these compatibility concerns. Evgenii proposed a prctl() with a wider
mask, but I think this sort of approach just kicks the can down the
road (well, maybe for *this* feature we can't expect there to be too
many new modes added, but can we say the same for everything else in
tagged_addr_ctrl?). And an attempt to guess the user intent from which
bits are set seems prone to failure and unnecessarily restrictive. It
seems far more preferable to have a separate prctl() control for the
TCF bits and start working towards splitting out the other bits to
their own prctl()s. Then any user code that manipulates these bits
will "naturally" work. For compatibility the TCF bits would still be
accessible via the existing prctl(), but an attempt to set TCF via
this prctl() would clear the ASYMM bit.

The only downside that I've seen mentioned is that we will end up
issuing more syscalls on process startup which could harm performance.
But is this really significant? In the past I've measured the overhead
of a syscall to be on the order of a few hundred nanoseconds on recent
arm64 platforms. Starting a new process is expected to be a relatively
expensive operation anyway so I wouldn't expect this to make much
difference. It also seems like prioritizing performance here is the
wrong tradeoff for a kernel with a stable userspace ABI. If it is
really a concern I think I would prefer that we add a way to issue
multiple syscalls with a single kernel entry instead (this is
something that I've wanted for other use cases as well).

Peter