| Message ID | 1589301419-24459-15-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | prctl.2 man page updates for Linux 5.6 | expand |
Hi Dave, On Tue, May 12, 2020 at 05:36:59PM +0100, Dave Martin wrote: > diff --git a/man2/prctl.2 b/man2/prctl.2 > index dd16227..7ea60e2 100644 > --- a/man2/prctl.2 > +++ b/man2/prctl.2 > @@ -950,6 +950,46 @@ 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 creating a new execution context. I see what you're saying, but the keys are also reset on exec() iirc, so we don't want to encourage people to issue the prctl() unnecessarily immediately following an exec(). > +.IP > +The remaining arguments > +.IR arg3 ", " arg4 " and " arg5 > +must all be zero. > .\" prctl PR_SET_PDEATHSIG > .TP > .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)" > @@ -1920,6 +1960,27 @@ are not 0. > .B EINVAL > .I option > was > +.B PR_PAC_RESET_KEYS > +and > +.I arg2 > +contains non-zero bits other than > +.BR > +.BR PR_PAC_APIAKEY , > +.BR PR_PAC_APIBKEY , > +.BR PR_PAC_APDAKEY , > +.B PR_PAC_APDBKEY > +and > +.BR PR_PAC_APGAKEY ; > +or > +.IR arg3 , > +.I arg4 > +and > +.I arg5 > +were not all zero. Do we care about other reasons for -EINVAL, such as the system not supporting pointer authentication? Will
On Wed, May 13, 2020 at 08:25:31AM +0100, Will Deacon wrote: > Hi Dave, > > On Tue, May 12, 2020 at 05:36:59PM +0100, Dave Martin wrote: > > diff --git a/man2/prctl.2 b/man2/prctl.2 > > index dd16227..7ea60e2 100644 > > --- a/man2/prctl.2 > > +++ b/man2/prctl.2 > > @@ -950,6 +950,46 @@ 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 creating a new execution context. > > I see what you're saying, but the keys are also reset on exec() iirc, so we > don't want to encourage people to issue the prctl() unnecessarily > immediately following an exec(). I thought of saying that, then pulled it out again. How about: "[...] a new execution context within an existing process. Note that execve() always resets all the keys as part of its operation, without the need for this prctl() call. PR_PAC_RESET_KEYS is intended for custom situations that do not involve execve(), such as creating a new managed run-time sandbox." I deliberately don't say "thread" because that's probably libc's job. I'll need to check glibc does, though. There may be issues with pthreads semantics that mean we can't reset the keys there. > > > +.IP > > +The remaining arguments > > +.IR arg3 ", " arg4 " and " arg5 > > +must all be zero. > > .\" prctl PR_SET_PDEATHSIG > > .TP > > .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)" > > @@ -1920,6 +1960,27 @@ are not 0. > > .B EINVAL > > .I option > > was > > +.B PR_PAC_RESET_KEYS > > +and > > +.I arg2 > > +contains non-zero bits other than > > +.BR > > +.BR PR_PAC_APIAKEY , > > +.BR PR_PAC_APIBKEY , > > +.BR PR_PAC_APDAKEY , > > +.B PR_PAC_APDBKEY > > +and > > +.BR PR_PAC_APGAKEY ; > > +or > > +.IR arg3 , > > +.I arg4 > > +and > > +.I arg5 > > +were not all zero. > > Do we care about other reasons for -EINVAL, such as the system not > supporting pointer authentication? Again, I tried to catch that under the new "not supported by this platform" wording in the earlier patch. Do you think that's sufficient, or do we need something else here? Cheers ---Dave
On Wed, May 13, 2020 at 03:36:54PM +0100, Dave Martin wrote: > On Wed, May 13, 2020 at 08:25:31AM +0100, Will Deacon wrote: > > On Tue, May 12, 2020 at 05:36:59PM +0100, Dave Martin wrote: > > > +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 creating a new execution context. > > > > I see what you're saying, but the keys are also reset on exec() iirc, so we > > don't want to encourage people to issue the prctl() unnecessarily > > immediately following an exec(). > > I thought of saying that, then pulled it out again. > > How about: > > "[...] a new execution context within an existing process. Note that > execve() always resets all the keys as part of its operation, without > the need for this prctl() call. PR_PAC_RESET_KEYS is intended for > custom situations that do not involve execve(), such as creating a new > managed run-time sandbox." > > I deliberately don't say "thread" because that's probably libc's job. > I'll need to check glibc does, though. There may be issues with > pthreads semantics that mean we can't reset the keys there. That's better, but you may even be able to drop the "such as..." part, I reckon. > > > @@ -1920,6 +1960,27 @@ are not 0. > > > .B EINVAL > > > .I option > > > was > > > +.B PR_PAC_RESET_KEYS > > > +and > > > +.I arg2 > > > +contains non-zero bits other than > > > +.BR > > > +.BR PR_PAC_APIAKEY , > > > +.BR PR_PAC_APIBKEY , > > > +.BR PR_PAC_APDAKEY , > > > +.B PR_PAC_APDBKEY > > > +and > > > +.BR PR_PAC_APGAKEY ; > > > +or > > > +.IR arg3 , > > > +.I arg4 > > > +and > > > +.I arg5 > > > +were not all zero. > > > > Do we care about other reasons for -EINVAL, such as the system not > > supporting pointer authentication? > > Again, I tried to catch that under the new "not supported by this > platform" wording in the earlier patch. Do you think that's sufficient, > or do we need something else here? As long as it's clear that the prctl() *can* fail and userspace can't just ignore the return value, then I'm happy. If it's not obvious, then spelling it out seems harmless to me. Will
On Wed, May 13, 2020 at 10:00:22PM +0100, Will Deacon wrote: > On Wed, May 13, 2020 at 03:36:54PM +0100, Dave Martin wrote: > > On Wed, May 13, 2020 at 08:25:31AM +0100, Will Deacon wrote: > > > On Tue, May 12, 2020 at 05:36:59PM +0100, Dave Martin wrote: > > > > +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 creating a new execution context. > > > > > > I see what you're saying, but the keys are also reset on exec() iirc, so we > > > don't want to encourage people to issue the prctl() unnecessarily > > > immediately following an exec(). > > > > I thought of saying that, then pulled it out again. > > > > How about: > > > > "[...] a new execution context within an existing process. Note that > > execve() always resets all the keys as part of its operation, without > > the need for this prctl() call. PR_PAC_RESET_KEYS is intended for > > custom situations that do not involve execve(), such as creating a new > > managed run-time sandbox." > > > > I deliberately don't say "thread" because that's probably libc's job. > > I'll need to check glibc does, though. There may be issues with > > pthreads semantics that mean we can't reset the keys there. > > That's better, but you may even be able to drop the "such as..." part, I > reckon. > > > > > @@ -1920,6 +1960,27 @@ are not 0. > > > > .B EINVAL > > > > .I option > > > > was > > > > +.B PR_PAC_RESET_KEYS > > > > +and > > > > +.I arg2 > > > > +contains non-zero bits other than > > > > +.BR > > > > +.BR PR_PAC_APIAKEY , > > > > +.BR PR_PAC_APIBKEY , > > > > +.BR PR_PAC_APDAKEY , > > > > +.B PR_PAC_APDBKEY > > > > +and > > > > +.BR PR_PAC_APGAKEY ; > > > > +or > > > > +.IR arg3 , > > > > +.I arg4 > > > > +and > > > > +.I arg5 > > > > +were not all zero. > > > > > > Do we care about other reasons for -EINVAL, such as the system not > > > supporting pointer authentication? > > > > Again, I tried to catch that under the new "not supported by this > > platform" wording in the earlier patch. Do you think that's sufficient, > > or do we need something else here? > > As long as it's clear that the prctl() *can* fail and userspace can't just > ignore the return value, then I'm happy. If it's not obvious, then spelling > it out seems harmless to me. OK, I'll try to figure out a way to capture this. Since prctl is really the wild west when it comes to error codes I was presuming that's it's best to say nothing and rely on people's common sense. But I guess this isn't great either. How about summarising the key error cases here, and just putting a cross- reference in the ERRORS section rather than trying to describe them there? I really don't want to duplicate this stuff -- that will get unmaintanable, fast (if it hasn't already). Cheers ---Dave
On Mon, May 18, 2020 at 05:11:28PM +0100, Dave Martin wrote: > How about summarising the key error cases here, and just putting a cross- > reference in the ERRORS section rather than trying to describe them > there? I really don't want to duplicate this stuff -- that will get > unmaintanable, fast (if it hasn't already). Makes sense to me. Will
diff --git a/man2/prctl.2 b/man2/prctl.2 index dd16227..7ea60e2 100644 --- a/man2/prctl.2 +++ b/man2/prctl.2 @@ -950,6 +950,46 @@ 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 creating a new execution context. +.IP +The remaining arguments +.IR arg3 ", " arg4 " and " arg5 +must all be zero. .\" prctl PR_SET_PDEATHSIG .TP .BR PR_SET_PDEATHSIG " (since Linux 2.1.57)" @@ -1920,6 +1960,27 @@ are not 0. .B EINVAL .I option was +.B PR_PAC_RESET_KEYS +and +.I arg2 +contains non-zero bits other than +.BR +.BR PR_PAC_APIAKEY , +.BR PR_PAC_APIBKEY , +.BR PR_PAC_APDAKEY , +.B PR_PAC_APDBKEY +and +.BR PR_PAC_APGAKEY ; +or +.IR arg3 , +.I arg4 +and +.I arg5 +were not all zero. +.TP +.B EINVAL +.I option +was .B PR_SVE_SET_VL and .I arg2
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> --- Note that the comment about PR_PAC_RESET_KEYS(0) being the recommended way to set up a fresh execution context is not present in the existing kernel documentation. I vaguely remember some discussion to this effect; in any case, it seems sensible, given that there must have been _some_ rationale for this feature... Shout if it sounds wrong! --- man2/prctl.2 | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)