Page MenuHomeFreeBSD

pkru(3)
ClosedPublic

Authored by kib on Feb 16 2019, 1:36 PM.
Tags
None
Referenced Files
F80105652: D19211.id53987.diff
Wed, Mar 27, 11:56 PM
Unknown Object (File)
Dec 26 2023, 4:31 AM
Unknown Object (File)
Dec 22 2023, 10:03 PM
Unknown Object (File)
Dec 5 2023, 9:34 AM
Unknown Object (File)
Nov 10 2023, 1:19 AM
Unknown Object (File)
Nov 6 2023, 8:13 PM
Unknown Object (File)
Nov 5 2023, 6:08 PM
Unknown Object (File)
Nov 4 2023, 4:50 AM

Details

Reviewers
alc
markj
pho
jilles
Group Reviewers
manpages
Commits
rS344355: pkru(3) man page.
Summary

Build glue is not added for a reason.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 22556

Event Timeline

lib/libc/x86/sys/pkru.3
61

access

jilles added inline comments.
lib/libc/x86/sys/pkru.3
63

an associated 4-bit protection key

64

A new per-thread PKRU hardware register determines

65

for each protection key, whether user-mode addresses with that protection key

73–76

Perhaps it should be mentioned that this EFAULT is not always a "proper" error in the sense that a failed system call may have been partially successful. For example, if sigaction(2) could not write to *oact, it will return the EFAULT error code even if the signal disposition was updated.

It is best for applications to pass pointers that may cause protection faults only for data buffers for read, write and the like, where the error can be properly reported (either as immediate EFAULT or as an incomplete transfer).

If the EFAULT is used as a trigger to abort the process, the above is not a problem.

91

The system provides

kib marked 5 inline comments as done.Feb 16 2019, 3:44 PM
kib added inline comments.
lib/libc/x86/sys/pkru.3
73–76

I am not sure what to do about this note and even how to interpret it. If taken literally, I have to note that sigaction(2) would fail in the same manner if oact is invalid but non-NULL pointer. So really the behavior of the syscalls is same as if passed pointer was invalid (but not NULL) or valid but readonly. In the corner case, it might be even a race where initially usable pointer now points to unusable (e.g. not writable) page.

So I do not think that this additional permission check changes potential syscalls outcome.

What it changes is that it makes easier to make useracc(9) useless, but this falls under the already acceptable behavior.

lib/libc/x86/sys/pkru.3
56

The dash here isn't needed.

62

Enabled by what?

65

No need for a comma after "key".

74

"... it returns the EFAULT..."

78

"that the system"

79

"only available on amd64 systems"?

80

Usually written "64-bit" and "32-bit".

84

"The key indexes"

87

"managed using the user-mode instructions"

95

"The x86_pkru_protect_range function assigns..."

98

"assigns key keyidx to..."

100

"having length len."

103

It should probably be explained that only one key may apply to a given range at a time. If _EXCL is not specified, x86_pkru_protect_range() will replace any existing key.

110

"the zero key"

111

"You must first remove any existing key with x86_pkru_unprotect_range() in order for this request to succeed.

118

"You must use a x86_pkru_unprotect_range call"

125

Missing "function" after the function name.

140

I think you can s/at least the //.

141

"the variable"

143

"indicates that write access"

167

"completion of the operation," or "the operation's completion"

Is it still possible for these calls to fail with ENOMEM?

182

This expands to nothing, did you mean .Nm?

lib/libc/x86/sys/pkru.3
73–76

My point is that while you can fix up userspace accesses by catching SIGSEGV, remapping the faulting address and returning from the signal handler, there is no way to do something similar for system calls in the general case. Reading the standards strictly, it is undefined behaviour to pass a pointer to a system function that does not permit the required access (where NULL is also valid if documented for the function). PKRU does not change this, but makes it more likely to trigger.

Perhaps a sentence "Note that some side effects may have occurred if this error is reported." could be added.

0mp added inline comments.
lib/libc/x86/sys/pkru.3
3

All rights reserved is usually omitted in recent commits.

101

.Fa len . (missing space)

kib marked 24 inline comments as done.

Editing.

lib/libc/x86/sys/pkru.3
62

By user. More precisely, 'not disabled'. I removed the 'enabled' part.

79

I mean that user might run i386 on the same machine.

167

I want to document the interface. Not returning ENOMEM is actually an impl detail, e.g. we can start enforcing the limit (and probably should).

markj added inline comments.
lib/libc/x86/sys/pkru.3
84

Should be "Developer's"

93

"for both"

137

s/call/function/

167

Ok.

This revision is now accepted and ready to land.Feb 17 2019, 5:07 PM
This revision now requires review to proceed.Feb 17 2019, 5:31 PM

I question if there shouldn't be an explicit statement somewhere in this man page that this feature can be used by an application to implement memory "safety" mechanisms, but not "security". Because the RDPKRU and WRPKRU instructions are not privileged, a compromised application could always change the access rights. This is different from the protection key mechanisms on other architectures such as 32-bit ARM.

lib/libc/x86/sys/pkru.3
36

In this place, the 's' on the end of 'Keys' makes my ears hurt. :-) "Protection Key Rights" would be better.

80

I think that 'AKA' is too informal for man pages.

152

"Conversely, ..."

kib marked 7 inline comments as done.

Alan' notes.

This revision is now accepted and ready to land.Feb 17 2019, 9:15 PM
lib/libc/x86/sys/pkru.3
75–76

I wanted to highlight this sentence. My actual question is in my next comment.

118–119

The earlier sentence that I highlighted could be interpreted as any address range has a initial, default key value of zero. But AMD64_PKRU_EXCL fails even for a zero key, so such an operation could never succeed.

I assume that this is not the intended interpretation, i.e., that there can be address ranges with no associated key. Yes?

lib/libc/x86/sys/pkru.3
118–119

When PKRU is enabled, hardware requires that any valid pte has a key assigned. By default, the key zero is installed.

There is a separate, software managed key, that can be in two states: assigned for a range, or not. If assigned, it can be removed, and then the state would be not assigned (in ptes, zero key is installed then). For _EXCL to succeed, there must be no sw key assigned to any page in the range.

lib/libc/x86/sys/pkru.3
118–119

This distinction between the hardware keys and the software managed keys doesn't really appear in the text, at least explicitly. Can you please add some text?

Consistently use the term 'assigned' to describe keys.
Explain assignment state changes and mention that hardware key zero is not a substitute for assigning key zero (which probably no useful app would do anyway).

This revision now requires review to proceed.Feb 18 2019, 5:04 PM
alc added inline comments.
lib/libc/x86/sys/pkru.3
112

"After a successful call, ..."

113

"changed" -> "change"

This revision is now accepted and ready to land.Feb 19 2019, 3:45 AM
This revision was automatically updated to reflect the committed changes.