Build glue is not added for a reason.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 60 ↗ | (On Diff #53987) | access |
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 62 ↗ | (On Diff #53987) | an associated 4-bit protection key |
| 63 ↗ | (On Diff #53987) | A new per-thread PKRU hardware register determines |
| 64 ↗ | (On Diff #53987) | for each protection key, whether user-mode addresses with that protection key |
| 72–75 ↗ | (On Diff #53987) | 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. |
| 90 ↗ | (On Diff #53987) | The system provides |
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 72–75 ↗ | (On Diff #53987) | 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 | ||
|---|---|---|
| 55 ↗ | (On Diff #53989) | The dash here isn't needed. |
| 61 ↗ | (On Diff #53989) | Enabled by what? |
| 64 ↗ | (On Diff #53989) | No need for a comma after "key". |
| 73 ↗ | (On Diff #53989) | "... it returns the EFAULT..." |
| 77 ↗ | (On Diff #53989) | "that the system" |
| 78 ↗ | (On Diff #53989) | "only available on amd64 systems"? |
| 79 ↗ | (On Diff #53989) | Usually written "64-bit" and "32-bit". |
| 83 ↗ | (On Diff #53989) | "The key indexes" |
| 86 ↗ | (On Diff #53989) | "managed using the user-mode instructions" |
| 94 ↗ | (On Diff #53989) | "The x86_pkru_protect_range function assigns..." |
| 97 ↗ | (On Diff #53989) | "assigns key keyidx to..." |
| 99 ↗ | (On Diff #53989) | "having length len." |
| 102 ↗ | (On Diff #53989) | 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. |
| 109 ↗ | (On Diff #53989) | "the zero key" |
| 110 ↗ | (On Diff #53989) | "You must first remove any existing key with x86_pkru_unprotect_range() in order for this request to succeed. |
| 117 ↗ | (On Diff #53989) | "You must use a x86_pkru_unprotect_range call" |
| 124 ↗ | (On Diff #53989) | Missing "function" after the function name. |
| 139 ↗ | (On Diff #53989) | I think you can s/at least the //. |
| 140 ↗ | (On Diff #53989) | "the variable" |
| 142 ↗ | (On Diff #53989) | "indicates that write access" |
| 166 ↗ | (On Diff #53989) | "completion of the operation," or "the operation's completion" Is it still possible for these calls to fail with ENOMEM? |
| 181 ↗ | (On Diff #53989) | This expands to nothing, did you mean .Nm? |
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 72–75 ↗ | (On Diff #53987) | 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. |
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 61 ↗ | (On Diff #53989) | By user. More precisely, 'not disabled'. I removed the 'enabled' part. |
| 78 ↗ | (On Diff #53989) | I mean that user might run i386 on the same machine. |
| 166 ↗ | (On Diff #53989) | 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). |
Comment Actions
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 | ||
|---|---|---|
| 35 ↗ | (On Diff #53996) | In this place, the 's' on the end of 'Keys' makes my ears hurt. :-) "Protection Key Rights" would be better. |
| 79 ↗ | (On Diff #53996) | I think that 'AKA' is too informal for man pages. |
| 151 ↗ | (On Diff #53996) | "Conversely, ..." |
| lib/libc/x86/sys/pkru.3 | ||
|---|---|---|
| 74–75 ↗ | (On Diff #54020) | I wanted to highlight this sentence. My actual question is in my next comment. |
| 117–118 ↗ | (On Diff #54020) | 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 | ||
|---|---|---|
| 117–118 ↗ | (On Diff #54020) | 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 | ||
|---|---|---|
| 117–118 ↗ | (On Diff #54020) | 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? |
Comment Actions
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).