Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Peter Collingbourne <pcc@google.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Tejas Belagod <Tejas.Belagod@arm.com>
Subject: Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis
Date: Fri, 25 Jun 2021 14:53:50 +0100	[thread overview]
Message-ID: <20210625135350.GD20835@arm.com> (raw)
In-Reply-To: <20210625123959.GB3170@willie-the-truck>

On Fri, Jun 25, 2021 at 01:39:59PM +0100, Will Deacon wrote:
> On Fri, Jun 25, 2021 at 01:01:37PM +0100, Catalin Marinas wrote:
> > So we can document that the mode requested by the app is an indication,
> > the system may change it to another value (and back-port documentation
> > to 5.10). If we get a request from developers to honour a specific mode,
> > we can add a new PR_MTE_TCF_EXACT bit or something but it's not
> > essential we do it now.
> > 
> > So if we allow the kernel to change the user requested mode (via sysfs),
> > I think we still have two more issues to clarify:
> > 
> > 1. Do we allow only "upgrade" (for some meaning of this) or sysfs can
> >    downgrade to a less strict mode. I'd go for upgrade here to a
> >    stricter check as in Peter's patch.
> > 
> > 2. Should the sysfs upgrade the PR_MTE_TCF_NONE? _MTAG_ENABLE does that,
> >    so I'd say yes.
> > 
> > Any other thoughts are welcome.
> 
> As I mentioned before, I think the sysfs interface should offer:
> 
> 	"task"	: Honour whatever the task has asked for (default)
> 	"async" : Force async on this CPU
> 	"sync"  : Force sync on this CPU
> 
> I don't think we should upgrade PR_MTE_TCF_NONE unless we also have a "none"
> option in here. I originally suggested that, but in hindsight it feels like
> a bad idea because a task could SIGILL on migration. So what we're saying is
> that PR_MTE_TCF_SYNC and PR_MTE_TCF_ASYNC will always enable MTE on success,
> but the reporting mode is a hint.
> 
> I don't think upgrade/downgrade makes a lot of sense given that the sysfs
> controls can be changed at any point in time. It should just be an override.

The problem with sysfs is that it's global, so it assumes that any
process has the same needs. The _MTAG_ENABLE glibc tunable at least can
be set per process.

> This means that we can force async for CPUs where sync mode is horribly
> slow, whilst honouring the task's request on CPUs which are better
> implemented.

This may hamper debugging on, for example, a system where the root
configured the modes for CPUs and a normal user wants to use MTE to
identify access bugs. Another case is some service that wants tightened
security from MTE irrespective of the performance.

The slight downside of the "upgrade" mode assumes that the user is aware
that async is the fastest and asks for this unless it has specific
needs. Of course, we can also extend the interface to "sync-force" or
"sync-upgrade" etc. but I think it's over-engineered.

-- 
Catalin

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