[PATCH v2 0/6] prctl.2 man page updates for Linux 5.6

[PATCH v2 0/6] prctl.2 man page updates for Linux 5.6

[Dave Martin]

A bunch of updates to the prctl(2) man page to fill in missing
prctls (mostly) up to Linux 5.6 (along with a few other tweaks and
fixes).

Patches from the v1 series [1] that have been applied or rejected
already have been dropped.

People not Cc'd on the whole series can find the whole series at
https://lore.kernel.org/linux-man/ .

Patches:

 * Patch 1 is a new (but trivial) formatting fix, unrelated to the new
   prctls.

 * Patches 2-3 relate to the speculation control prctls.  These are
   unmodified from v1, but need review.

 * Patches 4-5 relate to the arm64 prctls from v1, with reviewer
   feedback incorporated.  (See notes in the patches.)

 * Patch 6 is *draft* wording for the arm64 address tagging prctls.
   The semantics of address tagging is particularly slippery, so
   this needs discussion before merging.


[1] https://lore.kernel.org/linux-man/29a02b16-dd61-6186-1340-fcc7d5225ad0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org/T/#t


Dave Martin (6):
  prctl.2: ffix use literal hyphens when referencing kernel docs
  prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
  prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
  prctl.2: Add SVE prctls (arm64)
  prctl.2: Add PR_PAC_RESET_KEYS (arm64)
  prctl.2: Add tagged address ABI control prctls (arm64)

 man2/prctl.2 | 444 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 435 insertions(+), 9 deletions(-)

-- 
2.1.4

[PATCH v2 0/6] prctl.2 man page updates for Linux 5.6

[Dave Martin]

A bunch of updates to the prctl(2) man page to fill in missing
prctls (mostly) up to Linux 5.6 (along with a few other tweaks and
fixes).

Patches from the v1 series [1] that have been applied or rejected
already have been dropped.

People not Cc'd on the whole series can find the whole series at
https://lore.kernel.org/linux-man/ .

Patches:

 * Patch 1 is a new (but trivial) formatting fix, unrelated to the new
   prctls.

 * Patches 2-3 relate to the speculation control prctls.  These are
   unmodified from v1, but need review.

 * Patches 4-5 relate to the arm64 prctls from v1, with reviewer
   feedback incorporated.  (See notes in the patches.)

 * Patch 6 is *draft* wording for the arm64 address tagging prctls.
   The semantics of address tagging is particularly slippery, so
   this needs discussion before merging.


[1] https://lore.kernel.org/linux-man/29a02b16-dd61-6186-1340-fcc7d5225ad0@gmail.com/T/#t


Dave Martin (6):
  prctl.2: ffix use literal hyphens when referencing kernel docs
  prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
  prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
  prctl.2: Add SVE prctls (arm64)
  prctl.2: Add PR_PAC_RESET_KEYS (arm64)
  prctl.2: Add tagged address ABI control prctls (arm64)

 man2/prctl.2 | 444 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 435 insertions(+), 9 deletions(-)

-- 
2.1.4

[PATCH v2 1/6] prctl.2: ffix use literal hyphens when referencing kernel docs

[Dave Martin]

There is one case of a cross-reference to a kernel documentation
filename that uses unescaped hyphens.

To avoid misrendering, escape these as \- similarly to other
instances.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 man2/prctl.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/prctl.2 b/man2/prctl.2
index 968a75a..dc99218 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1261,7 +1261,7 @@ This parameter may enforce a read-only policy which will result in the
 call failing with the error
 .BR ENXIO .
 For further details, see the kernel source file
-.IR Documentation/admin-guide/kernel-parameters.txt .
+.IR Documentation/admin\-guide/kernel\-parameters.txt .
 .\"
 .\" prctl PR_TASK_PERF_EVENTS_DISABLE
 .TP
-- 
2.1.4

Re: [PATCH v2 1/6] prctl.2: ffix use literal hyphens when referencing kernel docs

[Michael Kerrisk (man-pages)]

On 5/27/20 11:17 PM, Dave Martin wrote:
> There is one case of a cross-reference to a kernel documentation
> filename that uses unescaped hyphens.
> 
> To avoid misrendering, escape these as \- similarly to other
> instances.
> 
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> ---
>  man2/prctl.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index 968a75a..dc99218 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1261,7 +1261,7 @@ This parameter may enforce a read-only policy which will result in the
>  call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
> -.IR Documentation/admin-guide/kernel-parameters.txt .
> +.IR Documentation/admin\-guide/kernel\-parameters.txt .
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP

Thanks, Dave. Applied.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 1/6] prctl.2: ffix use literal hyphens when referencing kernel docs

[Michael Kerrisk (man-pages)]

On 5/27/20 11:17 PM, Dave Martin wrote:
> There is one case of a cross-reference to a kernel documentation
> filename that uses unescaped hyphens.
> 
> To avoid misrendering, escape these as \- similarly to other
> instances.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  man2/prctl.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index 968a75a..dc99218 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1261,7 +1261,7 @@ This parameter may enforce a read-only policy which will result in the
>  call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
> -.IR Documentation/admin-guide/kernel-parameters.txt .
> +.IR Documentation/admin\-guide/kernel\-parameters.txt .
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP

Thanks, Dave. Applied.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

[PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Dave Martin]

Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 man2/prctl.2 | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/man2/prctl.2 b/man2/prctl.2
index dc99218..b6fb51c 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
 .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
 Sets the state of the speculation misfeature specified in
 .IR arg2 .
-Currently, the only permitted value for this argument is
+Currently, this argument must be one of:
+.RS
+.TP
 .B PR_SPEC_STORE_BYPASS
-(otherwise the call fails with the error
+speculative store bypass control, or
+.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
+.TP
+.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
+indirect branch speculation control.
+.RE
+.IP
+(Otherwise the call fails with the error
 .BR ENODEV ).
-This setting is a per-thread attribute.
+These settings are per-thread attributes.
 The
 .IR arg3
 argument is used to hand in the control value,
@@ -1235,13 +1244,16 @@ Same as
 .BR PR_SPEC_DISABLE ,
 but cannot be undone.
 A subsequent
-.B
-prctl(..., PR_SPEC_ENABLE)
+.BR prctl (\c
+.IR arg2 ,
+.BR PR_SPEC_ENABLE )
+with the same value for
+.I arg2
 will fail with the error
 .BR EPERM .
 .RE
 .IP
-Any other value in
+Any unsupported value in
 .IR arg3
 will result in the call failing with the error
 .BR ERANGE .
-- 
2.1.4

Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
>
> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Tim Chen <tim.c.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

I had also applied this patch from the email you sent earlier. I've
pushed those changes to master now.

Thanks,

Michael

> ---
>  man2/prctl.2 | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index dc99218..b6fb51c 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
>  Sets the state of the speculation misfeature specified in
>  .IR arg2 .
> -Currently, the only permitted value for this argument is
> +Currently, this argument must be one of:
> +.RS
> +.TP
>  .B PR_SPEC_STORE_BYPASS
> -(otherwise the call fails with the error
> +speculative store bypass control, or
> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> +.TP
> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> +indirect branch speculation control.
> +.RE
> +.IP
> +(Otherwise the call fails with the error
>  .BR ENODEV ).
> -This setting is a per-thread attribute.
> +These settings are per-thread attributes.
>  The
>  .IR arg3
>  argument is used to hand in the control value,
> @@ -1235,13 +1244,16 @@ Same as
>  .BR PR_SPEC_DISABLE ,
>  but cannot be undone.
>  A subsequent
> -.B
> -prctl(..., PR_SPEC_ENABLE)
> +.BR prctl (\c
> +.IR arg2 ,
> +.BR PR_SPEC_ENABLE )
> +with the same value for
> +.I arg2
>  will fail with the error
>  .BR EPERM .
>  .RE
>  .IP
> -Any other value in
> +Any unsupported value in
>  .IR arg3
>  will result in the call failing with the error
>  .BR ERANGE .
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
>
> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>

I had also applied this patch from the email you sent earlier. I've
pushed those changes to master now.

Thanks,

Michael

> ---
>  man2/prctl.2 | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index dc99218..b6fb51c 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
>  Sets the state of the speculation misfeature specified in
>  .IR arg2 .
> -Currently, the only permitted value for this argument is
> +Currently, this argument must be one of:
> +.RS
> +.TP
>  .B PR_SPEC_STORE_BYPASS
> -(otherwise the call fails with the error
> +speculative store bypass control, or
> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> +.TP
> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> +indirect branch speculation control.
> +.RE
> +.IP
> +(Otherwise the call fails with the error
>  .BR ENODEV ).
> -This setting is a per-thread attribute.
> +These settings are per-thread attributes.
>  The
>  .IR arg3
>  argument is used to hand in the control value,
> @@ -1235,13 +1244,16 @@ Same as
>  .BR PR_SPEC_DISABLE ,
>  but cannot be undone.
>  A subsequent
> -.B
> -prctl(..., PR_SPEC_ENABLE)
> +.BR prctl (\c
> +.IR arg2 ,
> +.BR PR_SPEC_ENABLE )
> +with the same value for
> +.I arg2
>  will fail with the error
>  .BR EPERM .
>  .RE
>  .IP
> -Any other value in
> +Any unsupported value in
>  .IR arg3
>  will result in the call failing with the error
>  .BR ERANGE .
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Dave Martin]

On Thu, May 28, 2020 at 09:01:59AM +0200, Michael Kerrisk (man-pages) wrote:
> Hi Dave,
> 
> On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
> >
> > Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> > for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> 
> I had also applied this patch from the email you sent earlier. I've
> pushed those changes to master now.

Thanks for this.

Do you publish a "development" branch somewhere, or similar?

If possible, it's nice to have git rebase work out which patches to drop
for me rather than me doing it by hand.

Don't bother if publishing an extra branch doesn't fit with your
workflow, though.

Cheers
---Dave

Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

On 6/1/20 3:51 PM, Dave Martin wrote:
> On Thu, May 28, 2020 at 09:01:59AM +0200, Michael Kerrisk (man-pages) wrote:
>> Hi Dave,
>>
>> On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
>>>
>>> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
>>> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Tim Chen <tim.c.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>
>> I had also applied this patch from the email you sent earlier. I've
>> pushed those changes to master now.
> 
> Thanks for this.
> 
> Do you publish a "development" branch somewhere, or similar?

Ocassionally, I do this on a per-series basis.

> 
> If possible, it's nice to have git rebase work out which patches to drop
> for me rather than me doing it by hand.

Yuh, sorry about that. I'l try to do it bette rnext time.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 2/6] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

On 6/1/20 3:51 PM, Dave Martin wrote:
> On Thu, May 28, 2020 at 09:01:59AM +0200, Michael Kerrisk (man-pages) wrote:
>> Hi Dave,
>>
>> On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
>>>
>>> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
>>> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>
>> I had also applied this patch from the email you sent earlier. I've
>> pushed those changes to master now.
> 
> Thanks for this.
> 
> Do you publish a "development" branch somewhere, or similar?

Ocassionally, I do this on a per-series basis.

> 
> If possible, it's nice to have git rebase work out which patches to drop
> for me rather than me doing it by hand.

Yuh, sorry about that. I'l try to do it bette rnext time.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

[PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Dave Martin]

Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
for the PR_SPEC_STORE_BYPASS "misfeature" of
PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.

Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
Cc: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 man2/prctl.2 | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/man2/prctl.2 b/man2/prctl.2
index b6fb51c..cab9915 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
 Same as
 .B PR_SPEC_DISABLE
 but cannot be undone.
+.TP
+.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
+Same as
+.BR PR_SPEC_DISABLE ,
+but but the state will be cleared on
+.BR execve (2).
 .RE
 .IP
 If all bits are 0,
@@ -1251,6 +1257,17 @@ with the same value for
 .I arg2
 will fail with the error
 .BR EPERM .
+.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
+.TP
+.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
+Same as
+.BR PR_SPEC_DISABLE ,
+but but the state will be cleared on
+.BR execve (2).
+Currently only supported for
+.I arg2
+equal to
+.B PR_SPEC_STORE_BYPASS.
 .RE
 .IP
 Any unsupported value in
@@ -1899,11 +1916,12 @@ was
 .BR PR_SET_SPECULATION_CTRL
 and
 .IR arg3
-is neither
+is not
 .BR PR_SPEC_ENABLE ,
 .BR PR_SPEC_DISABLE ,
+.BR PR_SPEC_FORCE_DISABLE ,
 nor
-.BR PR_SPEC_FORCE_DISABLE .
+.BR PR_SPEC_DISABLE_NOEXEC .
 .SH VERSIONS
 The
 .BR prctl ()
-- 
2.1.4

[PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Dave Martin]

Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
for the PR_SPEC_STORE_BYPASS "misfeature" of
PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 man2/prctl.2 | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/man2/prctl.2 b/man2/prctl.2
index b6fb51c..cab9915 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
 Same as
 .B PR_SPEC_DISABLE
 but cannot be undone.
+.TP
+.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
+Same as
+.BR PR_SPEC_DISABLE ,
+but but the state will be cleared on
+.BR execve (2).
 .RE
 .IP
 If all bits are 0,
@@ -1251,6 +1257,17 @@ with the same value for
 .I arg2
 will fail with the error
 .BR EPERM .
+.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
+.TP
+.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
+Same as
+.BR PR_SPEC_DISABLE ,
+but but the state will be cleared on
+.BR execve (2).
+Currently only supported for
+.I arg2
+equal to
+.B PR_SPEC_STORE_BYPASS.
 .RE
 .IP
 Any unsupported value in
@@ -1899,11 +1916,12 @@ was
 .BR PR_SET_SPECULATION_CTRL
 and
 .IR arg3
-is neither
+is not
 .BR PR_SPEC_ENABLE ,
 .BR PR_SPEC_DISABLE ,
+.BR PR_SPEC_FORCE_DISABLE ,
 nor
-.BR PR_SPEC_FORCE_DISABLE .
+.BR PR_SPEC_DISABLE_NOEXEC .
 .SH VERSIONS
 The
 .BR prctl ()
-- 
2.1.4

Re: [PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
>
> Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
> for the PR_SPEC_STORE_BYPASS "misfeature" of
> PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

I had already applied your earlier send of this patch (in a private
branch). I'll push those changes shortly.

Cheers,

Michael

> ---
>  man2/prctl.2 | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b6fb51c..cab9915 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
>  Same as
>  .B PR_SPEC_DISABLE
>  but cannot be undone.
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
>  .RE
>  .IP
>  If all bits are 0,
> @@ -1251,6 +1257,17 @@ with the same value for
>  .I arg2
>  will fail with the error
>  .BR EPERM .
> +.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
> +Currently only supported for
> +.I arg2
> +equal to
> +.B PR_SPEC_STORE_BYPASS.
>  .RE
>  .IP
>  Any unsupported value in
> @@ -1899,11 +1916,12 @@ was
>  .BR PR_SET_SPECULATION_CTRL
>  and
>  .IR arg3
> -is neither
> +is not
>  .BR PR_SPEC_ENABLE ,
>  .BR PR_SPEC_DISABLE ,
> +.BR PR_SPEC_FORCE_DISABLE ,
>  nor
> -.BR PR_SPEC_FORCE_DISABLE .
> +.BR PR_SPEC_DISABLE_NOEXEC .
>  .SH VERSIONS
>  The
>  .BR prctl ()
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Michael Kerrisk (man-pages)]

Hi Dave,

On Wed, 27 May 2020 at 23:18, Dave Martin <Dave.Martin@arm.com> wrote:
>
> Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
> for the PR_SPEC_STORE_BYPASS "misfeature" of
> PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>

I had already applied your earlier send of this patch (in a private
branch). I'll push those changes shortly.

Cheers,

Michael

> ---
>  man2/prctl.2 | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b6fb51c..cab9915 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
>  Same as
>  .B PR_SPEC_DISABLE
>  but cannot be undone.
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
>  .RE
>  .IP
>  If all bits are 0,
> @@ -1251,6 +1257,17 @@ with the same value for
>  .I arg2
>  will fail with the error
>  .BR EPERM .
> +.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
> +Currently only supported for
> +.I arg2
> +equal to
> +.B PR_SPEC_STORE_BYPASS.
>  .RE
>  .IP
>  Any unsupported value in
> @@ -1899,11 +1916,12 @@ was
>  .BR PR_SET_SPECULATION_CTRL
>  and
>  .IR arg3
> -is neither
> +is not
>  .BR PR_SPEC_ENABLE ,
>  .BR PR_SPEC_DISABLE ,
> +.BR PR_SPEC_FORCE_DISABLE ,
>  nor
> -.BR PR_SPEC_FORCE_DISABLE .
> +.BR PR_SPEC_DISABLE_NOEXEC .
>  .SH VERSIONS
>  The
>  .BR prctl ()
> --
> 2.1.4
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Waiman Long]

On 5/27/20 5:17 PM, Dave Martin wrote:
> Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
> for the PR_SPEC_STORE_BYPASS "misfeature" of
> PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> ---
>   man2/prctl.2 | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b6fb51c..cab9915 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
>   Same as
>   .B PR_SPEC_DISABLE
>   but cannot be undone.
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
>   .RE
>   .IP
>   If all bits are 0,
> @@ -1251,6 +1257,17 @@ with the same value for
>   .I arg2
>   will fail with the error
>   .BR EPERM .
> +.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
> +Currently only supported for
> +.I arg2
> +equal to
> +.B PR_SPEC_STORE_BYPASS.
>   .RE
>   .IP
>   Any unsupported value in
> @@ -1899,11 +1916,12 @@ was
>   .BR PR_SET_SPECULATION_CTRL
>   and
>   .IR arg3
> -is neither
> +is not
>   .BR PR_SPEC_ENABLE ,
>   .BR PR_SPEC_DISABLE ,
> +.BR PR_SPEC_FORCE_DISABLE ,
>   nor
> -.BR PR_SPEC_FORCE_DISABLE .
> +.BR PR_SPEC_DISABLE_NOEXEC .
>   .SH VERSIONS
>   The
>   .BR prctl ()

Acked-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v2 3/6] prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls

[Waiman Long]

On 5/27/20 5:17 PM, Dave Martin wrote:
> Add the PR_SPEC_DISABLE_NOEXEC mode added in Linux 5.1
> for the PR_SPEC_STORE_BYPASS "misfeature" of
> PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>   man2/prctl.2 | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b6fb51c..cab9915 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1187,6 +1187,12 @@ The speculation feature is disabled, mitigation is enabled.
>   Same as
>   .B PR_SPEC_DISABLE
>   but cannot be undone.
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
>   .RE
>   .IP
>   If all bits are 0,
> @@ -1251,6 +1257,17 @@ with the same value for
>   .I arg2
>   will fail with the error
>   .BR EPERM .
> +.\" commit 71368af9027f18fe5d1c6f372cfdff7e4bde8b48
> +.TP
> +.BR PR_SPEC_DISABLE_NOEXEC " (since Linux 5.1)"
> +Same as
> +.BR PR_SPEC_DISABLE ,
> +but but the state will be cleared on
> +.BR execve (2).
> +Currently only supported for
> +.I arg2
> +equal to
> +.B PR_SPEC_STORE_BYPASS.
>   .RE
>   .IP
>   Any unsupported value in
> @@ -1899,11 +1916,12 @@ was
>   .BR PR_SET_SPECULATION_CTRL
>   and
>   .IR arg3
> -is neither
> +is not
>   .BR PR_SPEC_ENABLE ,
>   .BR PR_SPEC_DISABLE ,
> +.BR PR_SPEC_FORCE_DISABLE ,
>   nor
> -.BR PR_SPEC_FORCE_DISABLE .
> +.BR PR_SPEC_DISABLE_NOEXEC .
>   .SH VERSIONS
>   The
>   .BR prctl ()

Acked-by: Waiman Long <longman@redhat.com>

[PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
prctls added in Linux 4.15 for arm64.

Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

---

Since v1:

 * Minor rewordings and typo fixes.

 * Fix typo'd #define names.

 * Add type annotation for PR_SVE_SET_VL arg2.

 * Clarify return value semantics of PR_SVE_SET_VL

 * Add note to say that the args for PR_SVE_GET_VL are ignored.

 * Note for PR_SVE_SET_VL that the PR_SVE_VL_LEN_MASK field specifies
   an upper bound on what vector length to set, not an exact value.

 * Rework PR_SVE_SET_VL arg2 description to enumerate all possible flag
   combinations rather than describing the flags independently.

   Coming up with a clear description of each flag that is independent
   of the description of the other flag turns out to be hard.

 * In lieu of having a separate man page to cross reference for detailed
   guidance, cross-reference the kernel documentation.

 * Avoid confusing cross-reference to PR_SVE_SET_VL when describing the
   return value of PR_SVE_GET_VL.

 * Clarify error conditions for PR_SVE_SET_VL and PR_SVE_GET_VL, and
   move detail to the individual prctl descriptions to keep related
   content close together while minimising duplication.

 * Add safety warning.  This is deliberately vague, pending ongoing
   discussions with libc folks.
---
 man2/prctl.2 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)

diff --git a/man2/prctl.2 b/man2/prctl.2
index cab9915..91df7c8 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1291,6 +1291,148 @@ call failing with the error
 .BR ENXIO .
 For further details, see the kernel source file
 .IR Documentation/admin\-guide/kernel\-parameters.txt .
+.\" prctl PR_SVE_SET_VL
+.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
+.\" linux-5.6/Documentation/arm64/sve.rst
+.TP
+.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
+Configure the thread's SVE vector length,
+as specified by
+.IR "(int) arg2" .
+Arguments
+.IR arg3 ", " arg4 " and " arg5
+are ignored.
+.IP
+The bits of
+.I arg2
+corresponding to
+.B PR_SVE_VL_LEN_MASK
+must be set to the desired vector length in bytes.
+This is interpreted as an upper bound:
+the kernel will select the greatest available vector length
+that does not exceed the value specified.
+In particular, specifying
+.B SVE_VL_MAX
+(defined in
+.I <asm/sigcontext.h>)
+for the
+.B PR_SVE_VL_LEN_MASK
+bits requests the maximum supported vector length.
+.IP
+In addition,
+.I arg2
+must be set to one of the following combinations of flags:
+.RS
+.TP
+.B 0
+Perform the change immediately.
+At the next
+.BR execve (2)
+in the thread,
+the vector length will be reset to the value configured in
+.IR /proc/sys/abi/sve_default_vector_length .
+.TP
+.B PR_SVE_VL_INHERIT
+Perform the change immediately.
+Subsequent
+.BR execve (2)
+calls will preserve the new vector length.
+.TP
+.B PR_SVE_SET_VL_ONEXEC
+Defer the change, so that it is performed at the next
+.BR execve (2)
+in the thread.
+Further
+.BR execve (2)
+calls will reset the vector length to the value configured in
+.IR /proc/sys/abi/sve_default_vector_length .
+.TP
+.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
+Defer the change, so that it is performed at the next
+.BR execve (2)
+in the thread.
+Further
+.BR execve (2)
+calls will preserve the new vector length.
+.RE
+.IP
+In all cases,
+any previously pending deferred change is canceled.
+.IP
+The call fails with error
+.B EINVAL
+if SVE is not supported on the platform, if
+.I arg2
+is unrecognized or invalid, or the value in the bits of
+.I arg2
+corresponding to
+.B PR_SVE_VL_LEN_MASK
+is outside the range
+.BR SVE_VL_MIN .. SVE_VL_MAX
+or is not a multiple of 16.
+.IP
+On success,
+a nonnegative value is returned that describes the
+.I selected
+configuration,
+which may differ from the current configuration if
+.B PR_SVE_SET_VL_ONEXEC
+was specified.
+The value is encoded in the same way as the return value of
+.BR PR_SVE_GET_VL .
+.IP
+The configuration (including any pending deferred change)
+is inherited across
+.BR fork (2)
+and
+.BR clone (2).
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may be using SVE, using this call without the
+.B PR_SVE_SET_VL_ONEXEC
+flag may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you really know what you are doing.
+.IP
+For more information, see the kernel source file
+.I Documentation/arm64/sve.rst
+.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
+(or
+.I Documentation/arm64/sve.txt
+before Linux 5.3).
+.\" prctl PR_SVE_GET_VL
+.TP
+.BR PR_SVE_GET_VL " (since Linux 4.15, only on arm64)"
+Get the thread's current SVE vector length configuration.
+.IP
+Arguments
+.IR arg2 ", " arg3 ", " arg4 " and " arg5
+are ignored.
+.IP
+Providing that the kernel and platform support SVE
+this operation always succeeds,
+returning a nonnegative value that describes the
+.I current
+configuration.
+The bits corresponding to
+.B PR_SVE_VL_LEN_MASK
+contain the currently configured vector length in bytes.
+The bit corresponding to
+.B PR_SVE_VL_INHERIT
+indicates whether the vector length will be inherited
+across
+.BR execve (2).
+.IP
+Note that there is no way to determine whether there is
+a pending vector length change that has not yet taken effect.
+.IP
+For more information, see the kernel source file
+.I Documentation/arm64/sve.rst
+.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
+(or
+.I Documentation/arm64/sve.txt
+before Linux 5.3).
 .\"
 .\" prctl PR_TASK_PERF_EVENTS_DISABLE
 .TP
@@ -1534,6 +1676,8 @@ On success,
 .BR PR_GET_NO_NEW_PRIVS ,
 .BR PR_GET_SECUREBITS ,
 .BR PR_GET_SPECULATION_CTRL ,
+.BR PR_SVE_GET_VL ,
+.BR PR_SVE_SET_VL ,
 .BR PR_GET_THP_DISABLE ,
 .BR PR_GET_TIMING ,
 .BR PR_GET_TIMERSLACK ,
@@ -1817,6 +1961,22 @@ and unused arguments to
 .BR prctl ()
 are not 0.
 .TP
+.B EINVAL
+.I option
+is
+.B PR_SVE_SET_VL
+and the arguments are invalid or unsupported,
+or SVE is not available on this platform.
+See the description of
+.B PR_SVE_SET_VL
+above for details.
+.TP
+.B EINVAL
+.I option
+is
+.B PR_SVE_GET_VL
+and SVE is not available on this platform.
+.TP
 .B ENODEV
 .I option
 was
-- 
2.1.4

[PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
prctls added in Linux 4.15 for arm64.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>

---

Since v1:

 * Minor rewordings and typo fixes.

 * Fix typo'd #define names.

 * Add type annotation for PR_SVE_SET_VL arg2.

 * Clarify return value semantics of PR_SVE_SET_VL

 * Add note to say that the args for PR_SVE_GET_VL are ignored.

 * Note for PR_SVE_SET_VL that the PR_SVE_VL_LEN_MASK field specifies
   an upper bound on what vector length to set, not an exact value.

 * Rework PR_SVE_SET_VL arg2 description to enumerate all possible flag
   combinations rather than describing the flags independently.

   Coming up with a clear description of each flag that is independent
   of the description of the other flag turns out to be hard.

 * In lieu of having a separate man page to cross reference for detailed
   guidance, cross-reference the kernel documentation.

 * Avoid confusing cross-reference to PR_SVE_SET_VL when describing the
   return value of PR_SVE_GET_VL.

 * Clarify error conditions for PR_SVE_SET_VL and PR_SVE_GET_VL, and
   move detail to the individual prctl descriptions to keep related
   content close together while minimising duplication.

 * Add safety warning.  This is deliberately vague, pending ongoing
   discussions with libc folks.
---
 man2/prctl.2 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)

diff --git a/man2/prctl.2 b/man2/prctl.2
index cab9915..91df7c8 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1291,6 +1291,148 @@ call failing with the error
 .BR ENXIO .
 For further details, see the kernel source file
 .IR Documentation/admin\-guide/kernel\-parameters.txt .
+.\" prctl PR_SVE_SET_VL
+.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
+.\" linux-5.6/Documentation/arm64/sve.rst
+.TP
+.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
+Configure the thread's SVE vector length,
+as specified by
+.IR "(int) arg2" .
+Arguments
+.IR arg3 ", " arg4 " and " arg5
+are ignored.
+.IP
+The bits of
+.I arg2
+corresponding to
+.B PR_SVE_VL_LEN_MASK
+must be set to the desired vector length in bytes.
+This is interpreted as an upper bound:
+the kernel will select the greatest available vector length
+that does not exceed the value specified.
+In particular, specifying
+.B SVE_VL_MAX
+(defined in
+.I <asm/sigcontext.h>)
+for the
+.B PR_SVE_VL_LEN_MASK
+bits requests the maximum supported vector length.
+.IP
+In addition,
+.I arg2
+must be set to one of the following combinations of flags:
+.RS
+.TP
+.B 0
+Perform the change immediately.
+At the next
+.BR execve (2)
+in the thread,
+the vector length will be reset to the value configured in
+.IR /proc/sys/abi/sve_default_vector_length .
+.TP
+.B PR_SVE_VL_INHERIT
+Perform the change immediately.
+Subsequent
+.BR execve (2)
+calls will preserve the new vector length.
+.TP
+.B PR_SVE_SET_VL_ONEXEC
+Defer the change, so that it is performed at the next
+.BR execve (2)
+in the thread.
+Further
+.BR execve (2)
+calls will reset the vector length to the value configured in
+.IR /proc/sys/abi/sve_default_vector_length .
+.TP
+.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
+Defer the change, so that it is performed at the next
+.BR execve (2)
+in the thread.
+Further
+.BR execve (2)
+calls will preserve the new vector length.
+.RE
+.IP
+In all cases,
+any previously pending deferred change is canceled.
+.IP
+The call fails with error
+.B EINVAL
+if SVE is not supported on the platform, if
+.I arg2
+is unrecognized or invalid, or the value in the bits of
+.I arg2
+corresponding to
+.B PR_SVE_VL_LEN_MASK
+is outside the range
+.BR SVE_VL_MIN .. SVE_VL_MAX
+or is not a multiple of 16.
+.IP
+On success,
+a nonnegative value is returned that describes the
+.I selected
+configuration,
+which may differ from the current configuration if
+.B PR_SVE_SET_VL_ONEXEC
+was specified.
+The value is encoded in the same way as the return value of
+.BR PR_SVE_GET_VL .
+.IP
+The configuration (including any pending deferred change)
+is inherited across
+.BR fork (2)
+and
+.BR clone (2).
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may be using SVE, using this call without the
+.B PR_SVE_SET_VL_ONEXEC
+flag may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you really know what you are doing.
+.IP
+For more information, see the kernel source file
+.I Documentation/arm64/sve.rst
+.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
+(or
+.I Documentation/arm64/sve.txt
+before Linux 5.3).
+.\" prctl PR_SVE_GET_VL
+.TP
+.BR PR_SVE_GET_VL " (since Linux 4.15, only on arm64)"
+Get the thread's current SVE vector length configuration.
+.IP
+Arguments
+.IR arg2 ", " arg3 ", " arg4 " and " arg5
+are ignored.
+.IP
+Providing that the kernel and platform support SVE
+this operation always succeeds,
+returning a nonnegative value that describes the
+.I current
+configuration.
+The bits corresponding to
+.B PR_SVE_VL_LEN_MASK
+contain the currently configured vector length in bytes.
+The bit corresponding to
+.B PR_SVE_VL_INHERIT
+indicates whether the vector length will be inherited
+across
+.BR execve (2).
+.IP
+Note that there is no way to determine whether there is
+a pending vector length change that has not yet taken effect.
+.IP
+For more information, see the kernel source file
+.I Documentation/arm64/sve.rst
+.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
+(or
+.I Documentation/arm64/sve.txt
+before Linux 5.3).
 .\"
 .\" prctl PR_TASK_PERF_EVENTS_DISABLE
 .TP
@@ -1534,6 +1676,8 @@ On success,
 .BR PR_GET_NO_NEW_PRIVS ,
 .BR PR_GET_SECUREBITS ,
 .BR PR_GET_SPECULATION_CTRL ,
+.BR PR_SVE_GET_VL ,
+.BR PR_SVE_SET_VL ,
 .BR PR_GET_THP_DISABLE ,
 .BR PR_GET_TIMING ,
 .BR PR_GET_TIMERSLACK ,
@@ -1817,6 +1961,22 @@ and unused arguments to
 .BR prctl ()
 are not 0.
 .TP
+.B EINVAL
+.I option
+is
+.B PR_SVE_SET_VL
+and the arguments are invalid or unsupported,
+or SVE is not available on this platform.
+See the description of
+.B PR_SVE_SET_VL
+above for details.
+.TP
+.B EINVAL
+.I option
+is
+.B PR_SVE_GET_VL
+and SVE is not available on this platform.
+.TP
 .B ENODEV
 .I option
 was
-- 
2.1.4

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Will Deacon]

Hi Dave,

On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
> prctls added in Linux 4.15 for arm64.

Looks really good to me, thanks. Just a few comments inline.

> diff --git a/man2/prctl.2 b/man2/prctl.2
> index cab9915..91df7c8 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1291,6 +1291,148 @@ call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
>  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> +.\" prctl PR_SVE_SET_VL
> +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> +.\" linux-5.6/Documentation/arm64/sve.rst
> +.TP
> +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> +Configure the thread's SVE vector length,
> +as specified by
> +.IR "(int) arg2" .
> +Arguments
> +.IR arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +The bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +must be set to the desired vector length in bytes.
> +This is interpreted as an upper bound:
> +the kernel will select the greatest available vector length
> +that does not exceed the value specified.
> +In particular, specifying
> +.B SVE_VL_MAX
> +(defined in
> +.I <asm/sigcontext.h>)
> +for the
> +.B PR_SVE_VL_LEN_MASK
> +bits requests the maximum supported vector length.
> +.IP
> +In addition,
> +.I arg2
> +must be set to one of the following combinations of flags:

How about saying:

  In addition, the other bits of arg2 must be set according to the following
  combinations of flags:

Otherwise I find it a bit fiddly to read, because it's valid to have
flags of 0 and a non-zero length.

> +.RS
> +.TP
> +.B 0
> +Perform the change immediately.
> +At the next
> +.BR execve (2)
> +in the thread,
> +the vector length will be reset to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .

(implementation note: does this mean that 'sve_default_vl' should be
 an atomic_t, as it can be accessed concurrently? We probably need
 {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
 that help us here...)

> +.TP
> +.B PR_SVE_VL_INHERIT
> +Perform the change immediately.
> +Subsequent
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.TP
> +.B PR_SVE_SET_VL_ONEXEC
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will reset the vector length to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.RE
> +.IP
> +In all cases,
> +any previously pending deferred change is canceled.
> +.IP
> +The call fails with error
> +.B EINVAL
> +if SVE is not supported on the platform, if
> +.I arg2
> +is unrecognized or invalid, or the value in the bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +is outside the range
> +.BR SVE_VL_MIN .. SVE_VL_MAX
> +or is not a multiple of 16.
> +.IP
> +On success,
> +a nonnegative value is returned that describes the
> +.I selected
> +configuration,

If I'm reading the kernel code correctly, this is slightly weird, as
the returned value may contain the PR_SVE_VL_INHERIT flag but it will
never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?

If so, maybe just say something like:

  On success, a nonnegative value is returned that describes the selected
  configuration in the same way as PR_SVE_GET_VL.

> +which may differ from the current configuration if
> +.B PR_SVE_SET_VL_ONEXEC
> +was specified.
> +The value is encoded in the same way as the return value of
> +.BR PR_SVE_GET_VL .
> +.IP
> +The configuration (including any pending deferred change)
> +is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using SVE, using this call without the
> +.B PR_SVE_SET_VL_ONEXEC
> +flag may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you really know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).

I think I'd drop the kernel reference here, as it feels like we're saying
"only do this if you know what you're doing" on one hand, but then "if you
don't know what you're doing, see this other documentation" on the other.

Will

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Will Deacon]

Hi Dave,

On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
> prctls added in Linux 4.15 for arm64.

Looks really good to me, thanks. Just a few comments inline.

> diff --git a/man2/prctl.2 b/man2/prctl.2
> index cab9915..91df7c8 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1291,6 +1291,148 @@ call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
>  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> +.\" prctl PR_SVE_SET_VL
> +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> +.\" linux-5.6/Documentation/arm64/sve.rst
> +.TP
> +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> +Configure the thread's SVE vector length,
> +as specified by
> +.IR "(int) arg2" .
> +Arguments
> +.IR arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +The bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +must be set to the desired vector length in bytes.
> +This is interpreted as an upper bound:
> +the kernel will select the greatest available vector length
> +that does not exceed the value specified.
> +In particular, specifying
> +.B SVE_VL_MAX
> +(defined in
> +.I <asm/sigcontext.h>)
> +for the
> +.B PR_SVE_VL_LEN_MASK
> +bits requests the maximum supported vector length.
> +.IP
> +In addition,
> +.I arg2
> +must be set to one of the following combinations of flags:

How about saying:

  In addition, the other bits of arg2 must be set according to the following
  combinations of flags:

Otherwise I find it a bit fiddly to read, because it's valid to have
flags of 0 and a non-zero length.

> +.RS
> +.TP
> +.B 0
> +Perform the change immediately.
> +At the next
> +.BR execve (2)
> +in the thread,
> +the vector length will be reset to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .

(implementation note: does this mean that 'sve_default_vl' should be
 an atomic_t, as it can be accessed concurrently? We probably need
 {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
 that help us here...)

> +.TP
> +.B PR_SVE_VL_INHERIT
> +Perform the change immediately.
> +Subsequent
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.TP
> +.B PR_SVE_SET_VL_ONEXEC
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will reset the vector length to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.RE
> +.IP
> +In all cases,
> +any previously pending deferred change is canceled.
> +.IP
> +The call fails with error
> +.B EINVAL
> +if SVE is not supported on the platform, if
> +.I arg2
> +is unrecognized or invalid, or the value in the bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +is outside the range
> +.BR SVE_VL_MIN .. SVE_VL_MAX
> +or is not a multiple of 16.
> +.IP
> +On success,
> +a nonnegative value is returned that describes the
> +.I selected
> +configuration,

If I'm reading the kernel code correctly, this is slightly weird, as
the returned value may contain the PR_SVE_VL_INHERIT flag but it will
never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?

If so, maybe just say something like:

  On success, a nonnegative value is returned that describes the selected
  configuration in the same way as PR_SVE_GET_VL.

> +which may differ from the current configuration if
> +.B PR_SVE_SET_VL_ONEXEC
> +was specified.
> +The value is encoded in the same way as the return value of
> +.BR PR_SVE_GET_VL .
> +.IP
> +The configuration (including any pending deferred change)
> +is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using SVE, using this call without the
> +.B PR_SVE_SET_VL_ONEXEC
> +flag may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you really know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).

I think I'd drop the kernel reference here, as it feels like we're saying
"only do this if you know what you're doing" on one hand, but then "if you
don't know what you're doing, see this other documentation" on the other.

Will

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> Hi Dave,
> 
> On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
> > prctls added in Linux 4.15 for arm64.
> 
> Looks really good to me, thanks. Just a few comments inline.
> 
> > diff --git a/man2/prctl.2 b/man2/prctl.2
> > index cab9915..91df7c8 100644
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1291,6 +1291,148 @@ call failing with the error
> >  .BR ENXIO .
> >  For further details, see the kernel source file
> >  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> > +.\" prctl PR_SVE_SET_VL
> > +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> > +.\" linux-5.6/Documentation/arm64/sve.rst
> > +.TP
> > +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> > +Configure the thread's SVE vector length,
> > +as specified by
> > +.IR "(int) arg2" .
> > +Arguments
> > +.IR arg3 ", " arg4 " and " arg5
> > +are ignored.
> > +.IP
> > +The bits of
> > +.I arg2
> > +corresponding to
> > +.B PR_SVE_VL_LEN_MASK
> > +must be set to the desired vector length in bytes.
> > +This is interpreted as an upper bound:
> > +the kernel will select the greatest available vector length
> > +that does not exceed the value specified.
> > +In particular, specifying
> > +.B SVE_VL_MAX
> > +(defined in
> > +.I <asm/sigcontext.h>)
> > +for the
> > +.B PR_SVE_VL_LEN_MASK
> > +bits requests the maximum supported vector length.
> > +.IP
> > +In addition,
> > +.I arg2
> > +must be set to one of the following combinations of flags:
> 
> How about saying:
> 
>   In addition, the other bits of arg2 must be set according to the following
>   combinations of flags:
> 
> Otherwise I find it a bit fiddly to read, because it's valid to have
> flags of 0 and a non-zero length.

0 is listed, so I hoped that was clear enough.

Maybe just write "must be one of the following values:"?

0 is a value, but I can see why you might be uneasy about 0 being
described as a "combination of flags".

> > +.RS
> > +.TP
> > +.B 0
> > +Perform the change immediately.
> > +At the next
> > +.BR execve (2)
> > +in the thread,
> > +the vector length will be reset to the value configured in
> > +.IR /proc/sys/abi/sve_default_vector_length .
> 
> (implementation note: does this mean that 'sve_default_vl' should be
>  an atomic_t, as it can be accessed concurrently? We probably need
>  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
>  that help us here...)

Is this purely theoretical?  Can you point to what could go wrong?

While I doubt I thought about this very hard and I agree that you're
right in principle, I think there are probably non-atomic sysctls and
debugs files etc. all over the place.

I didn't want to clutter the code unnecessarily.

> > +.B PR_SVE_VL_INHERIT
> > +Perform the change immediately.
> > +Subsequent
> > +.BR execve (2)
> > +calls will preserve the new vector length.
> > +.TP
> > +.B PR_SVE_SET_VL_ONEXEC
> > +Defer the change, so that it is performed at the next
> > +.BR execve (2)
> > +in the thread.
> > +Further
> > +.BR execve (2)
> > +calls will reset the vector length to the value configured in
> > +.IR /proc/sys/abi/sve_default_vector_length .
> > +.TP
> > +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> > +Defer the change, so that it is performed at the next
> > +.BR execve (2)
> > +in the thread.
> > +Further
> > +.BR execve (2)
> > +calls will preserve the new vector length.
> > +.RE
> > +.IP
> > +In all cases,
> > +any previously pending deferred change is canceled.
> > +.IP
> > +The call fails with error
> > +.B EINVAL
> > +if SVE is not supported on the platform, if
> > +.I arg2
> > +is unrecognized or invalid, or the value in the bits of
> > +.I arg2
> > +corresponding to
> > +.B PR_SVE_VL_LEN_MASK
> > +is outside the range
> > +.BR SVE_VL_MIN .. SVE_VL_MAX
> > +or is not a multiple of 16.
> > +.IP
> > +On success,
> > +a nonnegative value is returned that describes the
> > +.I selected
> > +configuration,
> 
> If I'm reading the kernel code correctly, this is slightly weird, as
> the returned value may contain the PR_SVE_VL_INHERIT flag but it will
> never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?

Yes, which is an oddity.

I suppose we could fake that up actually by returning that flag if
sve_vl and sve_vl_onexec are different, but we don't currently do this.

> If so, maybe just say something like:
> 
>   On success, a nonnegative value is returned that describes the selected
>   configuration in the same way as PR_SVE_GET_VL.

How does that help?  PR_SVE_GET_VL doesn't fully clarify the oddity you
call out anyway.

Really, I preferred not to have people relying on this one way or the
other.  The only sensible reason for an _ONEXEC is because you've
committed to calling execve().  On such a path, queryng the vector
length isn't likely to be useful.

Maybe I was optimistic.

> > +which may differ from the current configuration if
> > +.B PR_SVE_SET_VL_ONEXEC
> > +was specified.
> > +The value is encoded in the same way as the return value of
> > +.BR PR_SVE_GET_VL .
> > +.IP
> > +The configuration (including any pending deferred change)
> > +is inherited across
> > +.BR fork (2)
> > +and
> > +.BR clone (2).
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may be using SVE, using this call without the
> > +.B PR_SVE_SET_VL_ONEXEC
> > +flag may crash the calling process.
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you really know what you are doing.
> > +.IP
> > +For more information, see the kernel source file
> > +.I Documentation/arm64/sve.rst
> > +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> > +(or
> > +.I Documentation/arm64/sve.txt
> > +before Linux 5.3).
> 
> I think I'd drop the kernel reference here, as it feels like we're saying
> "only do this if you know what you're doing" on one hand, but then "if you
> don't know what you're doing, see this other documentation" on the other.

Well, the docmuentation doesn't answer those questions either.

I could just swap the warning and the cross-reference, so that the
cross-reference doesn't seem to follow on from "knowing what you're
doing"?

Cheers
---Dave

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Will Deacon]

On Tue, Jun 09, 2020 at 03:11:42PM +0100, Dave Martin wrote:
> On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> > On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > > diff --git a/man2/prctl.2 b/man2/prctl.2
> > > index cab9915..91df7c8 100644
> > > --- a/man2/prctl.2
> > > +++ b/man2/prctl.2
> > > @@ -1291,6 +1291,148 @@ call failing with the error
> > >  .BR ENXIO .
> > >  For further details, see the kernel source file
> > >  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> > > +.\" prctl PR_SVE_SET_VL
> > > +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> > > +.\" linux-5.6/Documentation/arm64/sve.rst
> > > +.TP
> > > +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> > > +Configure the thread's SVE vector length,
> > > +as specified by
> > > +.IR "(int) arg2" .
> > > +Arguments
> > > +.IR arg3 ", " arg4 " and " arg5
> > > +are ignored.
> > > +.IP
> > > +The bits of
> > > +.I arg2
> > > +corresponding to
> > > +.B PR_SVE_VL_LEN_MASK
> > > +must be set to the desired vector length in bytes.
> > > +This is interpreted as an upper bound:
> > > +the kernel will select the greatest available vector length
> > > +that does not exceed the value specified.
> > > +In particular, specifying
> > > +.B SVE_VL_MAX
> > > +(defined in
> > > +.I <asm/sigcontext.h>)
> > > +for the
> > > +.B PR_SVE_VL_LEN_MASK
> > > +bits requests the maximum supported vector length.
> > > +.IP
> > > +In addition,
> > > +.I arg2
> > > +must be set to one of the following combinations of flags:
> > 
> > How about saying:
> > 
> >   In addition, the other bits of arg2 must be set according to the following
> >   combinations of flags:
> > 
> > Otherwise I find it a bit fiddly to read, because it's valid to have
> > flags of 0 and a non-zero length.
> 
> 0 is listed, so I hoped that was clear enough.
> 
> Maybe just write "must be one of the following values:"?
> 
> 0 is a value, but I can see why you might be uneasy about 0 being
> described as a "combination of flags".

It's more that arg2 *also* holds the length, so saying that arg2 must
be set to a combination of flags isn't quite right, because it's actually
to set to a combination of flags and the length.

> > > +.RS
> > > +.TP
> > > +.B 0
> > > +Perform the change immediately.
> > > +At the next
> > > +.BR execve (2)
> > > +in the thread,
> > > +the vector length will be reset to the value configured in
> > > +.IR /proc/sys/abi/sve_default_vector_length .
> > 
> > (implementation note: does this mean that 'sve_default_vl' should be
> >  an atomic_t, as it can be accessed concurrently? We probably need
> >  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
> >  that help us here...)
> 
> Is this purely theoretical?  Can you point to what could go wrong?

If the write is torn by the compiler, then a concurrent reader could end
up seeing a bogus value. There could also be ToCToU issues if it's re-read.

> While I doubt I thought about this very hard and I agree that you're
> right in principle, I think there are probably non-atomic sysctls and
> debugs files etc. all over the place.
> 
> I didn't want to clutter the code unnecessarily.

Right, but KCSAN is coming along and so somebody less familiar with the code
will hit this eventually.

> > > +.B PR_SVE_VL_INHERIT
> > > +Perform the change immediately.
> > > +Subsequent
> > > +.BR execve (2)
> > > +calls will preserve the new vector length.
> > > +.TP
> > > +.B PR_SVE_SET_VL_ONEXEC
> > > +Defer the change, so that it is performed at the next
> > > +.BR execve (2)
> > > +in the thread.
> > > +Further
> > > +.BR execve (2)
> > > +calls will reset the vector length to the value configured in
> > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > +.TP
> > > +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> > > +Defer the change, so that it is performed at the next
> > > +.BR execve (2)
> > > +in the thread.
> > > +Further
> > > +.BR execve (2)
> > > +calls will preserve the new vector length.
> > > +.RE
> > > +.IP
> > > +In all cases,
> > > +any previously pending deferred change is canceled.
> > > +.IP
> > > +The call fails with error
> > > +.B EINVAL
> > > +if SVE is not supported on the platform, if
> > > +.I arg2
> > > +is unrecognized or invalid, or the value in the bits of
> > > +.I arg2
> > > +corresponding to
> > > +.B PR_SVE_VL_LEN_MASK
> > > +is outside the range
> > > +.BR SVE_VL_MIN .. SVE_VL_MAX
> > > +or is not a multiple of 16.
> > > +.IP
> > > +On success,
> > > +a nonnegative value is returned that describes the
> > > +.I selected
> > > +configuration,
> > 
> > If I'm reading the kernel code correctly, this is slightly weird, as
> > the returned value may contain the PR_SVE_VL_INHERIT flag but it will
> > never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?
> 
> Yes, which is an oddity.
> 
> I suppose we could fake that up actually by returning that flag if
> sve_vl and sve_vl_onexec are different, but we don't currently do this.

I don't think there's any need to change the code, but I think this stuff
is worth documenting.

> > If so, maybe just say something like:
> > 
> >   On success, a nonnegative value is returned that describes the selected
> >   configuration in the same way as PR_SVE_GET_VL.
> 
> How does that help?  PR_SVE_GET_VL doesn't fully clarify the oddity you
> call out anyway.

It clarifies it enough for my liking (by explicitly talking about "the bit
corresponding to PR_SVE_VL_INHERIT" and not about PR_SVE_SET_VL_ONEXEC),
but either way, I think saying that the return value is the same is a
useful clarification. If you want to make PR_SVE_GET_VL more explicit,
we could do that too.

> > > +.B PR_SVE_SET_VL_ONEXEC
> > > +flag may crash the calling process.
> > > +The conditions for using it safely are complex and system-dependent.
> > > +Don't use it unless you really know what you are doing.
> > > +.IP
> > > +For more information, see the kernel source file
> > > +.I Documentation/arm64/sve.rst
> > > +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> > > +(or
> > > +.I Documentation/arm64/sve.txt
> > > +before Linux 5.3).
> > 
> > I think I'd drop the kernel reference here, as it feels like we're saying
> > "only do this if you know what you're doing" on one hand, but then "if you
> > don't know what you're doing, see this other documentation" on the other.
> 
> Well, the docmuentation doesn't answer those questions either.
> 
> I could just swap the warning and the cross-reference, so that the
> cross-reference doesn't seem to follow on from "knowing what you're
> doing"?

Ok.

Will

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 03:49:05PM +0100, Will Deacon wrote:
> On Tue, Jun 09, 2020 at 03:11:42PM +0100, Dave Martin wrote:
> > On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> > > On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > > > diff --git a/man2/prctl.2 b/man2/prctl.2
> > > > index cab9915..91df7c8 100644
> > > > --- a/man2/prctl.2
> > > > +++ b/man2/prctl.2
> > > > @@ -1291,6 +1291,148 @@ call failing with the error
> > > >  .BR ENXIO .
> > > >  For further details, see the kernel source file
> > > >  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> > > > +.\" prctl PR_SVE_SET_VL
> > > > +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> > > > +.\" linux-5.6/Documentation/arm64/sve.rst
> > > > +.TP
> > > > +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> > > > +Configure the thread's SVE vector length,
> > > > +as specified by
> > > > +.IR "(int) arg2" .
> > > > +Arguments
> > > > +.IR arg3 ", " arg4 " and " arg5
> > > > +are ignored.
> > > > +.IP
> > > > +The bits of
> > > > +.I arg2
> > > > +corresponding to
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +must be set to the desired vector length in bytes.
> > > > +This is interpreted as an upper bound:
> > > > +the kernel will select the greatest available vector length
> > > > +that does not exceed the value specified.
> > > > +In particular, specifying
> > > > +.B SVE_VL_MAX
> > > > +(defined in
> > > > +.I <asm/sigcontext.h>)
> > > > +for the
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +bits requests the maximum supported vector length.
> > > > +.IP
> > > > +In addition,
> > > > +.I arg2
> > > > +must be set to one of the following combinations of flags:
> > > 
> > > How about saying:
> > > 
> > >   In addition, the other bits of arg2 must be set according to the following
> > >   combinations of flags:
> > > 
> > > Otherwise I find it a bit fiddly to read, because it's valid to have
> > > flags of 0 and a non-zero length.
> > 
> > 0 is listed, so I hoped that was clear enough.
> > 
> > Maybe just write "must be one of the following values:"?
> > 
> > 0 is a value, but I can see why you might be uneasy about 0 being
> > described as a "combination of flags".
> 
> It's more that arg2 *also* holds the length, so saying that arg2 must
> be set to a combination of flags isn't quite right, because it's actually
> to set to a combination of flags and the length.
> 
> > > > +.RS
> > > > +.TP
> > > > +.B 0
> > > > +Perform the change immediately.
> > > > +At the next
> > > > +.BR execve (2)
> > > > +in the thread,
> > > > +the vector length will be reset to the value configured in
> > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > 
> > > (implementation note: does this mean that 'sve_default_vl' should be
> > >  an atomic_t, as it can be accessed concurrently? We probably need
> > >  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
> > >  that help us here...)
> > 
> > Is this purely theoretical?  Can you point to what could go wrong?
> 
> If the write is torn by the compiler, then a concurrent reader could end
> up seeing a bogus value. There could also be ToCToU issues if it's re-read.

It won't be torn in practice, no decision logic depends on the value
read, and you can't even get from the write to the read or vice-versa
without crossing a TU boundary (even under LTO), so there's basically
zero scope for sabotXXXXXoptimisation by the compiler.

Only root is allowed to write this thing anyway.

> > While I doubt I thought about this very hard and I agree that you're
> > right in principle, I think there are probably non-atomic sysctls and
> > debugs files etc. all over the place.
> > 
> > I didn't want to clutter the code unnecessarily.
> 
> Right, but KCSAN is coming along and so somebody less familiar with the code
> will hit this eventually.

So the issue is theoretical, probably one of very many similar issues,
and anyway we have a tool for tracking them down if we need to?

I'm playing devil's advocate here, but I'd debate whether it's worth
it -- or even wise -- to fix these piecemeal unless we're confident this
is an egregious case.  Doing so may encourage a false sense of safety.
When we're in a position to do a treewide cleanup, that would be better,
no?

> > > > +.B PR_SVE_VL_INHERIT
> > > > +Perform the change immediately.
> > > > +Subsequent
> > > > +.BR execve (2)
> > > > +calls will preserve the new vector length.
> > > > +.TP
> > > > +.B PR_SVE_SET_VL_ONEXEC
> > > > +Defer the change, so that it is performed at the next
> > > > +.BR execve (2)
> > > > +in the thread.
> > > > +Further
> > > > +.BR execve (2)
> > > > +calls will reset the vector length to the value configured in
> > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > > +.TP
> > > > +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> > > > +Defer the change, so that it is performed at the next
> > > > +.BR execve (2)
> > > > +in the thread.
> > > > +Further
> > > > +.BR execve (2)
> > > > +calls will preserve the new vector length.
> > > > +.RE
> > > > +.IP
> > > > +In all cases,
> > > > +any previously pending deferred change is canceled.
> > > > +.IP
> > > > +The call fails with error
> > > > +.B EINVAL
> > > > +if SVE is not supported on the platform, if
> > > > +.I arg2
> > > > +is unrecognized or invalid, or the value in the bits of
> > > > +.I arg2
> > > > +corresponding to
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +is outside the range
> > > > +.BR SVE_VL_MIN .. SVE_VL_MAX
> > > > +or is not a multiple of 16.
> > > > +.IP
> > > > +On success,
> > > > +a nonnegative value is returned that describes the
> > > > +.I selected
> > > > +configuration,
> > > 
> > > If I'm reading the kernel code correctly, this is slightly weird, as
> > > the returned value may contain the PR_SVE_VL_INHERIT flag but it will
> > > never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?
> > 
> > Yes, which is an oddity.
> > 
> > I suppose we could fake that up actually by returning that flag if
> > sve_vl and sve_vl_onexec are different, but we don't currently do this.
> 
> I don't think there's any need to change the code, but I think this stuff
> is worth documenting.
> 
> > > If so, maybe just say something like:
> > > 
> > >   On success, a nonnegative value is returned that describes the selected
> > >   configuration in the same way as PR_SVE_GET_VL.
> > 
> > How does that help?  PR_SVE_GET_VL doesn't fully clarify the oddity you
> > call out anyway.
> 
> It clarifies it enough for my liking (by explicitly talking about "the bit
> corresponding to PR_SVE_VL_INHERIT" and not about PR_SVE_SET_VL_ONEXEC),
> but either way, I think saying that the return value is the same is a
> useful clarification. If you want to make PR_SVE_GET_VL more explicit,
> we could do that too.

Fair enough.  I'll just refer to PR_SVE_GET_VL, as you suggest.

I'm not keen to add any new wording at this stage.

> > > > +.B PR_SVE_SET_VL_ONEXEC
> > > > +flag may crash the calling process.
> > > > +The conditions for using it safely are complex and system-dependent.
> > > > +Don't use it unless you really know what you are doing.
> > > > +.IP
> > > > +For more information, see the kernel source file
> > > > +.I Documentation/arm64/sve.rst
> > > > +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> > > > +(or
> > > > +.I Documentation/arm64/sve.txt
> > > > +before Linux 5.3).
> > > 
> > > I think I'd drop the kernel reference here, as it feels like we're saying
> > > "only do this if you know what you're doing" on one hand, but then "if you
> > > don't know what you're doing, see this other documentation" on the other.
> > 
> > Well, the docmuentation doesn't answer those questions either.
> > 
> > I could just swap the warning and the cross-reference, so that the
> > cross-reference doesn't seem to follow on from "knowing what you're
> > doing"?
> 
> Ok.

OK, I'll aim to do that then.

Cheers
---Dave

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 03:49:05PM +0100, Will Deacon wrote:
> On Tue, Jun 09, 2020 at 03:11:42PM +0100, Dave Martin wrote:
> > On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> > > On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > > > diff --git a/man2/prctl.2 b/man2/prctl.2
> > > > index cab9915..91df7c8 100644
> > > > --- a/man2/prctl.2
> > > > +++ b/man2/prctl.2
> > > > @@ -1291,6 +1291,148 @@ call failing with the error
> > > >  .BR ENXIO .
> > > >  For further details, see the kernel source file
> > > >  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> > > > +.\" prctl PR_SVE_SET_VL
> > > > +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> > > > +.\" linux-5.6/Documentation/arm64/sve.rst
> > > > +.TP
> > > > +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> > > > +Configure the thread's SVE vector length,
> > > > +as specified by
> > > > +.IR "(int) arg2" .
> > > > +Arguments
> > > > +.IR arg3 ", " arg4 " and " arg5
> > > > +are ignored.
> > > > +.IP
> > > > +The bits of
> > > > +.I arg2
> > > > +corresponding to
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +must be set to the desired vector length in bytes.
> > > > +This is interpreted as an upper bound:
> > > > +the kernel will select the greatest available vector length
> > > > +that does not exceed the value specified.
> > > > +In particular, specifying
> > > > +.B SVE_VL_MAX
> > > > +(defined in
> > > > +.I <asm/sigcontext.h>)
> > > > +for the
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +bits requests the maximum supported vector length.
> > > > +.IP
> > > > +In addition,
> > > > +.I arg2
> > > > +must be set to one of the following combinations of flags:
> > > 
> > > How about saying:
> > > 
> > >   In addition, the other bits of arg2 must be set according to the following
> > >   combinations of flags:
> > > 
> > > Otherwise I find it a bit fiddly to read, because it's valid to have
> > > flags of 0 and a non-zero length.
> > 
> > 0 is listed, so I hoped that was clear enough.
> > 
> > Maybe just write "must be one of the following values:"?
> > 
> > 0 is a value, but I can see why you might be uneasy about 0 being
> > described as a "combination of flags".
> 
> It's more that arg2 *also* holds the length, so saying that arg2 must
> be set to a combination of flags isn't quite right, because it's actually
> to set to a combination of flags and the length.
> 
> > > > +.RS
> > > > +.TP
> > > > +.B 0
> > > > +Perform the change immediately.
> > > > +At the next
> > > > +.BR execve (2)
> > > > +in the thread,
> > > > +the vector length will be reset to the value configured in
> > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > 
> > > (implementation note: does this mean that 'sve_default_vl' should be
> > >  an atomic_t, as it can be accessed concurrently? We probably need
> > >  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
> > >  that help us here...)
> > 
> > Is this purely theoretical?  Can you point to what could go wrong?
> 
> If the write is torn by the compiler, then a concurrent reader could end
> up seeing a bogus value. There could also be ToCToU issues if it's re-read.

It won't be torn in practice, no decision logic depends on the value
read, and you can't even get from the write to the read or vice-versa
without crossing a TU boundary (even under LTO), so there's basically
zero scope for sabotXXXXXoptimisation by the compiler.

Only root is allowed to write this thing anyway.

> > While I doubt I thought about this very hard and I agree that you're
> > right in principle, I think there are probably non-atomic sysctls and
> > debugs files etc. all over the place.
> > 
> > I didn't want to clutter the code unnecessarily.
> 
> Right, but KCSAN is coming along and so somebody less familiar with the code
> will hit this eventually.

So the issue is theoretical, probably one of very many similar issues,
and anyway we have a tool for tracking them down if we need to?

I'm playing devil's advocate here, but I'd debate whether it's worth
it -- or even wise -- to fix these piecemeal unless we're confident this
is an egregious case.  Doing so may encourage a false sense of safety.
When we're in a position to do a treewide cleanup, that would be better,
no?

> > > > +.B PR_SVE_VL_INHERIT
> > > > +Perform the change immediately.
> > > > +Subsequent
> > > > +.BR execve (2)
> > > > +calls will preserve the new vector length.
> > > > +.TP
> > > > +.B PR_SVE_SET_VL_ONEXEC
> > > > +Defer the change, so that it is performed at the next
> > > > +.BR execve (2)
> > > > +in the thread.
> > > > +Further
> > > > +.BR execve (2)
> > > > +calls will reset the vector length to the value configured in
> > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > > +.TP
> > > > +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> > > > +Defer the change, so that it is performed at the next
> > > > +.BR execve (2)
> > > > +in the thread.
> > > > +Further
> > > > +.BR execve (2)
> > > > +calls will preserve the new vector length.
> > > > +.RE
> > > > +.IP
> > > > +In all cases,
> > > > +any previously pending deferred change is canceled.
> > > > +.IP
> > > > +The call fails with error
> > > > +.B EINVAL
> > > > +if SVE is not supported on the platform, if
> > > > +.I arg2
> > > > +is unrecognized or invalid, or the value in the bits of
> > > > +.I arg2
> > > > +corresponding to
> > > > +.B PR_SVE_VL_LEN_MASK
> > > > +is outside the range
> > > > +.BR SVE_VL_MIN .. SVE_VL_MAX
> > > > +or is not a multiple of 16.
> > > > +.IP
> > > > +On success,
> > > > +a nonnegative value is returned that describes the
> > > > +.I selected
> > > > +configuration,
> > > 
> > > If I'm reading the kernel code correctly, this is slightly weird, as
> > > the returned value may contain the PR_SVE_VL_INHERIT flag but it will
> > > never contain the PR_SVE_SET_VL_ONEXEC flag. Is that right?
> > 
> > Yes, which is an oddity.
> > 
> > I suppose we could fake that up actually by returning that flag if
> > sve_vl and sve_vl_onexec are different, but we don't currently do this.
> 
> I don't think there's any need to change the code, but I think this stuff
> is worth documenting.
> 
> > > If so, maybe just say something like:
> > > 
> > >   On success, a nonnegative value is returned that describes the selected
> > >   configuration in the same way as PR_SVE_GET_VL.
> > 
> > How does that help?  PR_SVE_GET_VL doesn't fully clarify the oddity you
> > call out anyway.
> 
> It clarifies it enough for my liking (by explicitly talking about "the bit
> corresponding to PR_SVE_VL_INHERIT" and not about PR_SVE_SET_VL_ONEXEC),
> but either way, I think saying that the return value is the same is a
> useful clarification. If you want to make PR_SVE_GET_VL more explicit,
> we could do that too.

Fair enough.  I'll just refer to PR_SVE_GET_VL, as you suggest.

I'm not keen to add any new wording at this stage.

> > > > +.B PR_SVE_SET_VL_ONEXEC
> > > > +flag may crash the calling process.
> > > > +The conditions for using it safely are complex and system-dependent.
> > > > +Don't use it unless you really know what you are doing.
> > > > +.IP
> > > > +For more information, see the kernel source file
> > > > +.I Documentation/arm64/sve.rst
> > > > +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> > > > +(or
> > > > +.I Documentation/arm64/sve.txt
> > > > +before Linux 5.3).
> > > 
> > > I think I'd drop the kernel reference here, as it feels like we're saying
> > > "only do this if you know what you're doing" on one hand, but then "if you
> > > don't know what you're doing, see this other documentation" on the other.
> > 
> > Well, the docmuentation doesn't answer those questions either.
> > 
> > I could just swap the warning and the cross-reference, so that the
> > cross-reference doesn't seem to follow on from "knowing what you're
> > doing"?
> 
> Ok.

OK, I'll aim to do that then.

Cheers
---Dave

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Will Deacon]

[Dropped linux-man and Michael]

On Wed, Jun 10, 2020 at 10:44:42AM +0100, Dave Martin wrote:
> On Tue, Jun 09, 2020 at 03:49:05PM +0100, Will Deacon wrote:
> > On Tue, Jun 09, 2020 at 03:11:42PM +0100, Dave Martin wrote:
> > > On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> > > > On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > > > > +.RS
> > > > > +.TP
> > > > > +.B 0
> > > > > +Perform the change immediately.
> > > > > +At the next
> > > > > +.BR execve (2)
> > > > > +in the thread,
> > > > > +the vector length will be reset to the value configured in
> > > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > > 
> > > > (implementation note: does this mean that 'sve_default_vl' should be
> > > >  an atomic_t, as it can be accessed concurrently? We probably need
> > > >  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
> > > >  that help us here...)
> > > 
> > > Is this purely theoretical?  Can you point to what could go wrong?
> > 
> > If the write is torn by the compiler, then a concurrent reader could end
> > up seeing a bogus value. There could also be ToCToU issues if it's re-read.
> 
> It won't be torn in practice, no decision logic depends on the value
> read, and you can't even get from the write to the read or vice-versa
> without crossing a TU boundary (even under LTO), so there's basically
> zero scope for sabotXXXXXoptimisation by the compiler.

Perhaps, but I'm not brave enough to state that :) Look at this crazy
thing, for example:

https://lore.kernel.org/lkml/CAG48ez2nFks+yN1Kp4TZisso+rjvv_4UW0FTo8iFUd4Qyq1qDw@mail.gmail.com/

Could the same sort of technique be applied to:


	vl = current->thread.sve_vl_onexec ?
	     current->thread.sve_vl_onexec : sve_default_vl;

	if (WARN_ON(!sve_vl_valid(vl)))
		vl = SVE_VL_MIN;

	supported_vl = find_supported_vector_length(vl);


so that the compiled code does something like:


	if (within_valid_bounds(sve_default_vl)) {
		supported_vl = jump_table(sve_default_vl); // Reload the variable
	} else {
		WARN_ON(1);
		supported_vl = SVE_VL_MIN;
	}


?

I'd certainly prefer not to have to think about that!

> Only root is allowed to write this thing anyway.
> 
> > > While I doubt I thought about this very hard and I agree that you're
> > > right in principle, I think there are probably non-atomic sysctls and
> > > debugs files etc. all over the place.
> > > 
> > > I didn't want to clutter the code unnecessarily.
> > 
> > Right, but KCSAN is coming along and so somebody less familiar with the code
> > will hit this eventually.
> 
> So the issue is theoretical, probably one of very many similar issues,
> and anyway we have a tool for tracking them down if we need to?
> 
> I'm playing devil's advocate here, but I'd debate whether it's worth
> it -- or even wise -- to fix these piecemeal unless we're confident this
> is an egregious case.  Doing so may encourage a false sense of safety.
> When we're in a position to do a treewide cleanup, that would be better,
> no?

That's a good point, but it is inevitable that people will try to attempt
treewide introduction of {READ,WRITE}_ONCE() based solely on KCSAN reports
rather than an understanding of the code, and so I'd much rather somebody
who understands the code (that's you ;) deals with it first.

If the race is benign, then you can annotate the accesses with data_race()
and add a comment along the lines of your "It won't be torn in practice..."
paragraph above.

Anyway, this is entirely independent to the manpage effort, just that the
concurrency wasn't clear to me before I read what you'd written and thought
I'd mention this before I forget. It's also looking less likely that KCSAN
is going to land for 5.8, so there's no urgency to this at all.

Will

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Wed, Jun 10, 2020 at 11:16:49AM +0100, Will Deacon wrote:
> [Dropped linux-man and Michael]
> 
> On Wed, Jun 10, 2020 at 10:44:42AM +0100, Dave Martin wrote:
> > On Tue, Jun 09, 2020 at 03:49:05PM +0100, Will Deacon wrote:
> > > On Tue, Jun 09, 2020 at 03:11:42PM +0100, Dave Martin wrote:
> > > > On Tue, Jun 09, 2020 at 10:57:35AM +0100, Will Deacon wrote:
> > > > > On Wed, May 27, 2020 at 10:17:36PM +0100, Dave Martin wrote:
> > > > > > +.RS
> > > > > > +.TP
> > > > > > +.B 0
> > > > > > +Perform the change immediately.
> > > > > > +At the next
> > > > > > +.BR execve (2)
> > > > > > +in the thread,
> > > > > > +the vector length will be reset to the value configured in
> > > > > > +.IR /proc/sys/abi/sve_default_vector_length .
> > > > > 
> > > > > (implementation note: does this mean that 'sve_default_vl' should be
> > > > >  an atomic_t, as it can be accessed concurrently? We probably need
> > > > >  {READ,WRITE}_ONCE() at the very least, as I'm not seeing any locks
> > > > >  that help us here...)
> > > > 
> > > > Is this purely theoretical?  Can you point to what could go wrong?
> > > 
> > > If the write is torn by the compiler, then a concurrent reader could end
> > > up seeing a bogus value. There could also be ToCToU issues if it's re-read.
> > 
> > It won't be torn in practice, no decision logic depends on the value
> > read, and you can't even get from the write to the read or vice-versa
> > without crossing a TU boundary (even under LTO), so there's basically
> > zero scope for sabotXXXXXoptimisation by the compiler.
> 
> Perhaps, but I'm not brave enough to state that :) Look at this crazy
> thing, for example:
> 
> https://lore.kernel.org/lkml/CAG48ez2nFks+yN1Kp4TZisso+rjvv_4UW0FTo8iFUd4Qyq1qDw@mail.gmail.com/
> 
> Could the same sort of technique be applied to:
> 
> 
> 	vl = current->thread.sve_vl_onexec ?
> 	     current->thread.sve_vl_onexec : sve_default_vl;
> 
> 	if (WARN_ON(!sve_vl_valid(vl)))
> 		vl = SVE_VL_MIN;
> 
> 	supported_vl = find_supported_vector_length(vl);
> 
> 
> so that the compiled code does something like:
> 
> 
> 	if (within_valid_bounds(sve_default_vl)) {
> 		supported_vl = jump_table(sve_default_vl); // Reload the variable
> 	} else {
> 		WARN_ON(1);
> 		supported_vl = SVE_VL_MIN;
> 	}
> 
> 
> ?
> 
> I'd certainly prefer not to have to think about that!

Well sure, but the compiler has much to lose and nothing to gain from
such a transformation here.  This is a bit different from a load of
conditional code that can be heavily const-folded during specialisation.

Anyway, I'm not saying that you're not correct about the risk, just that
this feels like a common pattern.


> > Only root is allowed to write this thing anyway.
> > 
> > > > While I doubt I thought about this very hard and I agree that you're
> > > > right in principle, I think there are probably non-atomic sysctls and
> > > > debugs files etc. all over the place.
> > > > 
> > > > I didn't want to clutter the code unnecessarily.
> > > 
> > > Right, but KCSAN is coming along and so somebody less familiar with the code
> > > will hit this eventually.
> > 
> > So the issue is theoretical, probably one of very many similar issues,
> > and anyway we have a tool for tracking them down if we need to?
> > 
> > I'm playing devil's advocate here, but I'd debate whether it's worth
> > it -- or even wise -- to fix these piecemeal unless we're confident this
> > is an egregious case.  Doing so may encourage a false sense of safety.
> > When we're in a position to do a treewide cleanup, that would be better,
> > no?
> 
> That's a good point, but it is inevitable that people will try to attempt
> treewide introduction of {READ,WRITE}_ONCE() based solely on KCSAN reports
> rather than an understanding of the code, and so I'd much rather somebody
> who understands the code (that's you ;) deals with it first.
> 
> If the race is benign, then you can annotate the accesses with data_race()
> and add a comment along the lines of your "It won't be torn in practice..."
> paragraph above.

Oh, it's complex enough to reason about that we should definitely use
proper atomics here so that we don't have to think about it.  Also, I'd
concede that the fact that this code has a custom sysctl accessor may
make justify a special case fix.

For most users, it would be better to clip sysctl's wings so that only
atomic accesses are allowed if the default implementation is used.  sysctl
is not a fast path: for single values of fundamental types, there's no
reason I can think of not to use atomics across the board.


> Anyway, this is entirely independent to the manpage effort, just that the
> concurrency wasn't clear to me before I read what you'd written and thought
> I'd mention this before I forget. It's also looking less likely that KCSAN
> is going to land for 5.8, so there's no urgency to this at all.

Sure, and I don't think I thought much beyond "I wonder what happens if
... nah, probably fine, if it mattered then everyone would be doing it."

I'm pretty sure I didn't get that wording out of the C spec.

It's a good spot though, and I may look at a fix if I get around to it.
Can't promise when, though.

Cheers
---Dave

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Michael Kerrisk (man-pages)]

Hi Dave,

I've not applied this patch yet, in case you want to make
some changes in response to Will's comments.

I think all of the rest of the patches in the series are now
applied (and pushed to master).

Thanks,

Michael

On 5/27/20 11:17 PM, Dave Martin wrote:
> Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
> prctls added in Linux 4.15 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> ---
> 
> Since v1:
> 
>  * Minor rewordings and typo fixes.
> 
>  * Fix typo'd #define names.
> 
>  * Add type annotation for PR_SVE_SET_VL arg2.
> 
>  * Clarify return value semantics of PR_SVE_SET_VL
> 
>  * Add note to say that the args for PR_SVE_GET_VL are ignored.
> 
>  * Note for PR_SVE_SET_VL that the PR_SVE_VL_LEN_MASK field specifies
>    an upper bound on what vector length to set, not an exact value.
> 
>  * Rework PR_SVE_SET_VL arg2 description to enumerate all possible flag
>    combinations rather than describing the flags independently.
> 
>    Coming up with a clear description of each flag that is independent
>    of the description of the other flag turns out to be hard.
> 
>  * In lieu of having a separate man page to cross reference for detailed
>    guidance, cross-reference the kernel documentation.
> 
>  * Avoid confusing cross-reference to PR_SVE_SET_VL when describing the
>    return value of PR_SVE_GET_VL.
> 
>  * Clarify error conditions for PR_SVE_SET_VL and PR_SVE_GET_VL, and
>    move detail to the individual prctl descriptions to keep related
>    content close together while minimising duplication.
> 
>  * Add safety warning.  This is deliberately vague, pending ongoing
>    discussions with libc folks.
> ---
>  man2/prctl.2 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index cab9915..91df7c8 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1291,6 +1291,148 @@ call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
>  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> +.\" prctl PR_SVE_SET_VL
> +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> +.\" linux-5.6/Documentation/arm64/sve.rst
> +.TP
> +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> +Configure the thread's SVE vector length,
> +as specified by
> +.IR "(int) arg2" .
> +Arguments
> +.IR arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +The bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +must be set to the desired vector length in bytes.
> +This is interpreted as an upper bound:
> +the kernel will select the greatest available vector length
> +that does not exceed the value specified.
> +In particular, specifying
> +.B SVE_VL_MAX
> +(defined in
> +.I <asm/sigcontext.h>)
> +for the
> +.B PR_SVE_VL_LEN_MASK
> +bits requests the maximum supported vector length.
> +.IP
> +In addition,
> +.I arg2
> +must be set to one of the following combinations of flags:
> +.RS
> +.TP
> +.B 0
> +Perform the change immediately.
> +At the next
> +.BR execve (2)
> +in the thread,
> +the vector length will be reset to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B PR_SVE_VL_INHERIT
> +Perform the change immediately.
> +Subsequent
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.TP
> +.B PR_SVE_SET_VL_ONEXEC
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will reset the vector length to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.RE
> +.IP
> +In all cases,
> +any previously pending deferred change is canceled.
> +.IP
> +The call fails with error
> +.B EINVAL
> +if SVE is not supported on the platform, if
> +.I arg2
> +is unrecognized or invalid, or the value in the bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +is outside the range
> +.BR SVE_VL_MIN .. SVE_VL_MAX
> +or is not a multiple of 16.
> +.IP
> +On success,
> +a nonnegative value is returned that describes the
> +.I selected
> +configuration,
> +which may differ from the current configuration if
> +.B PR_SVE_SET_VL_ONEXEC
> +was specified.
> +The value is encoded in the same way as the return value of
> +.BR PR_SVE_GET_VL .
> +.IP
> +The configuration (including any pending deferred change)
> +is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using SVE, using this call without the
> +.B PR_SVE_SET_VL_ONEXEC
> +flag may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you really know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).
> +.\" prctl PR_SVE_GET_VL
> +.TP
> +.BR PR_SVE_GET_VL " (since Linux 4.15, only on arm64)"
> +Get the thread's current SVE vector length configuration.
> +.IP
> +Arguments
> +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +Providing that the kernel and platform support SVE
> +this operation always succeeds,
> +returning a nonnegative value that describes the
> +.I current
> +configuration.
> +The bits corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +contain the currently configured vector length in bytes.
> +The bit corresponding to
> +.B PR_SVE_VL_INHERIT
> +indicates whether the vector length will be inherited
> +across
> +.BR execve (2).
> +.IP
> +Note that there is no way to determine whether there is
> +a pending vector length change that has not yet taken effect.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP
> @@ -1534,6 +1676,8 @@ On success,
>  .BR PR_GET_NO_NEW_PRIVS ,
>  .BR PR_GET_SECUREBITS ,
>  .BR PR_GET_SPECULATION_CTRL ,
> +.BR PR_SVE_GET_VL ,
> +.BR PR_SVE_SET_VL ,
>  .BR PR_GET_THP_DISABLE ,
>  .BR PR_GET_TIMING ,
>  .BR PR_GET_TIMERSLACK ,
> @@ -1817,6 +1961,22 @@ and unused arguments to
>  .BR prctl ()
>  are not 0.
>  .TP
> +.B EINVAL
> +.I option
> +is
> +.B PR_SVE_SET_VL
> +and the arguments are invalid or unsupported,
> +or SVE is not available on this platform.
> +See the description of
> +.B PR_SVE_SET_VL
> +above for details.
> +.TP
> +.B EINVAL
> +.I option
> +is
> +.B PR_SVE_GET_VL
> +and SVE is not available on this platform.
> +.TP
>  .B ENODEV
>  .I option
>  was
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Michael Kerrisk (man-pages)]

Hi Dave,

I've not applied this patch yet, in case you want to make
some changes in response to Will's comments.

I think all of the rest of the patches in the series are now
applied (and pushed to master).

Thanks,

Michael

On 5/27/20 11:17 PM, Dave Martin wrote:
> Add documentation for the the PR_SVE_SET_VL and PR_SVE_GET_VL
> prctls added in Linux 4.15 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> 
> ---
> 
> Since v1:
> 
>  * Minor rewordings and typo fixes.
> 
>  * Fix typo'd #define names.
> 
>  * Add type annotation for PR_SVE_SET_VL arg2.
> 
>  * Clarify return value semantics of PR_SVE_SET_VL
> 
>  * Add note to say that the args for PR_SVE_GET_VL are ignored.
> 
>  * Note for PR_SVE_SET_VL that the PR_SVE_VL_LEN_MASK field specifies
>    an upper bound on what vector length to set, not an exact value.
> 
>  * Rework PR_SVE_SET_VL arg2 description to enumerate all possible flag
>    combinations rather than describing the flags independently.
> 
>    Coming up with a clear description of each flag that is independent
>    of the description of the other flag turns out to be hard.
> 
>  * In lieu of having a separate man page to cross reference for detailed
>    guidance, cross-reference the kernel documentation.
> 
>  * Avoid confusing cross-reference to PR_SVE_SET_VL when describing the
>    return value of PR_SVE_GET_VL.
> 
>  * Clarify error conditions for PR_SVE_SET_VL and PR_SVE_GET_VL, and
>    move detail to the individual prctl descriptions to keep related
>    content close together while minimising duplication.
> 
>  * Add safety warning.  This is deliberately vague, pending ongoing
>    discussions with libc folks.
> ---
>  man2/prctl.2 | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index cab9915..91df7c8 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1291,6 +1291,148 @@ call failing with the error
>  .BR ENXIO .
>  For further details, see the kernel source file
>  .IR Documentation/admin\-guide/kernel\-parameters.txt .
> +.\" prctl PR_SVE_SET_VL
> +.\" commit 2d2123bc7c7f843aa9db87720de159a049839862
> +.\" linux-5.6/Documentation/arm64/sve.rst
> +.TP
> +.BR PR_SVE_SET_VL " (since Linux 4.15, only on arm64)"
> +Configure the thread's SVE vector length,
> +as specified by
> +.IR "(int) arg2" .
> +Arguments
> +.IR arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +The bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +must be set to the desired vector length in bytes.
> +This is interpreted as an upper bound:
> +the kernel will select the greatest available vector length
> +that does not exceed the value specified.
> +In particular, specifying
> +.B SVE_VL_MAX
> +(defined in
> +.I <asm/sigcontext.h>)
> +for the
> +.B PR_SVE_VL_LEN_MASK
> +bits requests the maximum supported vector length.
> +.IP
> +In addition,
> +.I arg2
> +must be set to one of the following combinations of flags:
> +.RS
> +.TP
> +.B 0
> +Perform the change immediately.
> +At the next
> +.BR execve (2)
> +in the thread,
> +the vector length will be reset to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B PR_SVE_VL_INHERIT
> +Perform the change immediately.
> +Subsequent
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.TP
> +.B PR_SVE_SET_VL_ONEXEC
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will reset the vector length to the value configured in
> +.IR /proc/sys/abi/sve_default_vector_length .
> +.TP
> +.B "PR_SVE_SET_VL_ONEXEC | PR_SVE_VL_INHERIT"
> +Defer the change, so that it is performed at the next
> +.BR execve (2)
> +in the thread.
> +Further
> +.BR execve (2)
> +calls will preserve the new vector length.
> +.RE
> +.IP
> +In all cases,
> +any previously pending deferred change is canceled.
> +.IP
> +The call fails with error
> +.B EINVAL
> +if SVE is not supported on the platform, if
> +.I arg2
> +is unrecognized or invalid, or the value in the bits of
> +.I arg2
> +corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +is outside the range
> +.BR SVE_VL_MIN .. SVE_VL_MAX
> +or is not a multiple of 16.
> +.IP
> +On success,
> +a nonnegative value is returned that describes the
> +.I selected
> +configuration,
> +which may differ from the current configuration if
> +.B PR_SVE_SET_VL_ONEXEC
> +was specified.
> +The value is encoded in the same way as the return value of
> +.BR PR_SVE_GET_VL .
> +.IP
> +The configuration (including any pending deferred change)
> +is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using SVE, using this call without the
> +.B PR_SVE_SET_VL_ONEXEC
> +flag may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you really know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).
> +.\" prctl PR_SVE_GET_VL
> +.TP
> +.BR PR_SVE_GET_VL " (since Linux 4.15, only on arm64)"
> +Get the thread's current SVE vector length configuration.
> +.IP
> +Arguments
> +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> +are ignored.
> +.IP
> +Providing that the kernel and platform support SVE
> +this operation always succeeds,
> +returning a nonnegative value that describes the
> +.I current
> +configuration.
> +The bits corresponding to
> +.B PR_SVE_VL_LEN_MASK
> +contain the currently configured vector length in bytes.
> +The bit corresponding to
> +.B PR_SVE_VL_INHERIT
> +indicates whether the vector length will be inherited
> +across
> +.BR execve (2).
> +.IP
> +Note that there is no way to determine whether there is
> +a pending vector length change that has not yet taken effect.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/sve.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/sve.txt
> +before Linux 5.3).
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP
> @@ -1534,6 +1676,8 @@ On success,
>  .BR PR_GET_NO_NEW_PRIVS ,
>  .BR PR_GET_SECUREBITS ,
>  .BR PR_GET_SPECULATION_CTRL ,
> +.BR PR_SVE_GET_VL ,
> +.BR PR_SVE_SET_VL ,
>  .BR PR_GET_THP_DISABLE ,
>  .BR PR_GET_TIMING ,
>  .BR PR_GET_TIMERSLACK ,
> @@ -1817,6 +1961,22 @@ and unused arguments to
>  .BR prctl ()
>  are not 0.
>  .TP
> +.B EINVAL
> +.I option
> +is
> +.B PR_SVE_SET_VL
> +and the arguments are invalid or unsupported,
> +or SVE is not available on this platform.
> +See the description of
> +.B PR_SVE_SET_VL
> +above for details.
> +.TP
> +.B EINVAL
> +.I option
> +is
> +.B PR_SVE_GET_VL
> +and SVE is not available on this platform.
> +.TP
>  .B ENODEV
>  .I option
>  was
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 01:39:03PM +0200, Michael Kerrisk (man-pages) wrote:
> Hi Dave,
> 
> I've not applied this patch yet, in case you want to make
> some changes in response to Will's comments.
> 
> I think all of the rest of the patches in the series are now
> applied (and pushed to master).
> 
> Thanks,

Ack, thanks
---Dave

Re: [PATCH v2 4/6] prctl.2: Add SVE prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 01:39:03PM +0200, Michael Kerrisk (man-pages) wrote:
> Hi Dave,
> 
> I've not applied this patch yet, in case you want to make
> some changes in response to Will's comments.
> 
> I think all of the rest of the patches in the series are now
> applied (and pushed to master).
> 
> Thanks,

Ack, thanks
---Dave

[RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

** This patch is a draft for review and should not be applied before it
   has been discussed. **

Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.

Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Vincenzo Frascino <vincenzo.frascino-5wv7dgnIgG8@public.gmane.org>
---

 man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/man2/prctl.2 b/man2/prctl.2
index 3ee2702..062fd51 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1504,6 +1504,143 @@ For more information, see the kernel source file
 (or
 .I Documentation/arm64/sve.txt
 before Linux 5.3).
+.\" prctl PR_SET_TAGGED_ADDR_CTRL
+.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
+.TP
+.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
+Controls support for passing tagged userspace addresses to the kernel
+(i.e., addresses where bits 56\(em63 are not all zero).
+.IP
+The level of support is selected by
+.IR "(unsigned int) arg2" ,
+which can be one of the following:
+.RS
+.TP
+.B 0
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+must be untagged.
+.TP
+.B PR_TAGGED_ADDR_ENABLE
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+may be tagged, with the exceptions summarized below.
+.RE
+.IP
+The remaining arguments
+.IR arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+On success, the mode specified in
+.I arg2
+is set for the calling thread and the the return value is 0.
+If the arguments are invalid,
+the mode specified in
+.I arg2
+is unrecognized,
+or if this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+.IP
+In particular, if
+.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.B EINVAL
+then all addresses passed to the kernel must be untagged.
+.IP
+Irrespective of which mode is set,
+addresses passed to certain interfaces
+must always be untagged:
+.RS
+.IP \(em
+.BR brk (2),
+.BR mmap (2),
+.BR shmat (2),
+and the
+.I new_address
+argument of
+.BR mremap (2).
+.IP
+(Prior to Linux 5.6 these accepted tagged addresses,
+but the behaviour may not be what you expect.
+Don't rely on it.)
+.IP \(em
+\(oqpolymorphic\(cq interfaces
+that accept pointers to arbitrary types cast to a
+.I void *
+or other generic type, specifically
+.BR prctl (2),
+.BR ioctl (2),
+and in general
+.BR setsockopt (2)
+(only certain specific
+.BR setsockopt (2)
+options allow tagged addresses).
+.IP \(em
+.BR shmdt (2).
+.RE
+.IP
+This list of exclusions may shrink
+when moving from one kernel version to a later kernel version.
+While the kernel may make some guarantees
+for backwards compatibility reasons,
+for the purposes of new software
+the effect of passing tagged addresses to these interfaces
+is unspecified.
+.IP
+The mode set by this call is inherited across
+.BR fork (2)
+and
+.BR clone (2).
+The mode is reset by
+.BR execve (2)
+to 0
+(i.e., tagged addresses not permitted in the user/kernel ABI).
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may make use of address tagging,
+a successful
+.B PR_SET_TAGGED_ADDR_CTRL
+may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.
+.IP
+For more information, see the kernel source file
+.IR Documentation/arm64/tagged\-address\-abi.rst .
+.\" prctl PR_GET_TAGGED_ADDR_CTRL
+.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
+.TP
+.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
+Returns the current tagged address mode
+for the calling thread.
+.IP
+Arguments
+.IR arg2 ", " arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+If the arguments are invalid
+or this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+In particular, if
+.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.BR EINVAL ,
+then this feature is definitely unsupported or disabled,
+and all addresses passed to the kernel must be untagged.
+.IP
+Otherwise, the call returns a nonnegative value
+describing the current tagged address mode,
+encoded in the same way as the
+.I arg2
+argument of
+.BR PR_SET_TAGGED_ADDR_CTRL .
+.IP
+For more information, see the kernel source file
+.IR Documentation/arm64/tagged\-address\-abi.rst .
 .\"
 .\" prctl PR_TASK_PERF_EVENTS_DISABLE
 .TP
@@ -1749,6 +1886,7 @@ On success,
 .BR PR_GET_SPECULATION_CTRL ,
 .BR PR_SVE_GET_VL ,
 .BR PR_SVE_SET_VL ,
+.BR PR_GET_TAGGED_ADDR_CTRL ,
 .BR PR_GET_THP_DISABLE ,
 .BR PR_GET_TIMING ,
 .BR PR_GET_TIMERSLACK ,
@@ -2057,6 +2195,24 @@ is
 .B PR_SVE_GET_VL
 and SVE is not available on this platform.
 .TP
+.B EINVAL
+.I option
+is
+.BR PR_SET_TAGGED_ADDR_CTRL
+and the arguments are invalid or unsupported.
+See the description of
+.B PR_SET_TAGGED_ADDR_CTRL
+above for details.
+.TP
+.B EINVAL
+.I option
+is
+.BR PR_GET_TAGGED_ADDR_CTRL
+and the arguments are invalid or unsupported.
+See the description of
+.B PR_GET_TAGGED_ADDR_CTRL
+above for details.
+.TP
 .B ENODEV
 .I option
 was
-- 
2.1.4

[RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

** This patch is a draft for review and should not be applied before it
   has been discussed. **

Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
---

 man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/man2/prctl.2 b/man2/prctl.2
index 3ee2702..062fd51 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1504,6 +1504,143 @@ For more information, see the kernel source file
 (or
 .I Documentation/arm64/sve.txt
 before Linux 5.3).
+.\" prctl PR_SET_TAGGED_ADDR_CTRL
+.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
+.TP
+.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
+Controls support for passing tagged userspace addresses to the kernel
+(i.e., addresses where bits 56\(em63 are not all zero).
+.IP
+The level of support is selected by
+.IR "(unsigned int) arg2" ,
+which can be one of the following:
+.RS
+.TP
+.B 0
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+must be untagged.
+.TP
+.B PR_TAGGED_ADDR_ENABLE
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+may be tagged, with the exceptions summarized below.
+.RE
+.IP
+The remaining arguments
+.IR arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+On success, the mode specified in
+.I arg2
+is set for the calling thread and the the return value is 0.
+If the arguments are invalid,
+the mode specified in
+.I arg2
+is unrecognized,
+or if this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+.IP
+In particular, if
+.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.B EINVAL
+then all addresses passed to the kernel must be untagged.
+.IP
+Irrespective of which mode is set,
+addresses passed to certain interfaces
+must always be untagged:
+.RS
+.IP \(em
+.BR brk (2),
+.BR mmap (2),
+.BR shmat (2),
+and the
+.I new_address
+argument of
+.BR mremap (2).
+.IP
+(Prior to Linux 5.6 these accepted tagged addresses,
+but the behaviour may not be what you expect.
+Don't rely on it.)
+.IP \(em
+\(oqpolymorphic\(cq interfaces
+that accept pointers to arbitrary types cast to a
+.I void *
+or other generic type, specifically
+.BR prctl (2),
+.BR ioctl (2),
+and in general
+.BR setsockopt (2)
+(only certain specific
+.BR setsockopt (2)
+options allow tagged addresses).
+.IP \(em
+.BR shmdt (2).
+.RE
+.IP
+This list of exclusions may shrink
+when moving from one kernel version to a later kernel version.
+While the kernel may make some guarantees
+for backwards compatibility reasons,
+for the purposes of new software
+the effect of passing tagged addresses to these interfaces
+is unspecified.
+.IP
+The mode set by this call is inherited across
+.BR fork (2)
+and
+.BR clone (2).
+The mode is reset by
+.BR execve (2)
+to 0
+(i.e., tagged addresses not permitted in the user/kernel ABI).
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may make use of address tagging,
+a successful
+.B PR_SET_TAGGED_ADDR_CTRL
+may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.
+.IP
+For more information, see the kernel source file
+.IR Documentation/arm64/tagged\-address\-abi.rst .
+.\" prctl PR_GET_TAGGED_ADDR_CTRL
+.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
+.TP
+.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
+Returns the current tagged address mode
+for the calling thread.
+.IP
+Arguments
+.IR arg2 ", " arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+If the arguments are invalid
+or this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+In particular, if
+.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.BR EINVAL ,
+then this feature is definitely unsupported or disabled,
+and all addresses passed to the kernel must be untagged.
+.IP
+Otherwise, the call returns a nonnegative value
+describing the current tagged address mode,
+encoded in the same way as the
+.I arg2
+argument of
+.BR PR_SET_TAGGED_ADDR_CTRL .
+.IP
+For more information, see the kernel source file
+.IR Documentation/arm64/tagged\-address\-abi.rst .
 .\"
 .\" prctl PR_TASK_PERF_EVENTS_DISABLE
 .TP
@@ -1749,6 +1886,7 @@ On success,
 .BR PR_GET_SPECULATION_CTRL ,
 .BR PR_SVE_GET_VL ,
 .BR PR_SVE_SET_VL ,
+.BR PR_GET_TAGGED_ADDR_CTRL ,
 .BR PR_GET_THP_DISABLE ,
 .BR PR_GET_TIMING ,
 .BR PR_GET_TIMERSLACK ,
@@ -2057,6 +2195,24 @@ is
 .B PR_SVE_GET_VL
 and SVE is not available on this platform.
 .TP
+.B EINVAL
+.I option
+is
+.BR PR_SET_TAGGED_ADDR_CTRL
+and the arguments are invalid or unsupported.
+See the description of
+.B PR_SET_TAGGED_ADDR_CTRL
+above for details.
+.TP
+.B EINVAL
+.I option
+is
+.BR PR_GET_TAGGED_ADDR_CTRL
+and the arguments are invalid or unsupported.
+See the description of
+.B PR_GET_TAGGED_ADDR_CTRL
+above for details.
+.TP
 .B ENODEV
 .I option
 was
-- 
2.1.4

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Michael Kerrisk (man-pages)]

Hey folks,

Do we have any review comments for this (extensive!) patch from Dave?

Cheers,

Michael


On 5/27/20 11:17 PM, Dave Martin wrote:
> ** This patch is a draft for review and should not be applied before it
>    has been discussed. **
> 
> Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
> PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Vincenzo Frascino <vincenzo.frascino-5wv7dgnIgG8@public.gmane.org>
> ---
> 
>  man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index 3ee2702..062fd51 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
>  (or
>  .I Documentation/arm64/sve.txt
>  before Linux 5.3).
> +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Controls support for passing tagged userspace addresses to the kernel
> +(i.e., addresses where bits 56\(em63 are not all zero).
> +.IP
> +The level of support is selected by
> +.IR "(unsigned int) arg2" ,
> +which can be one of the following:
> +.RS
> +.TP
> +.B 0
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +must be untagged.
> +.TP
> +.B PR_TAGGED_ADDR_ENABLE
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +may be tagged, with the exceptions summarized below.
> +.RE
> +.IP
> +The remaining arguments
> +.IR arg3 ", " arg4 " and " arg5
> +must all be zero.
> +.IP
> +On success, the mode specified in
> +.I arg2
> +is set for the calling thread and the the return value is 0.
> +If the arguments are invalid,
> +the mode specified in
> +.I arg2
> +is unrecognized,
> +or if this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +.IP
> +In particular, if
> +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.B EINVAL
> +then all addresses passed to the kernel must be untagged.
> +.IP
> +Irrespective of which mode is set,
> +addresses passed to certain interfaces
> +must always be untagged:
> +.RS
> +.IP \(em
> +.BR brk (2),
> +.BR mmap (2),
> +.BR shmat (2),
> +and the
> +.I new_address
> +argument of
> +.BR mremap (2).
> +.IP
> +(Prior to Linux 5.6 these accepted tagged addresses,
> +but the behaviour may not be what you expect.
> +Don't rely on it.)
> +.IP \(em
> +\(oqpolymorphic\(cq interfaces
> +that accept pointers to arbitrary types cast to a
> +.I void *
> +or other generic type, specifically
> +.BR prctl (2),
> +.BR ioctl (2),
> +and in general
> +.BR setsockopt (2)
> +(only certain specific
> +.BR setsockopt (2)
> +options allow tagged addresses).
> +.IP \(em
> +.BR shmdt (2).
> +.RE
> +.IP
> +This list of exclusions may shrink
> +when moving from one kernel version to a later kernel version.
> +While the kernel may make some guarantees
> +for backwards compatibility reasons,
> +for the purposes of new software
> +the effect of passing tagged addresses to these interfaces
> +is unspecified.
> +.IP
> +The mode set by this call is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +The mode is reset by
> +.BR execve (2)
> +to 0
> +(i.e., tagged addresses not permitted in the user/kernel ABI).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may make use of address tagging,
> +a successful
> +.B PR_SET_TAGGED_ADDR_CTRL
> +may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
> +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Returns the current tagged address mode
> +for the calling thread.
> +.IP
> +Arguments
> +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> +must all be zero.
> +.IP
> +If the arguments are invalid
> +or this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +In particular, if
> +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.BR EINVAL ,
> +then this feature is definitely unsupported or disabled,
> +and all addresses passed to the kernel must be untagged.
> +.IP
> +Otherwise, the call returns a nonnegative value
> +describing the current tagged address mode,
> +encoded in the same way as the
> +.I arg2
> +argument of
> +.BR PR_SET_TAGGED_ADDR_CTRL .
> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP
> @@ -1749,6 +1886,7 @@ On success,
>  .BR PR_GET_SPECULATION_CTRL ,
>  .BR PR_SVE_GET_VL ,
>  .BR PR_SVE_SET_VL ,
> +.BR PR_GET_TAGGED_ADDR_CTRL ,
>  .BR PR_GET_THP_DISABLE ,
>  .BR PR_GET_TIMING ,
>  .BR PR_GET_TIMERSLACK ,
> @@ -2057,6 +2195,24 @@ is
>  .B PR_SVE_GET_VL
>  and SVE is not available on this platform.
>  .TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_SET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_SET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_GET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_GET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
>  .B ENODEV
>  .I option
>  was
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Michael Kerrisk (man-pages)]

Hey folks,

Do we have any review comments for this (extensive!) patch from Dave?

Cheers,

Michael


On 5/27/20 11:17 PM, Dave Martin wrote:
> ** This patch is a draft for review and should not be applied before it
>    has been discussed. **
> 
> Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
> PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
> 
>  man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index 3ee2702..062fd51 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
>  (or
>  .I Documentation/arm64/sve.txt
>  before Linux 5.3).
> +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Controls support for passing tagged userspace addresses to the kernel
> +(i.e., addresses where bits 56\(em63 are not all zero).
> +.IP
> +The level of support is selected by
> +.IR "(unsigned int) arg2" ,
> +which can be one of the following:
> +.RS
> +.TP
> +.B 0
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +must be untagged.
> +.TP
> +.B PR_TAGGED_ADDR_ENABLE
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +may be tagged, with the exceptions summarized below.
> +.RE
> +.IP
> +The remaining arguments
> +.IR arg3 ", " arg4 " and " arg5
> +must all be zero.
> +.IP
> +On success, the mode specified in
> +.I arg2
> +is set for the calling thread and the the return value is 0.
> +If the arguments are invalid,
> +the mode specified in
> +.I arg2
> +is unrecognized,
> +or if this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +.IP
> +In particular, if
> +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.B EINVAL
> +then all addresses passed to the kernel must be untagged.
> +.IP
> +Irrespective of which mode is set,
> +addresses passed to certain interfaces
> +must always be untagged:
> +.RS
> +.IP \(em
> +.BR brk (2),
> +.BR mmap (2),
> +.BR shmat (2),
> +and the
> +.I new_address
> +argument of
> +.BR mremap (2).
> +.IP
> +(Prior to Linux 5.6 these accepted tagged addresses,
> +but the behaviour may not be what you expect.
> +Don't rely on it.)
> +.IP \(em
> +\(oqpolymorphic\(cq interfaces
> +that accept pointers to arbitrary types cast to a
> +.I void *
> +or other generic type, specifically
> +.BR prctl (2),
> +.BR ioctl (2),
> +and in general
> +.BR setsockopt (2)
> +(only certain specific
> +.BR setsockopt (2)
> +options allow tagged addresses).
> +.IP \(em
> +.BR shmdt (2).
> +.RE
> +.IP
> +This list of exclusions may shrink
> +when moving from one kernel version to a later kernel version.
> +While the kernel may make some guarantees
> +for backwards compatibility reasons,
> +for the purposes of new software
> +the effect of passing tagged addresses to these interfaces
> +is unspecified.
> +.IP
> +The mode set by this call is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +The mode is reset by
> +.BR execve (2)
> +to 0
> +(i.e., tagged addresses not permitted in the user/kernel ABI).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may make use of address tagging,
> +a successful
> +.B PR_SET_TAGGED_ADDR_CTRL
> +may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.
> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
> +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Returns the current tagged address mode
> +for the calling thread.
> +.IP
> +Arguments
> +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> +must all be zero.
> +.IP
> +If the arguments are invalid
> +or this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +In particular, if
> +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.BR EINVAL ,
> +then this feature is definitely unsupported or disabled,
> +and all addresses passed to the kernel must be untagged.
> +.IP
> +Otherwise, the call returns a nonnegative value
> +describing the current tagged address mode,
> +encoded in the same way as the
> +.I arg2
> +argument of
> +.BR PR_SET_TAGGED_ADDR_CTRL .
> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP
> @@ -1749,6 +1886,7 @@ On success,
>  .BR PR_GET_SPECULATION_CTRL ,
>  .BR PR_SVE_GET_VL ,
>  .BR PR_SVE_SET_VL ,
> +.BR PR_GET_TAGGED_ADDR_CTRL ,
>  .BR PR_GET_THP_DISABLE ,
>  .BR PR_GET_TIMING ,
>  .BR PR_GET_TIMERSLACK ,
> @@ -2057,6 +2195,24 @@ is
>  .B PR_SVE_GET_VL
>  and SVE is not available on this platform.
>  .TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_SET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_SET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_GET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_GET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
>  .B ENODEV
>  .I option
>  was
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Will Deacon]

On Tue, Jun 09, 2020 at 01:04:25PM +0200, Michael Kerrisk (man-pages) wrote:
> Do we have any review comments for this (extensive!) patch from Dave?

(Adding Andrey, since he was involved with this ABI)

Regardless, it would be good to have Catalin's ack and I think he was
planning to take a look at this.

Will

> On 5/27/20 11:17 PM, Dave Martin wrote:
> > ** This patch is a draft for review and should not be applied before it
> >    has been discussed. **
> > 
> > Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
> > PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> > Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> > Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Vincenzo Frascino <vincenzo.frascino-5wv7dgnIgG8@public.gmane.org>
> > ---
> > 
> >  man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 156 insertions(+)
> > 
> > diff --git a/man2/prctl.2 b/man2/prctl.2
> > index 3ee2702..062fd51 100644
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> >  (or
> >  .I Documentation/arm64/sve.txt
> >  before Linux 5.3).
> > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Controls support for passing tagged userspace addresses to the kernel
> > +(i.e., addresses where bits 56\(em63 are not all zero).
> > +.IP
> > +The level of support is selected by
> > +.IR "(unsigned int) arg2" ,
> > +which can be one of the following:
> > +.RS
> > +.TP
> > +.B 0
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +must be untagged.
> > +.TP
> > +.B PR_TAGGED_ADDR_ENABLE
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +may be tagged, with the exceptions summarized below.
> > +.RE
> > +.IP
> > +The remaining arguments
> > +.IR arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +On success, the mode specified in
> > +.I arg2
> > +is set for the calling thread and the the return value is 0.
> > +If the arguments are invalid,
> > +the mode specified in
> > +.I arg2
> > +is unrecognized,
> > +or if this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +.IP
> > +In particular, if
> > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.B EINVAL
> > +then all addresses passed to the kernel must be untagged.
> > +.IP
> > +Irrespective of which mode is set,
> > +addresses passed to certain interfaces
> > +must always be untagged:
> > +.RS
> > +.IP \(em
> > +.BR brk (2),
> > +.BR mmap (2),
> > +.BR shmat (2),
> > +and the
> > +.I new_address
> > +argument of
> > +.BR mremap (2).
> > +.IP
> > +(Prior to Linux 5.6 these accepted tagged addresses,
> > +but the behaviour may not be what you expect.
> > +Don't rely on it.)
> > +.IP \(em
> > +\(oqpolymorphic\(cq interfaces
> > +that accept pointers to arbitrary types cast to a
> > +.I void *
> > +or other generic type, specifically
> > +.BR prctl (2),
> > +.BR ioctl (2),
> > +and in general
> > +.BR setsockopt (2)
> > +(only certain specific
> > +.BR setsockopt (2)
> > +options allow tagged addresses).
> > +.IP \(em
> > +.BR shmdt (2).
> > +.RE
> > +.IP
> > +This list of exclusions may shrink
> > +when moving from one kernel version to a later kernel version.
> > +While the kernel may make some guarantees
> > +for backwards compatibility reasons,
> > +for the purposes of new software
> > +the effect of passing tagged addresses to these interfaces
> > +is unspecified.
> > +.IP
> > +The mode set by this call is inherited across
> > +.BR fork (2)
> > +and
> > +.BR clone (2).
> > +The mode is reset by
> > +.BR execve (2)
> > +to 0
> > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may make use of address tagging,
> > +a successful
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +may crash the calling process.
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you know what you are doing.
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Returns the current tagged address mode
> > +for the calling thread.
> > +.IP
> > +Arguments
> > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +If the arguments are invalid
> > +or this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +In particular, if
> > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.BR EINVAL ,
> > +then this feature is definitely unsupported or disabled,
> > +and all addresses passed to the kernel must be untagged.
> > +.IP
> > +Otherwise, the call returns a nonnegative value
> > +describing the current tagged address mode,
> > +encoded in the same way as the
> > +.I arg2
> > +argument of
> > +.BR PR_SET_TAGGED_ADDR_CTRL .
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> >  .\"
> >  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
> >  .TP
> > @@ -1749,6 +1886,7 @@ On success,
> >  .BR PR_GET_SPECULATION_CTRL ,
> >  .BR PR_SVE_GET_VL ,
> >  .BR PR_SVE_SET_VL ,
> > +.BR PR_GET_TAGGED_ADDR_CTRL ,
> >  .BR PR_GET_THP_DISABLE ,
> >  .BR PR_GET_TIMING ,
> >  .BR PR_GET_TIMERSLACK ,
> > @@ -2057,6 +2195,24 @@ is
> >  .B PR_SVE_GET_VL
> >  and SVE is not available on this platform.
> >  .TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_SET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_GET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_GET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> >  .B ENODEV
> >  .I option
> >  was
> > 
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Will Deacon]

On Tue, Jun 09, 2020 at 01:04:25PM +0200, Michael Kerrisk (man-pages) wrote:
> Do we have any review comments for this (extensive!) patch from Dave?

(Adding Andrey, since he was involved with this ABI)

Regardless, it would be good to have Catalin's ack and I think he was
planning to take a look at this.

Will

> On 5/27/20 11:17 PM, Dave Martin wrote:
> > ** This patch is a draft for review and should not be applied before it
> >    has been discussed. **
> > 
> > Add documentation for the the PR_SET_TAGGED_ADDR_CTRL and
> > PR_GET_TAGGED_ADDR_CTRL prctls added in Linux 5.4 for arm64.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > ---
> > 
> >  man2/prctl.2 | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 156 insertions(+)
> > 
> > diff --git a/man2/prctl.2 b/man2/prctl.2
> > index 3ee2702..062fd51 100644
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> >  (or
> >  .I Documentation/arm64/sve.txt
> >  before Linux 5.3).
> > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Controls support for passing tagged userspace addresses to the kernel
> > +(i.e., addresses where bits 56\(em63 are not all zero).
> > +.IP
> > +The level of support is selected by
> > +.IR "(unsigned int) arg2" ,
> > +which can be one of the following:
> > +.RS
> > +.TP
> > +.B 0
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +must be untagged.
> > +.TP
> > +.B PR_TAGGED_ADDR_ENABLE
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +may be tagged, with the exceptions summarized below.
> > +.RE
> > +.IP
> > +The remaining arguments
> > +.IR arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +On success, the mode specified in
> > +.I arg2
> > +is set for the calling thread and the the return value is 0.
> > +If the arguments are invalid,
> > +the mode specified in
> > +.I arg2
> > +is unrecognized,
> > +or if this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +.IP
> > +In particular, if
> > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.B EINVAL
> > +then all addresses passed to the kernel must be untagged.
> > +.IP
> > +Irrespective of which mode is set,
> > +addresses passed to certain interfaces
> > +must always be untagged:
> > +.RS
> > +.IP \(em
> > +.BR brk (2),
> > +.BR mmap (2),
> > +.BR shmat (2),
> > +and the
> > +.I new_address
> > +argument of
> > +.BR mremap (2).
> > +.IP
> > +(Prior to Linux 5.6 these accepted tagged addresses,
> > +but the behaviour may not be what you expect.
> > +Don't rely on it.)
> > +.IP \(em
> > +\(oqpolymorphic\(cq interfaces
> > +that accept pointers to arbitrary types cast to a
> > +.I void *
> > +or other generic type, specifically
> > +.BR prctl (2),
> > +.BR ioctl (2),
> > +and in general
> > +.BR setsockopt (2)
> > +(only certain specific
> > +.BR setsockopt (2)
> > +options allow tagged addresses).
> > +.IP \(em
> > +.BR shmdt (2).
> > +.RE
> > +.IP
> > +This list of exclusions may shrink
> > +when moving from one kernel version to a later kernel version.
> > +While the kernel may make some guarantees
> > +for backwards compatibility reasons,
> > +for the purposes of new software
> > +the effect of passing tagged addresses to these interfaces
> > +is unspecified.
> > +.IP
> > +The mode set by this call is inherited across
> > +.BR fork (2)
> > +and
> > +.BR clone (2).
> > +The mode is reset by
> > +.BR execve (2)
> > +to 0
> > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may make use of address tagging,
> > +a successful
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +may crash the calling process.
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you know what you are doing.
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Returns the current tagged address mode
> > +for the calling thread.
> > +.IP
> > +Arguments
> > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +If the arguments are invalid
> > +or this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +In particular, if
> > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.BR EINVAL ,
> > +then this feature is definitely unsupported or disabled,
> > +and all addresses passed to the kernel must be untagged.
> > +.IP
> > +Otherwise, the call returns a nonnegative value
> > +describing the current tagged address mode,
> > +encoded in the same way as the
> > +.I arg2
> > +argument of
> > +.BR PR_SET_TAGGED_ADDR_CTRL .
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> >  .\"
> >  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
> >  .TP
> > @@ -1749,6 +1886,7 @@ On success,
> >  .BR PR_GET_SPECULATION_CTRL ,
> >  .BR PR_SVE_GET_VL ,
> >  .BR PR_SVE_SET_VL ,
> > +.BR PR_GET_TAGGED_ADDR_CTRL ,
> >  .BR PR_GET_THP_DISABLE ,
> >  .BR PR_GET_TIMING ,
> >  .BR PR_GET_TIMERSLACK ,
> > @@ -2057,6 +2195,24 @@ is
> >  .B PR_SVE_GET_VL
> >  and SVE is not available on this platform.
> >  .TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_SET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_GET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_GET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> >  .B ENODEV
> >  .I option
> >  was
> > 
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Catalin Marinas]

Hi Dave,

On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
>  (or
>  .I Documentation/arm64/sve.txt
>  before Linux 5.3).
> +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Controls support for passing tagged userspace addresses to the kernel
> +(i.e., addresses where bits 56\(em63 are not all zero).

Nitpick: maybe say "userspace addresses" again inside the brackets since
kernel addresses have all top bits 1.

> +.IP
> +The level of support is selected by
> +.IR "(unsigned int) arg2" ,

We use (unsigned long) for arg2.

> +which can be one of the following:
> +.RS
> +.TP
> +.B 0
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +must be untagged.
> +.TP
> +.B PR_TAGGED_ADDR_ENABLE
> +Addresses that are passed
> +for the purpose of being dereferenced by the kernel
> +may be tagged, with the exceptions summarized below.
> +.RE
> +.IP
> +The remaining arguments
> +.IR arg3 ", " arg4 " and " arg5
> +must all be zero.

Indeed. The above commit didn't have this, we added it later in commit
3e91ec89f527b9870fe42dcbdb74fd389d123a95.

> +.IP
> +On success, the mode specified in
> +.I arg2
> +is set for the calling thread and the the return value is 0.
> +If the arguments are invalid,
> +the mode specified in
> +.I arg2
> +is unrecognized,
> +or if this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +.IP
> +In particular, if
> +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.B EINVAL
> +then all addresses passed to the kernel must be untagged.
> +.IP
> +Irrespective of which mode is set,
> +addresses passed to certain interfaces
> +must always be untagged:

Maybe you could add some extra info from the kernel comment (commit
b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
avoid the creation of aliasing mappings in userspace).

> +.RS
> +.IP \(em
> +.BR brk (2),
> +.BR mmap (2),
> +.BR shmat (2),
> +and the
> +.I new_address
> +argument of
> +.BR mremap (2).
> +.IP
> +(Prior to Linux 5.6 these accepted tagged addresses,
> +but the behaviour may not be what you expect.
> +Don't rely on it.)

shmat() was not part of the subsequent fix
(dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
address. But I guess it doesn't matter much, the user should not pass
tagged addresses to these syscalls anyway.

You could move shmat() down together with shmdt().

> +.IP \(em
> +\(oqpolymorphic\(cq interfaces
> +that accept pointers to arbitrary types cast to a
> +.I void *
> +or other generic type, specifically
> +.BR prctl (2),
> +.BR ioctl (2),
> +and in general
> +.BR setsockopt (2)
> +(only certain specific
> +.BR setsockopt (2)
> +options allow tagged addresses).
> +.IP \(em
> +.BR shmdt (2).
> +.RE
> +.IP
> +This list of exclusions may shrink
> +when moving from one kernel version to a later kernel version.
> +While the kernel may make some guarantees
> +for backwards compatibility reasons,
> +for the purposes of new software
> +the effect of passing tagged addresses to these interfaces
> +is unspecified.
> +.IP
> +The mode set by this call is inherited across
> +.BR fork (2)
> +and
> +.BR clone (2).
> +The mode is reset by
> +.BR execve (2)
> +to 0
> +(i.e., tagged addresses not permitted in the user/kernel ABI).
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may make use of address tagging,
> +a successful
> +.B PR_SET_TAGGED_ADDR_CTRL
> +may crash the calling process.

I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
Rather disabling tagged addresses would break it. If a process is using
tagged addresses but does not pass them to the kernel, it will continue
to do so even when the syscalls accept such addresses.

> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.

This syscall is intended for the C library if the heap allocator
generates tagged addresses. So it's not a general purpose prctl() random
application code could call. Anyway I'm fine with your warning of not
doing it but you may want to clarify the intent.

> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
> +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> +.TP
> +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> +Returns the current tagged address mode
> +for the calling thread.
> +.IP
> +Arguments
> +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> +must all be zero.
> +.IP
> +If the arguments are invalid
> +or this feature is disabled or unsupported by the kernel,
> +the call fails with
> +.BR EINVAL .
> +In particular, if
> +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> +0, 0, 0, 0)
> +fails with
> +.BR EINVAL ,
> +then this feature is definitely unsupported or disabled,

I guess it's outside the scope of the prctl.2 to describe how the
feature was disabled (e.g. sysctl).

> +and all addresses passed to the kernel must be untagged.
> +.IP
> +Otherwise, the call returns a nonnegative value
> +describing the current tagged address mode,
> +encoded in the same way as the
> +.I arg2
> +argument of
> +.BR PR_SET_TAGGED_ADDR_CTRL .
> +.IP
> +For more information, see the kernel source file
> +.IR Documentation/arm64/tagged\-address\-abi.rst .
>  .\"
>  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
>  .TP
> @@ -1749,6 +1886,7 @@ On success,
>  .BR PR_GET_SPECULATION_CTRL ,
>  .BR PR_SVE_GET_VL ,
>  .BR PR_SVE_SET_VL ,
> +.BR PR_GET_TAGGED_ADDR_CTRL ,
>  .BR PR_GET_THP_DISABLE ,
>  .BR PR_GET_TIMING ,
>  .BR PR_GET_TIMERSLACK ,
> @@ -2057,6 +2195,24 @@ is
>  .B PR_SVE_GET_VL
>  and SVE is not available on this platform.
>  .TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_SET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_SET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
> +.B EINVAL
> +.I option
> +is
> +.BR PR_GET_TAGGED_ADDR_CTRL
> +and the arguments are invalid or unsupported.
> +See the description of
> +.B PR_GET_TAGGED_ADDR_CTRL
> +above for details.
> +.TP
>  .B ENODEV
>  .I option
>  was

Apart from some minor nits above, the documentation looks fine.

Thanks for writing this up.

-- 
Catalin

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> Hi Dave,
> 
> On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> >  (or
> >  .I Documentation/arm64/sve.txt
> >  before Linux 5.3).
> > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Controls support for passing tagged userspace addresses to the kernel
> > +(i.e., addresses where bits 56\(em63 are not all zero).
> 
> Nitpick: maybe say "userspace addresses" again inside the brackets since
> kernel addresses have all top bits 1.

Happy to do that.  This is a user-facing interface though: userspace
addresses are the only kind of address there is.

> 
> > +.IP
> > +The level of support is selected by
> > +.IR "(unsigned int) arg2" ,
> 
> We use (unsigned long) for arg2.

Hmmm, not quite sure how I came up with unsigned int here.  I'll just
drop this: the type in the prctl() prototype is unsigned long anyway.

The type is actually moot in this case, since the valid values all fit
in an unsigned int.

> > +which can be one of the following:
> > +.RS
> > +.TP
> > +.B 0
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +must be untagged.
> > +.TP
> > +.B PR_TAGGED_ADDR_ENABLE
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +may be tagged, with the exceptions summarized below.
> > +.RE
> > +.IP
> > +The remaining arguments
> > +.IR arg3 ", " arg4 " and " arg5
> > +must all be zero.
> 
> Indeed. The above commit didn't have this, we added it later in commit
> 3e91ec89f527b9870fe42dcbdb74fd389d123a95.

Ah, missed that.  Did any full kernel release expose the unchecked
behaviour?

Mind you, there's probably no need to document in any case.

> 
> > +.IP
> > +On success, the mode specified in
> > +.I arg2
> > +is set for the calling thread and the the return value is 0.
> > +If the arguments are invalid,
> > +the mode specified in
> > +.I arg2
> > +is unrecognized,
> > +or if this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +.IP
> > +In particular, if
> > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.B EINVAL
> > +then all addresses passed to the kernel must be untagged.
> > +.IP
> > +Irrespective of which mode is set,
> > +addresses passed to certain interfaces
> > +must always be untagged:
> 
> Maybe you could add some extra info from the kernel comment (commit
> b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
> avoid the creation of aliasing mappings in userspace).

It depends.  It's useful if it helps people to guess highly accurately
the rule for in interface that is too new or that we don't explicitly
document (such as a random setsockopt or perf widget).

If not, it might be best to say nothing and make guarantees only about
the explicitly listed interfaces though.

What do you think?

> > +.RS
> > +.IP \(em
> > +.BR brk (2),
> > +.BR mmap (2),
> > +.BR shmat (2),
> > +and the
> > +.I new_address
> > +argument of
> > +.BR mremap (2).
> > +.IP
> > +(Prior to Linux 5.6 these accepted tagged addresses,
> > +but the behaviour may not be what you expect.
> > +Don't rely on it.)
> 
> shmat() was not part of the subsequent fix
> (dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
> address. But I guess it doesn't matter much, the user should not pass
> tagged addresses to these syscalls anyway.
> 
> You could move shmat() down together with shmdt().

I guess I was highlighting that shmdt() is a special case, because the
user would guess from the pattern of the other listed calls that shmdt()
should accept tagged addresses.  If you think separating it just adds to
the confusion, I'm happy not too call it out specially here.

OTOH, we could fix shmdt() and document the legacy behaviour as a bug in
specific kernel versions rather than the canonical behaviour.  But I
guess that's one for later.

> > +.IP \(em
> > +\(oqpolymorphic\(cq interfaces
> > +that accept pointers to arbitrary types cast to a
> > +.I void *
> > +or other generic type, specifically
> > +.BR prctl (2),
> > +.BR ioctl (2),
> > +and in general
> > +.BR setsockopt (2)
> > +(only certain specific
> > +.BR setsockopt (2)
> > +options allow tagged addresses).
> > +.IP \(em
> > +.BR shmdt (2).
> > +.RE
> > +.IP
> > +This list of exclusions may shrink
> > +when moving from one kernel version to a later kernel version.
> > +While the kernel may make some guarantees
> > +for backwards compatibility reasons,
> > +for the purposes of new software
> > +the effect of passing tagged addresses to these interfaces
> > +is unspecified.
> > +.IP
> > +The mode set by this call is inherited across
> > +.BR fork (2)
> > +and
> > +.BR clone (2).
> > +The mode is reset by
> > +.BR execve (2)
> > +to 0
> > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may make use of address tagging,
> > +a successful
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +may crash the calling process.
> 
> I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
> Rather disabling tagged addresses would break it. If a process is using

This is precisely how PR_SET_TAGGED_ADDR_CTRL could crash the calling
process, no?

Rather than try to explain the different cases in detail here and have
the reader take it as gospel, I thought it would be better to scare them
a bit and encourage them to so some homework.  Perhaps I'm being too
cautious.

> tagged addresses but does not pass them to the kernel, it will continue
> to do so even when the syscalls accept such addresses.
> 
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you know what you are doing.
> 
> This syscall is intended for the C library if the heap allocator
> generates tagged addresses. So it's not a general purpose prctl() random
> application code could call. Anyway I'm fine with your warning of not
> doing it but you may want to clarify the intent.

Maybe add something like

"This call is primarily intended for use by the run-time environment."

?

> 
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Returns the current tagged address mode
> > +for the calling thread.
> > +.IP
> > +Arguments
> > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +If the arguments are invalid
> > +or this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +In particular, if
> > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.BR EINVAL ,
> > +then this feature is definitely unsupported or disabled,
> 
> I guess it's outside the scope of the prctl.2 to describe how the
> feature was disabled (e.g. sysctl).

We could include it here, but I'm thinking we might want a more
comprehensive separate page to describe how to use MTE in general.

For now, is referencing the kernel documentation enough?

> > +and all addresses passed to the kernel must be untagged.
> > +.IP
> > +Otherwise, the call returns a nonnegative value
> > +describing the current tagged address mode,
> > +encoded in the same way as the
> > +.I arg2
> > +argument of
> > +.BR PR_SET_TAGGED_ADDR_CTRL .
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> >  .\"
> >  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
> >  .TP
> > @@ -1749,6 +1886,7 @@ On success,
> >  .BR PR_GET_SPECULATION_CTRL ,
> >  .BR PR_SVE_GET_VL ,
> >  .BR PR_SVE_SET_VL ,
> > +.BR PR_GET_TAGGED_ADDR_CTRL ,
> >  .BR PR_GET_THP_DISABLE ,
> >  .BR PR_GET_TIMING ,
> >  .BR PR_GET_TIMERSLACK ,
> > @@ -2057,6 +2195,24 @@ is
> >  .B PR_SVE_GET_VL
> >  and SVE is not available on this platform.
> >  .TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_SET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_GET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_GET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> >  .B ENODEV
> >  .I option
> >  was
> 
> Apart from some minor nits above, the documentation looks fine.
> 
> Thanks for writing this up.

Thanks for the review.  I was taking a punt on some of it, so I'm
reassured that you didn't find too many controversial points.

Cheers
---Dave

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> Hi Dave,
> 
> On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> >  (or
> >  .I Documentation/arm64/sve.txt
> >  before Linux 5.3).
> > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Controls support for passing tagged userspace addresses to the kernel
> > +(i.e., addresses where bits 56\(em63 are not all zero).
> 
> Nitpick: maybe say "userspace addresses" again inside the brackets since
> kernel addresses have all top bits 1.

Happy to do that.  This is a user-facing interface though: userspace
addresses are the only kind of address there is.

> 
> > +.IP
> > +The level of support is selected by
> > +.IR "(unsigned int) arg2" ,
> 
> We use (unsigned long) for arg2.

Hmmm, not quite sure how I came up with unsigned int here.  I'll just
drop this: the type in the prctl() prototype is unsigned long anyway.

The type is actually moot in this case, since the valid values all fit
in an unsigned int.

> > +which can be one of the following:
> > +.RS
> > +.TP
> > +.B 0
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +must be untagged.
> > +.TP
> > +.B PR_TAGGED_ADDR_ENABLE
> > +Addresses that are passed
> > +for the purpose of being dereferenced by the kernel
> > +may be tagged, with the exceptions summarized below.
> > +.RE
> > +.IP
> > +The remaining arguments
> > +.IR arg3 ", " arg4 " and " arg5
> > +must all be zero.
> 
> Indeed. The above commit didn't have this, we added it later in commit
> 3e91ec89f527b9870fe42dcbdb74fd389d123a95.

Ah, missed that.  Did any full kernel release expose the unchecked
behaviour?

Mind you, there's probably no need to document in any case.

> 
> > +.IP
> > +On success, the mode specified in
> > +.I arg2
> > +is set for the calling thread and the the return value is 0.
> > +If the arguments are invalid,
> > +the mode specified in
> > +.I arg2
> > +is unrecognized,
> > +or if this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +.IP
> > +In particular, if
> > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.B EINVAL
> > +then all addresses passed to the kernel must be untagged.
> > +.IP
> > +Irrespective of which mode is set,
> > +addresses passed to certain interfaces
> > +must always be untagged:
> 
> Maybe you could add some extra info from the kernel comment (commit
> b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
> avoid the creation of aliasing mappings in userspace).

It depends.  It's useful if it helps people to guess highly accurately
the rule for in interface that is too new or that we don't explicitly
document (such as a random setsockopt or perf widget).

If not, it might be best to say nothing and make guarantees only about
the explicitly listed interfaces though.

What do you think?

> > +.RS
> > +.IP \(em
> > +.BR brk (2),
> > +.BR mmap (2),
> > +.BR shmat (2),
> > +and the
> > +.I new_address
> > +argument of
> > +.BR mremap (2).
> > +.IP
> > +(Prior to Linux 5.6 these accepted tagged addresses,
> > +but the behaviour may not be what you expect.
> > +Don't rely on it.)
> 
> shmat() was not part of the subsequent fix
> (dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
> address. But I guess it doesn't matter much, the user should not pass
> tagged addresses to these syscalls anyway.
> 
> You could move shmat() down together with shmdt().

I guess I was highlighting that shmdt() is a special case, because the
user would guess from the pattern of the other listed calls that shmdt()
should accept tagged addresses.  If you think separating it just adds to
the confusion, I'm happy not too call it out specially here.

OTOH, we could fix shmdt() and document the legacy behaviour as a bug in
specific kernel versions rather than the canonical behaviour.  But I
guess that's one for later.

> > +.IP \(em
> > +\(oqpolymorphic\(cq interfaces
> > +that accept pointers to arbitrary types cast to a
> > +.I void *
> > +or other generic type, specifically
> > +.BR prctl (2),
> > +.BR ioctl (2),
> > +and in general
> > +.BR setsockopt (2)
> > +(only certain specific
> > +.BR setsockopt (2)
> > +options allow tagged addresses).
> > +.IP \(em
> > +.BR shmdt (2).
> > +.RE
> > +.IP
> > +This list of exclusions may shrink
> > +when moving from one kernel version to a later kernel version.
> > +While the kernel may make some guarantees
> > +for backwards compatibility reasons,
> > +for the purposes of new software
> > +the effect of passing tagged addresses to these interfaces
> > +is unspecified.
> > +.IP
> > +The mode set by this call is inherited across
> > +.BR fork (2)
> > +and
> > +.BR clone (2).
> > +The mode is reset by
> > +.BR execve (2)
> > +to 0
> > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may make use of address tagging,
> > +a successful
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +may crash the calling process.
> 
> I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
> Rather disabling tagged addresses would break it. If a process is using

This is precisely how PR_SET_TAGGED_ADDR_CTRL could crash the calling
process, no?

Rather than try to explain the different cases in detail here and have
the reader take it as gospel, I thought it would be better to scare them
a bit and encourage them to so some homework.  Perhaps I'm being too
cautious.

> tagged addresses but does not pass them to the kernel, it will continue
> to do so even when the syscalls accept such addresses.
> 
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you know what you are doing.
> 
> This syscall is intended for the C library if the heap allocator
> generates tagged addresses. So it's not a general purpose prctl() random
> application code could call. Anyway I'm fine with your warning of not
> doing it but you may want to clarify the intent.

Maybe add something like

"This call is primarily intended for use by the run-time environment."

?

> 
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > +.TP
> > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > +Returns the current tagged address mode
> > +for the calling thread.
> > +.IP
> > +Arguments
> > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > +must all be zero.
> > +.IP
> > +If the arguments are invalid
> > +or this feature is disabled or unsupported by the kernel,
> > +the call fails with
> > +.BR EINVAL .
> > +In particular, if
> > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > +0, 0, 0, 0)
> > +fails with
> > +.BR EINVAL ,
> > +then this feature is definitely unsupported or disabled,
> 
> I guess it's outside the scope of the prctl.2 to describe how the
> feature was disabled (e.g. sysctl).

We could include it here, but I'm thinking we might want a more
comprehensive separate page to describe how to use MTE in general.

For now, is referencing the kernel documentation enough?

> > +and all addresses passed to the kernel must be untagged.
> > +.IP
> > +Otherwise, the call returns a nonnegative value
> > +describing the current tagged address mode,
> > +encoded in the same way as the
> > +.I arg2
> > +argument of
> > +.BR PR_SET_TAGGED_ADDR_CTRL .
> > +.IP
> > +For more information, see the kernel source file
> > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> >  .\"
> >  .\" prctl PR_TASK_PERF_EVENTS_DISABLE
> >  .TP
> > @@ -1749,6 +1886,7 @@ On success,
> >  .BR PR_GET_SPECULATION_CTRL ,
> >  .BR PR_SVE_GET_VL ,
> >  .BR PR_SVE_SET_VL ,
> > +.BR PR_GET_TAGGED_ADDR_CTRL ,
> >  .BR PR_GET_THP_DISABLE ,
> >  .BR PR_GET_TIMING ,
> >  .BR PR_GET_TIMERSLACK ,
> > @@ -2057,6 +2195,24 @@ is
> >  .B PR_SVE_GET_VL
> >  and SVE is not available on this platform.
> >  .TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_SET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_SET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> > +.B EINVAL
> > +.I option
> > +is
> > +.BR PR_GET_TAGGED_ADDR_CTRL
> > +and the arguments are invalid or unsupported.
> > +See the description of
> > +.B PR_GET_TAGGED_ADDR_CTRL
> > +above for details.
> > +.TP
> >  .B ENODEV
> >  .I option
> >  was
> 
> Apart from some minor nits above, the documentation looks fine.
> 
> Thanks for writing this up.

Thanks for the review.  I was taking a punt on some of it, so I'm
reassured that you didn't find too many controversial points.

Cheers
---Dave

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Catalin Marinas]

On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > --- a/man2/prctl.2
> > > +++ b/man2/prctl.2
> > > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> > >  (or
> > >  .I Documentation/arm64/sve.txt
> > >  before Linux 5.3).
> > > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > +.TP
> > > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > +Controls support for passing tagged userspace addresses to the kernel
> > > +(i.e., addresses where bits 56\(em63 are not all zero).
> > 
> > Nitpick: maybe say "userspace addresses" again inside the brackets since
> > kernel addresses have all top bits 1.
> 
> Happy to do that.  This is a user-facing interface though: userspace
> addresses are the only kind of address there is.

Living in the kernel land for too long, it's hard to switch perspective ;).
I think the original sentence makes sense anyway, it should be obvious
that it refers to user addresses.

> > > +.IP
> > > +The level of support is selected by
> > > +.IR "(unsigned int) arg2" ,
> > 
> > We use (unsigned long) for arg2.
> 
> Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> drop this: the type in the prctl() prototype is unsigned long anyway.
> 
> The type is actually moot in this case, since the valid values all fit
> in an unsigned int.

Passing an int doesn't require that the top 32-bit of the long are
zeroed (in case anyone writes the low-level SVC by hand).

> > > +which can be one of the following:
> > > +.RS
> > > +.TP
> > > +.B 0
> > > +Addresses that are passed
> > > +for the purpose of being dereferenced by the kernel
> > > +must be untagged.
> > > +.TP
> > > +.B PR_TAGGED_ADDR_ENABLE
> > > +Addresses that are passed
> > > +for the purpose of being dereferenced by the kernel
> > > +may be tagged, with the exceptions summarized below.
> > > +.RE
> > > +.IP
> > > +The remaining arguments
> > > +.IR arg3 ", " arg4 " and " arg5
> > > +must all be zero.
> > 
> > Indeed. The above commit didn't have this, we added it later in commit
> > 3e91ec89f527b9870fe42dcbdb74fd389d123a95.
> 
> Ah, missed that.  Did any full kernel release expose the unchecked
> behaviour?

No, they both went into 5.4-rc1. I probably didn't want to rebase the
series and just added a patch on top.

> Mind you, there's probably no need to document in any case.

I agree. Just mentioned it because I looked at the commit you mentioned
and there was no check for the arg3..arg5, so went to check the history.

> > > +.IP
> > > +On success, the mode specified in
> > > +.I arg2
> > > +is set for the calling thread and the the return value is 0.
> > > +If the arguments are invalid,
> > > +the mode specified in
> > > +.I arg2
> > > +is unrecognized,
> > > +or if this feature is disabled or unsupported by the kernel,
> > > +the call fails with
> > > +.BR EINVAL .
> > > +.IP
> > > +In particular, if
> > > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > > +0, 0, 0, 0)
> > > +fails with
> > > +.B EINVAL
> > > +then all addresses passed to the kernel must be untagged.
> > > +.IP
> > > +Irrespective of which mode is set,
> > > +addresses passed to certain interfaces
> > > +must always be untagged:
> > 
> > Maybe you could add some extra info from the kernel comment (commit
> > b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
> > avoid the creation of aliasing mappings in userspace).
> 
> It depends.  It's useful if it helps people to guess highly accurately
> the rule for in interface that is too new or that we don't explicitly
> document (such as a random setsockopt or perf widget).
> 
> If not, it might be best to say nothing and make guarantees only about
> the explicitly listed interfaces though.

Fine by me to keep it as it is. We can always update the man page if new
syscalls come into this category.

> > > +.RS
> > > +.IP \(em
> > > +.BR brk (2),
> > > +.BR mmap (2),
> > > +.BR shmat (2),
> > > +and the
> > > +.I new_address
> > > +argument of
> > > +.BR mremap (2).
> > > +.IP
> > > +(Prior to Linux 5.6 these accepted tagged addresses,
> > > +but the behaviour may not be what you expect.
> > > +Don't rely on it.)
> > 
> > shmat() was not part of the subsequent fix
> > (dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
> > address. But I guess it doesn't matter much, the user should not pass
> > tagged addresses to these syscalls anyway.
> > 
> > You could move shmat() down together with shmdt().
> 
> I guess I was highlighting that shmdt() is a special case, because the
> user would guess from the pattern of the other listed calls that shmdt()
> should accept tagged addresses.  If you think separating it just adds to
> the confusion, I'm happy not too call it out specially here.
> 
> OTOH, we could fix shmdt() and document the legacy behaviour as a bug in
> specific kernel versions rather than the canonical behaviour.  But I
> guess that's one for later.

If we patch shmdt() to allow tagged addresses, then it makes sense to
keep shmat() with the rest of the above. Just pointing out that shmat()
never allowed tagged pointers

> > > +.IP \(em
> > > +\(oqpolymorphic\(cq interfaces
> > > +that accept pointers to arbitrary types cast to a
> > > +.I void *
> > > +or other generic type, specifically
> > > +.BR prctl (2),
> > > +.BR ioctl (2),
> > > +and in general
> > > +.BR setsockopt (2)
> > > +(only certain specific
> > > +.BR setsockopt (2)
> > > +options allow tagged addresses).
> > > +.IP \(em
> > > +.BR shmdt (2).
> > > +.RE
> > > +.IP
> > > +This list of exclusions may shrink
> > > +when moving from one kernel version to a later kernel version.
> > > +While the kernel may make some guarantees
> > > +for backwards compatibility reasons,
> > > +for the purposes of new software
> > > +the effect of passing tagged addresses to these interfaces
> > > +is unspecified.
> > > +.IP
> > > +The mode set by this call is inherited across
> > > +.BR fork (2)
> > > +and
> > > +.BR clone (2).
> > > +The mode is reset by
> > > +.BR execve (2)
> > > +to 0
> > > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > > +.IP
> > > +.B Warning:
> > > +Because the compiler or run-time environment
> > > +may make use of address tagging,
> > > +a successful
> > > +.B PR_SET_TAGGED_ADDR_CTRL
> > > +may crash the calling process.
> > 
> > I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
> > Rather disabling tagged addresses would break it. If a process is using
> 
> This is precisely how PR_SET_TAGGED_ADDR_CTRL could crash the calling
> process, no?

I see your point. E.g. it was enabled by glibc but disabled by a user
application.

> Rather than try to explain the different cases in detail here and have
> the reader take it as gospel, I thought it would be better to scare them
> a bit and encourage them to so some homework.  Perhaps I'm being too
> cautious.

That's fine, leave the warning in place.

> > tagged addresses but does not pass them to the kernel, it will continue
> > to do so even when the syscalls accept such addresses.
> > 
> > > +The conditions for using it safely are complex and system-dependent.
> > > +Don't use it unless you know what you are doing.
> > 
> > This syscall is intended for the C library if the heap allocator
> > generates tagged addresses. So it's not a general purpose prctl() random
> > application code could call. Anyway I'm fine with your warning of not
> > doing it but you may want to clarify the intent.
> 
> Maybe add something like
> 
> "This call is primarily intended for use by the run-time environment."

That works as well, maybe in addition to the warning. Up to you.

> > > +.IP
> > > +For more information, see the kernel source file
> > > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > +.TP
> > > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > +Returns the current tagged address mode
> > > +for the calling thread.
> > > +.IP
> > > +Arguments
> > > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > > +must all be zero.
> > > +.IP
> > > +If the arguments are invalid
> > > +or this feature is disabled or unsupported by the kernel,
> > > +the call fails with
> > > +.BR EINVAL .
> > > +In particular, if
> > > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > > +0, 0, 0, 0)
> > > +fails with
> > > +.BR EINVAL ,
> > > +then this feature is definitely unsupported or disabled,
> > 
> > I guess it's outside the scope of the prctl.2 to describe how the
> > feature was disabled (e.g. sysctl).
> 
> We could include it here, but I'm thinking we might want a more
> comprehensive separate page to describe how to use MTE in general.
> 
> For now, is referencing the kernel documentation enough?

I think simply saying "unsupported or disabled" is sufficient in the man
page. The sysctl is not aimed at the user sw/run-time developers but
rather those controlling the system. For the latter, the kernel
documentation is sufficient.

So with the (unsigned int) fixed, feel free to add:

Reviewed-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>

I don't have a strong opinion on the other nitpicks, so I'll leave the
decision to you.

-- 
Catalin

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Catalin Marinas]

On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > --- a/man2/prctl.2
> > > +++ b/man2/prctl.2
> > > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> > >  (or
> > >  .I Documentation/arm64/sve.txt
> > >  before Linux 5.3).
> > > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > +.TP
> > > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > +Controls support for passing tagged userspace addresses to the kernel
> > > +(i.e., addresses where bits 56\(em63 are not all zero).
> > 
> > Nitpick: maybe say "userspace addresses" again inside the brackets since
> > kernel addresses have all top bits 1.
> 
> Happy to do that.  This is a user-facing interface though: userspace
> addresses are the only kind of address there is.

Living in the kernel land for too long, it's hard to switch perspective ;).
I think the original sentence makes sense anyway, it should be obvious
that it refers to user addresses.

> > > +.IP
> > > +The level of support is selected by
> > > +.IR "(unsigned int) arg2" ,
> > 
> > We use (unsigned long) for arg2.
> 
> Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> drop this: the type in the prctl() prototype is unsigned long anyway.
> 
> The type is actually moot in this case, since the valid values all fit
> in an unsigned int.

Passing an int doesn't require that the top 32-bit of the long are
zeroed (in case anyone writes the low-level SVC by hand).

> > > +which can be one of the following:
> > > +.RS
> > > +.TP
> > > +.B 0
> > > +Addresses that are passed
> > > +for the purpose of being dereferenced by the kernel
> > > +must be untagged.
> > > +.TP
> > > +.B PR_TAGGED_ADDR_ENABLE
> > > +Addresses that are passed
> > > +for the purpose of being dereferenced by the kernel
> > > +may be tagged, with the exceptions summarized below.
> > > +.RE
> > > +.IP
> > > +The remaining arguments
> > > +.IR arg3 ", " arg4 " and " arg5
> > > +must all be zero.
> > 
> > Indeed. The above commit didn't have this, we added it later in commit
> > 3e91ec89f527b9870fe42dcbdb74fd389d123a95.
> 
> Ah, missed that.  Did any full kernel release expose the unchecked
> behaviour?

No, they both went into 5.4-rc1. I probably didn't want to rebase the
series and just added a patch on top.

> Mind you, there's probably no need to document in any case.

I agree. Just mentioned it because I looked at the commit you mentioned
and there was no check for the arg3..arg5, so went to check the history.

> > > +.IP
> > > +On success, the mode specified in
> > > +.I arg2
> > > +is set for the calling thread and the the return value is 0.
> > > +If the arguments are invalid,
> > > +the mode specified in
> > > +.I arg2
> > > +is unrecognized,
> > > +or if this feature is disabled or unsupported by the kernel,
> > > +the call fails with
> > > +.BR EINVAL .
> > > +.IP
> > > +In particular, if
> > > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > > +0, 0, 0, 0)
> > > +fails with
> > > +.B EINVAL
> > > +then all addresses passed to the kernel must be untagged.
> > > +.IP
> > > +Irrespective of which mode is set,
> > > +addresses passed to certain interfaces
> > > +must always be untagged:
> > 
> > Maybe you could add some extra info from the kernel comment (commit
> > b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
> > avoid the creation of aliasing mappings in userspace).
> 
> It depends.  It's useful if it helps people to guess highly accurately
> the rule for in interface that is too new or that we don't explicitly
> document (such as a random setsockopt or perf widget).
> 
> If not, it might be best to say nothing and make guarantees only about
> the explicitly listed interfaces though.

Fine by me to keep it as it is. We can always update the man page if new
syscalls come into this category.

> > > +.RS
> > > +.IP \(em
> > > +.BR brk (2),
> > > +.BR mmap (2),
> > > +.BR shmat (2),
> > > +and the
> > > +.I new_address
> > > +argument of
> > > +.BR mremap (2).
> > > +.IP
> > > +(Prior to Linux 5.6 these accepted tagged addresses,
> > > +but the behaviour may not be what you expect.
> > > +Don't rely on it.)
> > 
> > shmat() was not part of the subsequent fix
> > (dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
> > address. But I guess it doesn't matter much, the user should not pass
> > tagged addresses to these syscalls anyway.
> > 
> > You could move shmat() down together with shmdt().
> 
> I guess I was highlighting that shmdt() is a special case, because the
> user would guess from the pattern of the other listed calls that shmdt()
> should accept tagged addresses.  If you think separating it just adds to
> the confusion, I'm happy not too call it out specially here.
> 
> OTOH, we could fix shmdt() and document the legacy behaviour as a bug in
> specific kernel versions rather than the canonical behaviour.  But I
> guess that's one for later.

If we patch shmdt() to allow tagged addresses, then it makes sense to
keep shmat() with the rest of the above. Just pointing out that shmat()
never allowed tagged pointers

> > > +.IP \(em
> > > +\(oqpolymorphic\(cq interfaces
> > > +that accept pointers to arbitrary types cast to a
> > > +.I void *
> > > +or other generic type, specifically
> > > +.BR prctl (2),
> > > +.BR ioctl (2),
> > > +and in general
> > > +.BR setsockopt (2)
> > > +(only certain specific
> > > +.BR setsockopt (2)
> > > +options allow tagged addresses).
> > > +.IP \(em
> > > +.BR shmdt (2).
> > > +.RE
> > > +.IP
> > > +This list of exclusions may shrink
> > > +when moving from one kernel version to a later kernel version.
> > > +While the kernel may make some guarantees
> > > +for backwards compatibility reasons,
> > > +for the purposes of new software
> > > +the effect of passing tagged addresses to these interfaces
> > > +is unspecified.
> > > +.IP
> > > +The mode set by this call is inherited across
> > > +.BR fork (2)
> > > +and
> > > +.BR clone (2).
> > > +The mode is reset by
> > > +.BR execve (2)
> > > +to 0
> > > +(i.e., tagged addresses not permitted in the user/kernel ABI).
> > > +.IP
> > > +.B Warning:
> > > +Because the compiler or run-time environment
> > > +may make use of address tagging,
> > > +a successful
> > > +.B PR_SET_TAGGED_ADDR_CTRL
> > > +may crash the calling process.
> > 
> > I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
> > Rather disabling tagged addresses would break it. If a process is using
> 
> This is precisely how PR_SET_TAGGED_ADDR_CTRL could crash the calling
> process, no?

I see your point. E.g. it was enabled by glibc but disabled by a user
application.

> Rather than try to explain the different cases in detail here and have
> the reader take it as gospel, I thought it would be better to scare them
> a bit and encourage them to so some homework.  Perhaps I'm being too
> cautious.

That's fine, leave the warning in place.

> > tagged addresses but does not pass them to the kernel, it will continue
> > to do so even when the syscalls accept such addresses.
> > 
> > > +The conditions for using it safely are complex and system-dependent.
> > > +Don't use it unless you know what you are doing.
> > 
> > This syscall is intended for the C library if the heap allocator
> > generates tagged addresses. So it's not a general purpose prctl() random
> > application code could call. Anyway I'm fine with your warning of not
> > doing it but you may want to clarify the intent.
> 
> Maybe add something like
> 
> "This call is primarily intended for use by the run-time environment."

That works as well, maybe in addition to the warning. Up to you.

> > > +.IP
> > > +For more information, see the kernel source file
> > > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > +.TP
> > > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > +Returns the current tagged address mode
> > > +for the calling thread.
> > > +.IP
> > > +Arguments
> > > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > > +must all be zero.
> > > +.IP
> > > +If the arguments are invalid
> > > +or this feature is disabled or unsupported by the kernel,
> > > +the call fails with
> > > +.BR EINVAL .
> > > +In particular, if
> > > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > > +0, 0, 0, 0)
> > > +fails with
> > > +.BR EINVAL ,
> > > +then this feature is definitely unsupported or disabled,
> > 
> > I guess it's outside the scope of the prctl.2 to describe how the
> > feature was disabled (e.g. sysctl).
> 
> We could include it here, but I'm thinking we might want a more
> comprehensive separate page to describe how to use MTE in general.
> 
> For now, is referencing the kernel documentation enough?

I think simply saying "unsupported or disabled" is sufficient in the man
page. The sysctl is not aimed at the user sw/run-time developers but
rather those controlling the system. For the latter, the kernel
documentation is sufficient.

So with the (unsigned int) fixed, feel free to add:

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

I don't have a strong opinion on the other nitpicks, so I'll leave the
decision to you.

-- 
Catalin

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > --- a/man2/prctl.2
> > > > +++ b/man2/prctl.2
> > > > @@ -1504,6 +1504,143 @@ For more information, see the kernel source file
> > > >  (or
> > > >  .I Documentation/arm64/sve.txt
> > > >  before Linux 5.3).
> > > > +.\" prctl PR_SET_TAGGED_ADDR_CTRL
> > > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > > +.TP
> > > > +.BR PR_SET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > > +Controls support for passing tagged userspace addresses to the kernel
> > > > +(i.e., addresses where bits 56\(em63 are not all zero).
> > > 
> > > Nitpick: maybe say "userspace addresses" again inside the brackets since
> > > kernel addresses have all top bits 1.
> > 
> > Happy to do that.  This is a user-facing interface though: userspace
> > addresses are the only kind of address there is.
> 
> Living in the kernel land for too long, it's hard to switch perspective ;).
> I think the original sentence makes sense anyway, it should be obvious
> that it refers to user addresses.

OK

> > > > +.IP
> > > > +The level of support is selected by
> > > > +.IR "(unsigned int) arg2" ,
> > > 
> > > We use (unsigned long) for arg2.
> > 
> > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > drop this: the type in the prctl() prototype is unsigned long anyway.
> > 
> > The type is actually moot in this case, since the valid values all fit
> > in an unsigned int.
> 
> Passing an int doesn't require that the top 32-bit of the long are
> zeroed (in case anyone writes the low-level SVC by hand).

Fair point, I was forgetting that wrinkle.  Anyway, the convention in
this page seems to be that if the type is unsigned long, we don't
mention it, because the prctl() prototype says that already.

Question: the glibc prototype for prctl is variadic, so surely any
calls that don't explicitly cast the args to unsigned long are already
theoretically broken?  The #defines (and 0) are all implicitly int.
This probably affects lots of prctls.

We may get away with it because the compiler is almost certainly going
to favour a mov over a ldr for getting small integers into regs, and mov
<Wd> fortunately zeroes the top bits for us anyway.

Seems icky though.

> > > > +which can be one of the following:
> > > > +.RS
> > > > +.TP
> > > > +.B 0
> > > > +Addresses that are passed
> > > > +for the purpose of being dereferenced by the kernel
> > > > +must be untagged.
> > > > +.TP
> > > > +.B PR_TAGGED_ADDR_ENABLE
> > > > +Addresses that are passed
> > > > +for the purpose of being dereferenced by the kernel
> > > > +may be tagged, with the exceptions summarized below.
> > > > +.RE
> > > > +.IP
> > > > +The remaining arguments
> > > > +.IR arg3 ", " arg4 " and " arg5
> > > > +must all be zero.
> > > 
> > > Indeed. The above commit didn't have this, we added it later in commit
> > > 3e91ec89f527b9870fe42dcbdb74fd389d123a95.
> > 
> > Ah, missed that.  Did any full kernel release expose the unchecked
> > behaviour?
> 
> No, they both went into 5.4-rc1. I probably didn't want to rebase the
> series and just added a patch on top.
> 
> > Mind you, there's probably no need to document in any case.
> 
> I agree. Just mentioned it because I looked at the commit you mentioned
> and there was no check for the arg3..arg5, so went to check the history.

Ah, OK.  We should keep quiet then, but I can add the commit reference
as an internal comment in case anyone wants to check where the arg
zeroing enforcement came from.

> 
> > > > +.IP
> > > > +On success, the mode specified in
> > > > +.I arg2
> > > > +is set for the calling thread and the the return value is 0.
> > > > +If the arguments are invalid,
> > > > +the mode specified in
> > > > +.I arg2
> > > > +is unrecognized,
> > > > +or if this feature is disabled or unsupported by the kernel,
> > > > +the call fails with
> > > > +.BR EINVAL .
> > > > +.IP
> > > > +In particular, if
> > > > +.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
> > > > +0, 0, 0, 0)
> > > > +fails with
> > > > +.B EINVAL
> > > > +then all addresses passed to the kernel must be untagged.
> > > > +.IP
> > > > +Irrespective of which mode is set,
> > > > +addresses passed to certain interfaces
> > > > +must always be untagged:
> > > 
> > > Maybe you could add some extra info from the kernel comment (commit
> > > b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
> > > avoid the creation of aliasing mappings in userspace).
> > 
> > It depends.  It's useful if it helps people to guess highly accurately
> > the rule for in interface that is too new or that we don't explicitly
> > document (such as a random setsockopt or perf widget).
> > 
> > If not, it might be best to say nothing and make guarantees only about
> > the explicitly listed interfaces though.
> 
> Fine by me to keep it as it is. We can always update the man page if new
> syscalls come into this category.

OK, I'll probably keep it as-is for now, but shout if you change your mind!

> > > > +.RS
> > > > +.IP \(em
> > > > +.BR brk (2),
> > > > +.BR mmap (2),
> > > > +.BR shmat (2),
> > > > +and the
> > > > +.I new_address
> > > > +argument of
> > > > +.BR mremap (2).
> > > > +.IP
> > > > +(Prior to Linux 5.6 these accepted tagged addresses,
> > > > +but the behaviour may not be what you expect.
> > > > +Don't rely on it.)
> > > 
> > > shmat() was not part of the subsequent fix
> > > (dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
> > > address. But I guess it doesn't matter much, the user should not pass
> > > tagged addresses to these syscalls anyway.
> > > 
> > > You could move shmat() down together with shmdt().
> > 
> > I guess I was highlighting that shmdt() is a special case, because the
> > user would guess from the pattern of the other listed calls that shmdt()
> > should accept tagged addresses.  If you think separating it just adds to
> > the confusion, I'm happy not too call it out specially here.
> > 
> > OTOH, we could fix shmdt() and document the legacy behaviour as a bug in
> > specific kernel versions rather than the canonical behaviour.  But I
> > guess that's one for later.
> 
> If we patch shmdt() to allow tagged addresses, then it makes sense to
> keep shmat() with the rest of the above. Just pointing out that shmat()
> never allowed tagged pointers

Oh, I see what you mean.  I'll have a think when I revisit this.

[...]

> > > > +.B Warning:
> > > > +Because the compiler or run-time environment
> > > > +may make use of address tagging,
> > > > +a successful
> > > > +.B PR_SET_TAGGED_ADDR_CTRL
> > > > +may crash the calling process.
> > > 
> > > I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
> > > Rather disabling tagged addresses would break it. If a process is using
> > 
> > This is precisely how PR_SET_TAGGED_ADDR_CTRL could crash the calling
> > process, no?
> 
> I see your point. E.g. it was enabled by glibc but disabled by a user
> application.
> 
> > Rather than try to explain the different cases in detail here and have
> > the reader take it as gospel, I thought it would be better to scare them
> > a bit and encourage them to so some homework.  Perhaps I'm being too
> > cautious.
> 
> That's fine, leave the warning in place.
> 
> > > tagged addresses but does not pass them to the kernel, it will continue
> > > to do so even when the syscalls accept such addresses.
> > > 
> > > > +The conditions for using it safely are complex and system-dependent.
> > > > +Don't use it unless you know what you are doing.
> > > 
> > > This syscall is intended for the C library if the heap allocator
> > > generates tagged addresses. So it's not a general purpose prctl() random
> > > application code could call. Anyway I'm fine with your warning of not
> > > doing it but you may want to clarify the intent.
> > 
> > Maybe add something like
> > 
> > "This call is primarily intended for use by the run-time environment."
> 
> That works as well, maybe in addition to the warning. Up to you.

OK, I may try to add that, but I'll try not to go overboard!

> > > > +.IP
> > > > +For more information, see the kernel source file
> > > > +.IR Documentation/arm64/tagged\-address\-abi.rst .
> > > > +.\" prctl PR_GET_TAGGED_ADDR_CTRL
> > > > +.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
> > > > +.TP
> > > > +.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
> > > > +Returns the current tagged address mode
> > > > +for the calling thread.
> > > > +.IP
> > > > +Arguments
> > > > +.IR arg2 ", " arg3 ", " arg4 " and " arg5
> > > > +must all be zero.
> > > > +.IP
> > > > +If the arguments are invalid
> > > > +or this feature is disabled or unsupported by the kernel,
> > > > +the call fails with
> > > > +.BR EINVAL .
> > > > +In particular, if
> > > > +.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
> > > > +0, 0, 0, 0)
> > > > +fails with
> > > > +.BR EINVAL ,
> > > > +then this feature is definitely unsupported or disabled,
> > > 
> > > I guess it's outside the scope of the prctl.2 to describe how the
> > > feature was disabled (e.g. sysctl).
> > 
> > We could include it here, but I'm thinking we might want a more
> > comprehensive separate page to describe how to use MTE in general.
> > 
> > For now, is referencing the kernel documentation enough?
> 
> I think simply saying "unsupported or disabled" is sufficient in the man
> page. The sysctl is not aimed at the user sw/run-time developers but
> rather those controlling the system. For the latter, the kernel
> documentation is sufficient.
> 
> So with the (unsigned int) fixed, feel free to add:
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> I don't have a strong opinion on the other nitpicks, so I'll leave the
> decision to you.

OK, thanks

---Dave

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Catalin Marinas]

On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > +.IP
> > > > > +The level of support is selected by
> > > > > +.IR "(unsigned int) arg2" ,
> > > > 
> > > > We use (unsigned long) for arg2.
> > > 
> > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > 
> > > The type is actually moot in this case, since the valid values all fit
> > > in an unsigned int.
> > 
> > Passing an int doesn't require that the top 32-bit of the long are
> > zeroed (in case anyone writes the low-level SVC by hand).
> 
> Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> this page seems to be that if the type is unsigned long, we don't
> mention it, because the prctl() prototype says that already.
> 
> Question: the glibc prototype for prctl is variadic, so surely any
> calls that don't explicitly cast the args to unsigned long are already
> theoretically broken?  The #defines (and 0) are all implicitly int.
> This probably affects lots of prctls.
> 
> We may get away with it because the compiler is almost certainly going
> to favour a mov over a ldr for getting small integers into regs, and mov
> <Wd> fortunately zeroes the top bits for us anyway.

So does LDR Wd.

Anyway, I think glibc (or my reading of it) has something like like:

  register long _x1 asm ("x1") = _x1tmp;

before invoking the SVC. I assume this would do the right conversion to
long. I can't tell about other libraries but I'd say it's their
responsibility to convert the args to long before calling the kernel's
prctl().

-- 
Catalin

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Catalin Marinas]

On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > +.IP
> > > > > +The level of support is selected by
> > > > > +.IR "(unsigned int) arg2" ,
> > > > 
> > > > We use (unsigned long) for arg2.
> > > 
> > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > 
> > > The type is actually moot in this case, since the valid values all fit
> > > in an unsigned int.
> > 
> > Passing an int doesn't require that the top 32-bit of the long are
> > zeroed (in case anyone writes the low-level SVC by hand).
> 
> Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> this page seems to be that if the type is unsigned long, we don't
> mention it, because the prctl() prototype says that already.
> 
> Question: the glibc prototype for prctl is variadic, so surely any
> calls that don't explicitly cast the args to unsigned long are already
> theoretically broken?  The #defines (and 0) are all implicitly int.
> This probably affects lots of prctls.
> 
> We may get away with it because the compiler is almost certainly going
> to favour a mov over a ldr for getting small integers into regs, and mov
> <Wd> fortunately zeroes the top bits for us anyway.

So does LDR Wd.

Anyway, I think glibc (or my reading of it) has something like like:

  register long _x1 asm ("x1") = _x1tmp;

before invoking the SVC. I assume this would do the right conversion to
long. I can't tell about other libraries but I'd say it's their
responsibility to convert the args to long before calling the kernel's
prctl().

-- 
Catalin

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Wed, Jun 10, 2020 at 06:42:05PM +0100, Catalin Marinas wrote:
> On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> > On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > > +.IP
> > > > > > +The level of support is selected by
> > > > > > +.IR "(unsigned int) arg2" ,
> > > > > 
> > > > > We use (unsigned long) for arg2.
> > > > 
> > > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > > 
> > > > The type is actually moot in this case, since the valid values all fit
> > > > in an unsigned int.
> > > 
> > > Passing an int doesn't require that the top 32-bit of the long are
> > > zeroed (in case anyone writes the low-level SVC by hand).
> > 
> > Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> > this page seems to be that if the type is unsigned long, we don't
> > mention it, because the prctl() prototype says that already.
> > 
> > Question: the glibc prototype for prctl is variadic, so surely any
> > calls that don't explicitly cast the args to unsigned long are already
> > theoretically broken?  The #defines (and 0) are all implicitly int.
> > This probably affects lots of prctls.
> > 
> > We may get away with it because the compiler is almost certainly going
> > to favour a mov over a ldr for getting small integers into regs, and mov
> > <Wd> fortunately zeroes the top bits for us anyway.
> 
> So does LDR Wd.
> 
> Anyway, I think glibc (or my reading of it) has something like like:
> 
>   register long _x1 asm ("x1") = _x1tmp;
> 
> before invoking the SVC. I assume this would do the right conversion to
> long. I can't tell about other libraries but I'd say it's their
> responsibility to convert the args to long before calling the kernel's
> prctl().

Ignore me.  I was worrying that glibc would propagate junk in the high
bits of int arguments, due to treating them as longs.  Actually, it
will, but it doesn't matter where we explicitly cast the argument to int
inside the kernel (thanks as usual to -fno-strict-overflow).

Cheers
---Dave

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Wed, Jun 10, 2020 at 06:42:05PM +0100, Catalin Marinas wrote:
> On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> > On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > > +.IP
> > > > > > +The level of support is selected by
> > > > > > +.IR "(unsigned int) arg2" ,
> > > > > 
> > > > > We use (unsigned long) for arg2.
> > > > 
> > > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > > 
> > > > The type is actually moot in this case, since the valid values all fit
> > > > in an unsigned int.
> > > 
> > > Passing an int doesn't require that the top 32-bit of the long are
> > > zeroed (in case anyone writes the low-level SVC by hand).
> > 
> > Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> > this page seems to be that if the type is unsigned long, we don't
> > mention it, because the prctl() prototype says that already.
> > 
> > Question: the glibc prototype for prctl is variadic, so surely any
> > calls that don't explicitly cast the args to unsigned long are already
> > theoretically broken?  The #defines (and 0) are all implicitly int.
> > This probably affects lots of prctls.
> > 
> > We may get away with it because the compiler is almost certainly going
> > to favour a mov over a ldr for getting small integers into regs, and mov
> > <Wd> fortunately zeroes the top bits for us anyway.
> 
> So does LDR Wd.
> 
> Anyway, I think glibc (or my reading of it) has something like like:
> 
>   register long _x1 asm ("x1") = _x1tmp;
> 
> before invoking the SVC. I assume this would do the right conversion to
> long. I can't tell about other libraries but I'd say it's their
> responsibility to convert the args to long before calling the kernel's
> prctl().

Ignore me.  I was worrying that glibc would propagate junk in the high
bits of int arguments, due to treating them as longs.  Actually, it
will, but it doesn't matter where we explicitly cast the argument to int
inside the kernel (thanks as usual to -fno-strict-overflow).

Cheers
---Dave

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Michael Kerrisk (man-pages)]

Hi Dave

Is there a plan for future work on this patch?

Thanks,

Michael

On Mon, 15 Jun 2020 at 16:51, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
>
> On Wed, Jun 10, 2020 at 06:42:05PM +0100, Catalin Marinas wrote:
> > On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> > > On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > > > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > > > +.IP
> > > > > > > +The level of support is selected by
> > > > > > > +.IR "(unsigned int) arg2" ,
> > > > > >
> > > > > > We use (unsigned long) for arg2.
> > > > >
> > > > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > > >
> > > > > The type is actually moot in this case, since the valid values all fit
> > > > > in an unsigned int.
> > > >
> > > > Passing an int doesn't require that the top 32-bit of the long are
> > > > zeroed (in case anyone writes the low-level SVC by hand).
> > >
> > > Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> > > this page seems to be that if the type is unsigned long, we don't
> > > mention it, because the prctl() prototype says that already.
> > >
> > > Question: the glibc prototype for prctl is variadic, so surely any
> > > calls that don't explicitly cast the args to unsigned long are already
> > > theoretically broken?  The #defines (and 0) are all implicitly int.
> > > This probably affects lots of prctls.
> > >
> > > We may get away with it because the compiler is almost certainly going
> > > to favour a mov over a ldr for getting small integers into regs, and mov
> > > <Wd> fortunately zeroes the top bits for us anyway.
> >
> > So does LDR Wd.
> >
> > Anyway, I think glibc (or my reading of it) has something like like:
> >
> >   register long _x1 asm ("x1") = _x1tmp;
> >
> > before invoking the SVC. I assume this would do the right conversion to
> > long. I can't tell about other libraries but I'd say it's their
> > responsibility to convert the args to long before calling the kernel's
> > prctl().
>
> Ignore me.  I was worrying that glibc would propagate junk in the high
> bits of int arguments, due to treating them as longs.  Actually, it
> will, but it doesn't matter where we explicitly cast the argument to int
> inside the kernel (thanks as usual to -fno-strict-overflow).
>
> Cheers
> ---Dave



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Michael Kerrisk (man-pages)]

Hi Dave

Is there a plan for future work on this patch?

Thanks,

Michael

On Mon, 15 Jun 2020 at 16:51, Dave Martin <Dave.Martin@arm.com> wrote:
>
> On Wed, Jun 10, 2020 at 06:42:05PM +0100, Catalin Marinas wrote:
> > On Wed, Jun 10, 2020 at 05:42:09PM +0100, Dave P Martin wrote:
> > > On Wed, Jun 10, 2020 at 04:26:34PM +0100, Catalin Marinas wrote:
> > > > On Wed, Jun 10, 2020 at 11:06:42AM +0100, Dave P Martin wrote:
> > > > > On Tue, Jun 09, 2020 at 06:22:32PM +0100, Catalin Marinas wrote:
> > > > > > On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:
> > > > > > > +.IP
> > > > > > > +The level of support is selected by
> > > > > > > +.IR "(unsigned int) arg2" ,
> > > > > >
> > > > > > We use (unsigned long) for arg2.
> > > > >
> > > > > Hmmm, not quite sure how I came up with unsigned int here.  I'll just
> > > > > drop this: the type in the prctl() prototype is unsigned long anyway.
> > > > >
> > > > > The type is actually moot in this case, since the valid values all fit
> > > > > in an unsigned int.
> > > >
> > > > Passing an int doesn't require that the top 32-bit of the long are
> > > > zeroed (in case anyone writes the low-level SVC by hand).
> > >
> > > Fair point, I was forgetting that wrinkle.  Anyway, the convention in
> > > this page seems to be that if the type is unsigned long, we don't
> > > mention it, because the prctl() prototype says that already.
> > >
> > > Question: the glibc prototype for prctl is variadic, so surely any
> > > calls that don't explicitly cast the args to unsigned long are already
> > > theoretically broken?  The #defines (and 0) are all implicitly int.
> > > This probably affects lots of prctls.
> > >
> > > We may get away with it because the compiler is almost certainly going
> > > to favour a mov over a ldr for getting small integers into regs, and mov
> > > <Wd> fortunately zeroes the top bits for us anyway.
> >
> > So does LDR Wd.
> >
> > Anyway, I think glibc (or my reading of it) has something like like:
> >
> >   register long _x1 asm ("x1") = _x1tmp;
> >
> > before invoking the SVC. I assume this would do the right conversion to
> > long. I can't tell about other libraries but I'd say it's their
> > responsibility to convert the args to long before calling the kernel's
> > prctl().
>
> Ignore me.  I was worrying that glibc would propagate junk in the high
> bits of int arguments, due to treating them as longs.  Actually, it
> will, but it doesn't matter where we explicitly cast the argument to int
> inside the kernel (thanks as usual to -fno-strict-overflow).
>
> Cheers
> ---Dave



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Wed, Jun 24, 2020 at 11:54:45AM +0200, Michael Kerrisk (man-pages) wrote:
> Hi Dave
> 
> Is there a plan for future work on this patch?

I think we have agreement on the content.  Mainly this is waiting for me
to repost.

Cheers
---Dave

[...]

Re: [RFC PATCH v2 6/6] prctl.2: Add tagged address ABI control prctls (arm64)

[Dave Martin]

On Wed, Jun 24, 2020 at 11:54:45AM +0200, Michael Kerrisk (man-pages) wrote:
> Hi Dave
> 
> Is there a plan for future work on this patch?

I think we have agreement on the content.  Mainly this is waiting for me
to repost.

Cheers
---Dave

[...]

[PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Dave Martin]

Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
5.0 for arm64.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>

---

Since v1:

 * Clarify explicitly that PR_PAC_RESET_KEYS is redundant when combined
   with execve().

 * Move error condition details into the prctl description, to avoid
   excessive duplication while keeping keeping related pieces of text
   closer together.

 * In lieu of having a separate man page to cross reference for detailed
   guidance, cross-reference the kernel documentation.

 * Add safety warning.  This is deliberately vague, pending ongoing
   discussions with libc folks.
---
 man2/prctl.2 | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/man2/prctl.2 b/man2/prctl.2
index 91df7c8..3ee2702 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -950,6 +950,77 @@ behavior.
 A value of 1 indicates
 .BR execve (2)
 will operate in the privilege-restricting mode described above.
+.\" prctl PR_PAC_RESET_KEYS
+.\" commit ba830885656414101b2f8ca88786524d4bb5e8c1
+.TP
+.BR PR_PAC_RESET_KEYS " (since Linux 5.0, only on arm64)"
+Securely reset the thread's pointer authentication keys
+to fresh random values generated by the kernel.
+.IP
+The set of keys to be reset is specified by
+.IR arg2 ,
+which must be a logical OR of zero or more of the following:
+.RS
+.TP
+.B PR_PAC_APIAKEY
+instruction authentication key A
+.TP
+.B PR_PAC_APIBKEY
+instruction authentication key B
+.TP
+.B PR_PAC_APDAKEY
+data authentication key A
+.TP
+.B PR_PAC_APDBKEY
+data authentication key B
+.TP
+.B PR_PAC_APGAKEY
+generic authentication \(lqA\(rq key.
+.IP
+(Yes folks, there really is no generic B key.)
+.RE
+.IP
+As a special case, if
+.I arg2
+is zero then all the keys are reset.
+Since new keys could be added in future,
+this is the recommended way to completely wipe the existing keys
+when establishing a clean execution context.
+Note that there is no need to use
+.BR PR_PAC_RESET_KEYS
+in preparation for calling
+.BR execve (2),
+since
+.BR execve (2)
+resets all the pointer authentication keys.
+.IP
+The remaining arguments
+.IR arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+If the arguments are invalid,
+and in particular if
+.I arg2
+contains set bits that are unrecognized
+or that correspond to a key not available on this platform,
+the call fails with error
+.BR EINVAL .
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may be using some or all of the keys,
+a successful
+.IP
+For more information, see the kernel source file
+.I Documentation/arm64/pointer\-authentication.rst
+.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
+(or
+.I Documentation/arm64/pointer\-authentication.txt
+before Linux 5.3).
+.B PR_PAC_RESET_KEYS
+may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.
 .\" prctl PR_SET_PDEATHSIG
 .TP
 .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)"
@@ -1964,6 +2035,15 @@ are not 0.
 .B EINVAL
 .I option
 is
+.B PR_PAC_RESET_KEYS
+and the arguments are invalid or unsupported.
+See the description of
+.B PR_PAC_RESET_KEYS
+above for details.
+.TP
+.B EINVAL
+.I option
+is
 .B PR_SVE_SET_VL
 and the arguments are invalid or unsupported,
 or SVE is not available on this platform.
-- 
2.1.4

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Will Deacon]

On Wed, May 27, 2020 at 10:17:37PM +0100, Dave Martin wrote:
> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
> 5.0 for arm64.

[...]

> +If the arguments are invalid,
> +and in particular if
> +.I arg2
> +contains set bits that are unrecognized
> +or that correspond to a key not available on this platform,
> +the call fails with error
> +.BR EINVAL .
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using some or all of the keys,
> +a successful
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/pointer\-authentication.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/pointer\-authentication.txt
> +before Linux 5.3).
> +.B PR_PAC_RESET_KEYS
> +may crash the calling process.

I might be misreading this, but this looks like the kernel reference appears
mid-sentence. Regardless, I think we should drop the kernel doc reference,
as I mentioned on the SVE patches.

With that:

Acked-by: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Will

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Will Deacon]

On Wed, May 27, 2020 at 10:17:37PM +0100, Dave Martin wrote:
> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
> 5.0 for arm64.

[...]

> +If the arguments are invalid,
> +and in particular if
> +.I arg2
> +contains set bits that are unrecognized
> +or that correspond to a key not available on this platform,
> +the call fails with error
> +.BR EINVAL .
> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using some or all of the keys,
> +a successful
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/pointer\-authentication.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/pointer\-authentication.txt
> +before Linux 5.3).
> +.B PR_PAC_RESET_KEYS
> +may crash the calling process.

I might be misreading this, but this looks like the kernel reference appears
mid-sentence. Regardless, I think we should drop the kernel doc reference,
as I mentioned on the SVE patches.

With that:

Acked-by: Will Deacon <will@kernel.org>

Will

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Michael Kerrisk (man-pages)]

Hi Will,

Thanks for reviewing.

On 6/9/20 12:02 PM, Will Deacon wrote:
> On Wed, May 27, 2020 at 10:17:37PM +0100, Dave Martin wrote:
>> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
>> 5.0 for arm64.
> 
> [...]
> 
>> +If the arguments are invalid,
>> +and in particular if
>> +.I arg2
>> +contains set bits that are unrecognized
>> +or that correspond to a key not available on this platform,
>> +the call fails with error
>> +.BR EINVAL .
>> +.IP
>> +.B Warning:
>> +Because the compiler or run-time environment
>> +may be using some or all of the keys,
>> +a successful
>> +.IP
>> +For more information, see the kernel source file
>> +.I Documentation/arm64/pointer\-authentication.rst
>> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
>> +(or
>> +.I Documentation/arm64/pointer\-authentication.txt
>> +before Linux 5.3).
>> +.B PR_PAC_RESET_KEYS
>> +may crash the calling process.
> 
> I might be misreading this, but this looks like the kernel reference appears
> mid-sentence. 

It's hidden as a comment (.\").

> Regardless, I think we should drop the kernel doc reference,
> as I mentioned on the SVE patches.

I actually request that people add these kinds of comments in the source.
It helps me verify details in patches, and is also handy when checking
details in the future.

> With that:
> 
> Acked-by: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks.

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Michael Kerrisk (man-pages)]

Hi Will,

Thanks for reviewing.

On 6/9/20 12:02 PM, Will Deacon wrote:
> On Wed, May 27, 2020 at 10:17:37PM +0100, Dave Martin wrote:
>> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
>> 5.0 for arm64.
> 
> [...]
> 
>> +If the arguments are invalid,
>> +and in particular if
>> +.I arg2
>> +contains set bits that are unrecognized
>> +or that correspond to a key not available on this platform,
>> +the call fails with error
>> +.BR EINVAL .
>> +.IP
>> +.B Warning:
>> +Because the compiler or run-time environment
>> +may be using some or all of the keys,
>> +a successful
>> +.IP
>> +For more information, see the kernel source file
>> +.I Documentation/arm64/pointer\-authentication.rst
>> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
>> +(or
>> +.I Documentation/arm64/pointer\-authentication.txt
>> +before Linux 5.3).
>> +.B PR_PAC_RESET_KEYS
>> +may crash the calling process.
> 
> I might be misreading this, but this looks like the kernel reference appears
> mid-sentence. 

It's hidden as a comment (.\").

> Regardless, I think we should drop the kernel doc reference,
> as I mentioned on the SVE patches.

I actually request that people add these kinds of comments in the source.
It helps me verify details in patches, and is also handy when checking
details in the future.

> With that:
> 
> Acked-by: Will Deacon <will@kernel.org>

Thanks.

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Michael Kerrisk (man-pages)]

Hello Dave,

I've applied this patch (manually, because 4/6 is not yet applied).
I have a question below.

On 5/27/20 11:17 PM, Dave Martin wrote:
> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
> 5.0 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org>
> Cc: Will Deacon <will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Amit Daniel Kachhap <amit.kachhap-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> 
> ---
> 
> Since v1:
> 
>  * Clarify explicitly that PR_PAC_RESET_KEYS is redundant when combined
>    with execve().
> 
>  * Move error condition details into the prctl description, to avoid
>    excessive duplication while keeping keeping related pieces of text
>    closer together.
> 
>  * In lieu of having a separate man page to cross reference for detailed
>    guidance, cross-reference the kernel documentation.
> 
>  * Add safety warning.  This is deliberately vague, pending ongoing
>    discussions with libc folks.
> ---
>  man2/prctl.2 | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 

[...]

> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using some or all of the keys,
> +a successful

Things got a bit garbled here. I think the next few lines should have been 
at the end.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/pointer\-authentication.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/pointer\-authentication.txt
> +before Linux 5.3).
> +.B PR_PAC_RESET_KEYS
> +may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.

I applied the following change after your patch; is it okay?

 .IP
 .B Warning:
 Because the compiler or run-time environment
 may be using some or all of the keys,
 a successful
+may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.
 .IP
 For more information, see the kernel source file
 .I Documentation/arm64/pointer\-authentication.rst
@@ -1020,9 +1023,6 @@ For more information, see the kernel source file
 .I Documentation/arm64/pointer\-authentication.txt
 before Linux 5.3).
 .B PR_PAC_RESET_KEYS
-may crash the calling process.
-The conditions for using it safely are complex and system-dependent.
-Don't use it unless you know what you are doing.
 .\" prctl PR_SET_PDEATHSIG
 .TP
 .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)"

[...]

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Michael Kerrisk (man-pages)]

Hello Dave,

I've applied this patch (manually, because 4/6 is not yet applied).
I have a question below.

On 5/27/20 11:17 PM, Dave Martin wrote:
> Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
> 5.0 for arm64.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> 
> ---
> 
> Since v1:
> 
>  * Clarify explicitly that PR_PAC_RESET_KEYS is redundant when combined
>    with execve().
> 
>  * Move error condition details into the prctl description, to avoid
>    excessive duplication while keeping keeping related pieces of text
>    closer together.
> 
>  * In lieu of having a separate man page to cross reference for detailed
>    guidance, cross-reference the kernel documentation.
> 
>  * Add safety warning.  This is deliberately vague, pending ongoing
>    discussions with libc folks.
> ---
>  man2/prctl.2 | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 

[...]

> +.IP
> +.B Warning:
> +Because the compiler or run-time environment
> +may be using some or all of the keys,
> +a successful

Things got a bit garbled here. I think the next few lines should have been 
at the end.
> +.IP
> +For more information, see the kernel source file
> +.I Documentation/arm64/pointer\-authentication.rst
> +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> +(or
> +.I Documentation/arm64/pointer\-authentication.txt
> +before Linux 5.3).
> +.B PR_PAC_RESET_KEYS
> +may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.

I applied the following change after your patch; is it okay?

 .IP
 .B Warning:
 Because the compiler or run-time environment
 may be using some or all of the keys,
 a successful
+may crash the calling process.
+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.
 .IP
 For more information, see the kernel source file
 .I Documentation/arm64/pointer\-authentication.rst
@@ -1020,9 +1023,6 @@ For more information, see the kernel source file
 .I Documentation/arm64/pointer\-authentication.txt
 before Linux 5.3).
 .B PR_PAC_RESET_KEYS
-may crash the calling process.
-The conditions for using it safely are complex and system-dependent.
-Don't use it unless you know what you are doing.
 .\" prctl PR_SET_PDEATHSIG
 .TP
 .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)"

[...]

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Dave Martin]

On Tue, Jun 09, 2020 at 01:36:42PM +0200, Michael Kerrisk (man-pages) wrote:
> Hello Dave,
> 
> I've applied this patch (manually, because 4/6 is not yet applied).
> I have a question below.
> 
> On 5/27/20 11:17 PM, Dave Martin wrote:
> > Add documentation for the PR_PAC_RESET_KEYS ioctl added in Linux
> > 5.0 for arm64.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > 
> > ---
> > 
> > Since v1:
> > 
> >  * Clarify explicitly that PR_PAC_RESET_KEYS is redundant when combined
> >    with execve().
> > 
> >  * Move error condition details into the prctl description, to avoid
> >    excessive duplication while keeping keeping related pieces of text
> >    closer together.
> > 
> >  * In lieu of having a separate man page to cross reference for detailed
> >    guidance, cross-reference the kernel documentation.
> > 
> >  * Add safety warning.  This is deliberately vague, pending ongoing
> >    discussions with libc folks.
> > ---
> >  man2/prctl.2 | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> > 
> 
> [...]
> 
> > +.IP
> > +.B Warning:
> > +Because the compiler or run-time environment
> > +may be using some or all of the keys,
> > +a successful
> 
> Things got a bit garbled here. I think the next few lines should have been 
> at the end.
> > +.IP
> > +For more information, see the kernel source file
> > +.I Documentation/arm64/pointer\-authentication.rst
> > +.\"commit b693d0b372afb39432e1c49ad7b3454855bc6bed
> > +(or
> > +.I Documentation/arm64/pointer\-authentication.txt
> > +before Linux 5.3).
> > +.B PR_PAC_RESET_KEYS
> > +may crash the calling process.
> > +The conditions for using it safely are complex and system-dependent.
> > +Don't use it unless you know what you are doing.
> 
> I applied the following change after your patch; is it okay?
> 
>  .IP
>  .B Warning:
>  Because the compiler or run-time environment
>  may be using some or all of the keys,
>  a successful

Looks fine, execpt that I think you need to move the

	.B PR_PAC_RESET_KEYS

line here also.

Thanks
---Dave

> +may crash the calling process.
> +The conditions for using it safely are complex and system-dependent.
> +Don't use it unless you know what you are doing.
>  .IP
>  For more information, see the kernel source file
>  .I Documentation/arm64/pointer\-authentication.rst
> @@ -1020,9 +1023,6 @@ For more information, see the kernel source file
>  .I Documentation/arm64/pointer\-authentication.txt
>  before Linux 5.3).
>  .B PR_PAC_RESET_KEYS
> -may crash the calling process.
> -The conditions for using it safely are complex and system-dependent.
> -Don't use it unless you know what you are doing.
>  .\" prctl PR_SET_PDEATHSIG
>  .TP
>  .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)"
> 

[...]

Re: [PATCH v2 5/6] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

[Michael Kerrisk (man-pages)]

>> I applied the following change after your patch; is it okay?
>>
>>  .IP
>>  .B Warning:
>>  Because the compiler or run-time environment
>>  may be using some or all of the keys,
>>  a successful
> 
> Looks fine, execpt that I think you need to move the
> 
> 	.B PR_PAC_RESET_KEYS
> 
> line here also.

Thanks. Fixed. But, the fix will be in the 5.08 man-pages release,
I'm sorry. I cut 5.07 a few hours ago.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v2 0/6] prctl.2 man page updates for Linux 5.6

[Michael Kerrisk (man-pages)]

Hi Dave

On Wed, 27 May 2020 at 23:17, Dave Martin <Dave.Martin@arm.com> wrote:
>
> A bunch of updates to the prctl(2) man page to fill in missing
> prctls (mostly) up to Linux 5.6 (along with a few other tweaks and
> fixes).
>
> Patches from the v1 series [1] that have been applied or rejected
> already have been dropped.
>
> People not Cc'd on the whole series can find the whole series at
> https://lore.kernel.org/linux-man/ .
>
> Patches:
>
>  * Patch 1 is a new (but trivial) formatting fix, unrelated to the new
>    prctls.

Applied.

>  * Patches 2-3 relate to the speculation control prctls.  These are
>    unmodified from v1, but need review.

Applied, and pushed (since there were no review comments from last version).

>  * Patches 4-5 relate to the arm64 prctls from v1, with reviewer
>    feedback incorporated.  (See notes in the patches.)

I'll hold off on these patches, to see if review comments come in.

>  * Patch 6 is *draft* wording for the arm64 address tagging prctls.
>    The semantics of address tagging is particularly slippery, so
>    this needs discussion before merging.

Okay -- I'll hold off with that patch too.

Cheers,

Michael

> [1] https://lore.kernel.org/linux-man/29a02b16-dd61-6186-1340-fcc7d5225ad0@gmail.com/T/#t
>
>
> Dave Martin (6):
>   prctl.2: ffix use literal hyphens when referencing kernel docs
>   prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
>   prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
>   prctl.2: Add SVE prctls (arm64)
>   prctl.2: Add PR_PAC_RESET_KEYS (arm64)
>   prctl.2: Add tagged address ABI control prctls (arm64)
>
>  man2/prctl.2 | 444 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 435 insertions(+), 9 deletions(-)
>
> --
> 2.1.4
>

Re: [PATCH v2 0/6] prctl.2 man page updates for Linux 5.6

[Michael Kerrisk (man-pages)]

Hi Dave

On Wed, 27 May 2020 at 23:17, Dave Martin <Dave.Martin@arm.com> wrote:
>
> A bunch of updates to the prctl(2) man page to fill in missing
> prctls (mostly) up to Linux 5.6 (along with a few other tweaks and
> fixes).
>
> Patches from the v1 series [1] that have been applied or rejected
> already have been dropped.
>
> People not Cc'd on the whole series can find the whole series at
> https://lore.kernel.org/linux-man/ .
>
> Patches:
>
>  * Patch 1 is a new (but trivial) formatting fix, unrelated to the new
>    prctls.

Applied.

>  * Patches 2-3 relate to the speculation control prctls.  These are
>    unmodified from v1, but need review.

Applied, and pushed (since there were no review comments from last version).

>  * Patches 4-5 relate to the arm64 prctls from v1, with reviewer
>    feedback incorporated.  (See notes in the patches.)

I'll hold off on these patches, to see if review comments come in.

>  * Patch 6 is *draft* wording for the arm64 address tagging prctls.
>    The semantics of address tagging is particularly slippery, so
>    this needs discussion before merging.

Okay -- I'll hold off with that patch too.

Cheers,

Michael

> [1] https://lore.kernel.org/linux-man/29a02b16-dd61-6186-1340-fcc7d5225ad0@gmail.com/T/#t
>
>
> Dave Martin (6):
>   prctl.2: ffix use literal hyphens when referencing kernel docs
>   prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
>   prctl.2: Add PR_SPEC_DISABLE_NOEXEC for SPECULATION_CTRL prctls
>   prctl.2: Add SVE prctls (arm64)
>   prctl.2: Add PR_PAC_RESET_KEYS (arm64)
>   prctl.2: Add tagged address ABI control prctls (arm64)
>
>  man2/prctl.2 | 444 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 435 insertions(+), 9 deletions(-)
>
> --
> 2.1.4
>


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/