Re: [PATCH 14/14] prctl.2: Add PR_PAC_RESET_KEYS (arm64)

From: Dave Martin <Dave.Martin@arm.com>
To: Will Deacon <will@kernel.org>
Cc: mtk.manpages@gmail.com, linux-man@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH 14/14] prctl.2: Add PR_PAC_RESET_KEYS (arm64)
Date: Wed, 13 May 2020 15:36:54 +0100	[thread overview]
Message-ID: <20200513143653.GQ21779@arm.com> (raw)
In-Reply-To: <20200513072530.GA18196@willie-the-truck>

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