[PATCH] arm64/mte: Remove asymmetric mode from the prctl() interface

[PATCH] arm64/mte: Remove asymmetric mode from the prctl() interface

[Mark Brown]

As pointed out by Evgenii Stepanov one potential issue with the new ABI for
enabling asymmetric is that if there are multiple places where MTE is
configured in a process, some of which were compiled with the old prctl.h
and some of which were compiled with the new prctl.h, there may be problems
keeping track of which MTE modes are requested. For example some code may
disable only sync and async modes leaving asymmetric mode enabled when it
intended to fully disable MTE.

In order to avoid such mishaps remove asymmetric mode from the prctl(),
instead implicitly allowing it if both sync and async modes are requested.
This should not disrupt userspace since a process requesting both may
already see a mix of sync and async modes due to differing defaults between
CPUs or changes in default while the process is running but it does mean
that userspace is unable to explicitly request asymmetric mode without
changing the system default for CPUs.

Reported-by: Evgenii Stepanov <eugenis@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: Peter Collingbourne <pcc@google.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Branislav Rankov <branislav.rankov@arm.com>
---

Just putting this proposal out there as a concrete patch, I'm not sure
that it's actually the best option however even if it's not what we end
up going for longer term we may wish to apply it just now since we're
getting near to the merge window and it means we don't add anything to
the prctl() ABI this release.  It'd mean we could still get some support
for asymmetric mode in userspace processes while giving us a cleaner
slate to figure out what we want to do with the ABI.

 Documentation/arm64/memory-tagging-extension.rst | 16 ++++++++--------
 arch/arm64/kernel/mte.c                          | 13 ++++++++++---
 arch/arm64/kernel/process.c                      |  2 --
 include/uapi/linux/prctl.h                       |  4 +---
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/Documentation/arm64/memory-tagging-extension.rst b/Documentation/arm64/memory-tagging-extension.rst
index 0ac34c301989..7e812a51e506 100644
--- a/Documentation/arm64/memory-tagging-extension.rst
+++ b/Documentation/arm64/memory-tagging-extension.rst
@@ -88,7 +88,6 @@ bit-field:
                          (ignored if combined with other options)
 - ``PR_MTE_TCF_SYNC``  - *Synchronous* tag check fault mode
 - ``PR_MTE_TCF_ASYNC`` - *Asynchronous* tag check fault mode
-- ``PR_MTE_TCF_ASYMM`` - *Asymmetric* tag check fault mode
 
 If no modes are specified, tag check faults are ignored. If a single
 mode is specified, the program will run in that mode. If multiple
@@ -149,18 +148,19 @@ default preferred mode for each CPU is ``async``.
 To allow a program to potentially run in the CPU's preferred tag
 checking mode, the user program may set multiple tag check fault mode
 bits in the ``flags`` argument to the ``prctl(PR_SET_TAGGED_ADDR_CTRL,
-flags, 0, 0, 0)`` system call. If the CPU's preferred tag checking
-mode is in the task's set of provided tag checking modes, that mode will
-be selected. Otherwise, one of the modes in the task's mode
-will be selected by the kernel from the task's mode set using the
-preference order:
+flags, 0, 0, 0)`` system call. If both synchronous and asynchronous
+modes are requested then asymmetric mode may also be selected by the
+kernel. If the CPU's preferred tag checking mode is in the task's set
+of provided tag checking modes, that mode will be selected. Otherwise,
+one of the modes in the task's mode will be selected by the kernel
+from the task's mode set using the preference order:
 
 	1. Asynchronous
 	2. Asymmetric
 	3. Synchronous
 
-If asymmetric mode is specified by the program but not supported by
-either the system or the kernel then an error will be returned.
+Note that there is no way for userspace to request multiple modes and
+also disable asymmetric mode.
 
 Initial process state
 ---------------------
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index fb777d8fea32..2b35b706a7ba 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -308,7 +308,16 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 		mte_ctrl |= MTE_CTRL_TCF_ASYNC;
 	if (arg & PR_MTE_TCF_SYNC)
 		mte_ctrl |= MTE_CTRL_TCF_SYNC;
-	if (arg & PR_MTE_TCF_ASYMM)
+
+	/*
+	 * If the system supports it and both sync and async modes are
+	 * specified then implicitly enable asymmetric mode.
+	 * Userspace could see a mix of both sync and async anyway due
+	 * to differing or changing defaults on CPUs.
+	 */
+	if (cpus_have_cap(ARM64_MTE_ASYMM) &&
+	    (arg & PR_MTE_TCF_ASYNC) &&
+	    (arg & PR_MTE_TCF_SYNC))
 		mte_ctrl |= MTE_CTRL_TCF_ASYMM;
 
 	task->thread.mte_ctrl = mte_ctrl;
@@ -338,8 +347,6 @@ long get_mte_ctrl(struct task_struct *task)
 		ret |= PR_MTE_TCF_ASYNC;
 	if (mte_ctrl & MTE_CTRL_TCF_SYNC)
 		ret |= PR_MTE_TCF_SYNC;
-	if (mte_ctrl & MTE_CTRL_TCF_ASYMM)
-		ret |= PR_MTE_TCF_ASYMM;
 
 	return ret;
 }
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 941cfa7117b9..7fa97df55e3a 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -637,8 +637,6 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg)
 	if (system_supports_mte())
 		valid_mask |= PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC \
 			| PR_MTE_TAG_MASK;
-	if (cpus_have_cap(ARM64_MTE_ASYMM))
-		valid_mask |= PR_MTE_TCF_ASYMM;
 
 	if (arg & ~valid_mask)
 		return -EINVAL;
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 4ae2b21e4066..e998764f0262 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -238,9 +238,7 @@ struct prctl_mm_map {
 # define PR_MTE_TCF_NONE		0UL
 # define PR_MTE_TCF_SYNC		(1UL << 1)
 # define PR_MTE_TCF_ASYNC		(1UL << 2)
-# define PR_MTE_TCF_ASYMM		(1UL << 19)
-# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC | \
-					 PR_MTE_TCF_ASYMM)
+# define PR_MTE_TCF_MASK		(PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC)
 /* MTE tag inclusion mask */
 # define PR_MTE_TAG_SHIFT		3
 # define PR_MTE_TAG_MASK		(0xffffUL << PR_MTE_TAG_SHIFT)
-- 
2.30.2


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

Re: [PATCH] arm64/mte: Remove asymmetric mode from the prctl() interface

[Catalin Marinas]

On Tue, Mar 08, 2022 at 02:22:30PM +0000, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index fb777d8fea32..2b35b706a7ba 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -308,7 +308,16 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
>  		mte_ctrl |= MTE_CTRL_TCF_ASYNC;
>  	if (arg & PR_MTE_TCF_SYNC)
>  		mte_ctrl |= MTE_CTRL_TCF_SYNC;
> -	if (arg & PR_MTE_TCF_ASYMM)
> +
> +	/*
> +	 * If the system supports it and both sync and async modes are
> +	 * specified then implicitly enable asymmetric mode.
> +	 * Userspace could see a mix of both sync and async anyway due
> +	 * to differing or changing defaults on CPUs.
> +	 */

Exactly, so it can't be confusing for an application.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

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

Re: [PATCH] arm64/mte: Remove asymmetric mode from the prctl() interface

[Evgenii Stepanov]

On Tue, Mar 8, 2022 at 6:22 AM Mark Brown <broonie@kernel.org> wrote:
> Just putting this proposal out there as a concrete patch, I'm not sure
> that it's actually the best option however even if it's not what we end
> up going for longer term we may wish to apply it just now since we're
> getting near to the merge window and it means we don't add anything to
> the prctl() ABI this release.  It'd mean we could still get some support
> for asymmetric mode in userspace processes while giving us a cleaner
> slate to figure out what we want to do with the ABI.

Thanks. I think this is the right approach in the short term.

Reviewed-by: Evgenii Stepanov <eugenis@google.com>

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

Re: [PATCH] arm64/mte: Remove asymmetric mode from the prctl() interface

[Will Deacon]

On Tue, Mar 08, 2022 at 02:22:30PM +0000, Mark Brown wrote:
> As pointed out by Evgenii Stepanov one potential issue with the new ABI for
> enabling asymmetric is that if there are multiple places where MTE is
> configured in a process, some of which were compiled with the old prctl.h
> and some of which were compiled with the new prctl.h, there may be problems
> keeping track of which MTE modes are requested. For example some code may
> disable only sync and async modes leaving asymmetric mode enabled when it
> intended to fully disable MTE.
> 
> In order to avoid such mishaps remove asymmetric mode from the prctl(),
> instead implicitly allowing it if both sync and async modes are requested.
> This should not disrupt userspace since a process requesting both may
> already see a mix of sync and async modes due to differing defaults between
> CPUs or changes in default while the process is running but it does mean
> that userspace is unable to explicitly request asymmetric mode without
> changing the system default for CPUs.
> 
> Reported-by: Evgenii Stepanov <eugenis@google.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Branislav Rankov <branislav.rankov@arm.com>
> ---
> 
> Just putting this proposal out there as a concrete patch, I'm not sure
> that it's actually the best option however even if it's not what we end
> up going for longer term we may wish to apply it just now since we're
> getting near to the merge window and it means we don't add anything to
> the prctl() ABI this release.  It'd mean we could still get some support
> for asymmetric mode in userspace processes while giving us a cleaner
> slate to figure out what we want to do with the ABI.
> 
>  Documentation/arm64/memory-tagging-extension.rst | 16 ++++++++--------

This doesn't apply on for-next/mte due to conflicts here ^^

Will

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