[PATCH v3 0/3] userfaultfd man page updates

[PATCH v3 0/3] userfaultfd man page updates

[Axel Rasmussen]

This series includes only the remaining patches not applied from v1/v2,
with review comments addressed. This series is based on the "contrib"
branch [1].

Changelog:

v2->v3:
 - Rebased onto updated contrib branch.
 - In patch 2 (patch 4 in v2), reordered error codes in alphabetical order,
   and fixed extra spacing in the EINVAL error description.
 - In patch 3, fix mistakenly copy-pasted "struct uffdio_continue" (should
   have been "struct uffdio_poison"), and alphabetically order
   UFFDIO_CONTINUE error codes.

v1->v2:
 - In patch 1 (patch 5 in v1), change "after" to "since" for consistency
   and to be clear that we mean 4.11+ (inclusive).
 - In patch 2 (patch 7 in v1), reorder error codes alphabetically (EINVAL
   then EPERM).
 - In patch 3 (patch 8 in v1), resolve conflicts with earlier review
   comments.

Original cover letter:

Various updates for userfaultfd man pages. To summarize the changes:

- Correctly / fully describe the two-step feature support handshake
  process.
- Describe new UFFDIO_POISON ioctl.
- Other small improvements (missing ioctls, error codes, etc).

[1]: https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/log/?h=contrib

Axel Rasmussen (3):
  ioctl_userfaultfd.2: clarify the state of the uffdio_api structure on
    error
  ioctl_userfaultfd.2: fix / update UFFDIO_REGISTER error code list
  ioctl_userfaultfd.2: document new UFFDIO_POISON ioctl

 man2/ioctl_userfaultfd.2 | 173 ++++++++++++++++++++++++++++++++-------
 1 file changed, 145 insertions(+), 28 deletions(-)

--
2.42.0.655.g421f12c284-goog

[PATCH v3 1/3] ioctl_userfaultfd.2: clarify the state of the uffdio_api structure on error

[Axel Rasmussen]

The old FIXME noted that the zeroing was done to differentiate the two
EINVAL cases. It's possible something like this was true historically,
but in current Linux we zero it in *both* EINVAL cases, so this is at
least no longer true.

After reading the code, I can't determine any clear reason why we zero
it in some cases but not in others. So, some simple advice we can give
userspace is: if an error occurs, treat the contents of the structure as
unspecified. Just re-initialize it before retrying UFFDIO_API again.

Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 man2/ioctl_userfaultfd.2 | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index e68085262..82aee667c 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -272,6 +272,14 @@ operation returns 0 on success.
 On error, \-1 is returned and
 .I errno
 is set to indicate the error.
+If an error occurs,
+the kernel may zero the provided
+.I uffdio_api
+structure.
+The caller should treat its contents as unspecified,
+and reinitialize it before re-attempting another
+.B UFFDIO_API
+call.
 Possible errors include:
 .TP
 .B EFAULT
@@ -305,14 +313,6 @@ feature was enabled,
 but the calling process doesn't have the
 .B CAP_SYS_PTRACE
 capability.
-.\" FIXME In the above error case, the returned 'uffdio_api' structure is
-.\" zeroed out. Why is this done? This should be explained in the manual page.
-.\"
-.\" Mike Rapoport:
-.\"     In my understanding the uffdio_api
-.\"     structure is zeroed to allow the caller
-.\"     to distinguish the reasons for -EINVAL.
-.\"
 .SS UFFDIO_REGISTER
 (Since Linux 4.3.)
 Register a memory address range with the userfaultfd object.
-- 
2.42.0.655.g421f12c284-goog

Re: [PATCH v3 1/3] ioctl_userfaultfd.2: clarify the state of the uffdio_api structure on error

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

On Tue, Oct 17, 2023 at 04:01:08PM -0700, Axel Rasmussen wrote:
> The old FIXME noted that the zeroing was done to differentiate the two
> EINVAL cases. It's possible something like this was true historically,
> but in current Linux we zero it in *both* EINVAL cases, so this is at
> least no longer true.
> 
> After reading the code, I can't determine any clear reason why we zero
> it in some cases but not in others. So, some simple advice we can give
> userspace is: if an error occurs, treat the contents of the structure as
> unspecified. Just re-initialize it before retrying UFFDIO_API again.
> 
> Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>

Hi Axel,

Patch applied.  Thanks, and thank you too Mike for the review.

Cheers,
Alex

> ---
>  man2/ioctl_userfaultfd.2 | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index e68085262..82aee667c 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -272,6 +272,14 @@ operation returns 0 on success.
>  On error, \-1 is returned and
>  .I errno
>  is set to indicate the error.
> +If an error occurs,
> +the kernel may zero the provided
> +.I uffdio_api
> +structure.
> +The caller should treat its contents as unspecified,
> +and reinitialize it before re-attempting another
> +.B UFFDIO_API
> +call.
>  Possible errors include:
>  .TP
>  .B EFAULT
> @@ -305,14 +313,6 @@ feature was enabled,
>  but the calling process doesn't have the
>  .B CAP_SYS_PTRACE
>  capability.
> -.\" FIXME In the above error case, the returned 'uffdio_api' structure is
> -.\" zeroed out. Why is this done? This should be explained in the manual page.
> -.\"
> -.\" Mike Rapoport:
> -.\"     In my understanding the uffdio_api
> -.\"     structure is zeroed to allow the caller
> -.\"     to distinguish the reasons for -EINVAL.
> -.\"
>  .SS UFFDIO_REGISTER
>  (Since Linux 4.3.)
>  Register a memory address range with the userfaultfd object.
> -- 
> 2.42.0.655.g421f12c284-goog
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 2/3] ioctl_userfaultfd.2: fix / update UFFDIO_REGISTER error code list

[Axel Rasmussen]

The list of error codes in the man page was out of date with respect to
the current state of the kernel. Some errors were partially /
incorrectly described.

Update the error code listing, so it matches the current state of the
kernel, and correctly describes all the errors.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 man2/ioctl_userfaultfd.2 | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index 82aee667c..40528001e 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -413,8 +413,6 @@ On error, \-1 is returned and
 .I errno
 is set to indicate the error.
 Possible errors include:
-.\" FIXME Is the following error list correct?
-.\"
 .TP
 .B EBUSY
 A mapping in the specified range is registered with another
@@ -433,21 +431,28 @@ field; or the
 field was zero.
 .TP
 .B EINVAL
-There is no mapping in the specified address range.
-.TP
-.B EINVAL
-.I range.start
+The specified address range was invalid.
+More specifically,
+no mapping exists in the given range,
+or the mapping that exists there is invalid
+(e.g. unsupported type of memory),
+or the range values
+.RI ( range.start
+or
+.IR range.len )
+are not multiples of the relevant page size,
 or
 .I range.len
-is not a multiple of the system page size; or,
-.I range.len
-is zero; or these fields are otherwise invalid.
+is zero.
 .TP
-.B EINVAL
-There as an incompatible mapping in the specified address range.
-.\" Mike Rapoport:
-.\" ENOMEM if the process is exiting and the
-.\" mm_struct has gone by the time userfault grabs it.
+.B ENOMEM
+The process is exiting,
+and its address space has already been torn down
+when userfaultfd attempts to reference it.
+.TP
+.B EPERM
+The userfaultfd would allow writing to a file backing the mapping,
+but the calling process lacks such write permissions.
 .SS UFFDIO_UNREGISTER
 (Since Linux 4.3.)
 Unregister a memory address range from userfaultfd.
-- 
2.42.0.655.g421f12c284-goog

Re: [PATCH v3 2/3] ioctl_userfaultfd.2: fix / update UFFDIO_REGISTER error code list

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3057 bytes --]

Hi Axel, Mike,

On Tue, Oct 17, 2023 at 04:01:09PM -0700, Axel Rasmussen wrote:
> The list of error codes in the man page was out of date with respect to
> the current state of the kernel. Some errors were partially /
> incorrectly described.
> 
> Update the error code listing, so it matches the current state of the
> kernel, and correctly describes all the errors.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>

I'll wait a bit before applying this one, in case Mike (or Peter) want
to review it.

The other two, you can find them here, as usual:
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=9ae03d21383151794b118ac8dd98b4d6e26c9689>
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=5f1acc81e55540d1276977e66d64649c6ca7b58b>

And BTW, while running `make lint build check`, I caught some accidents
from a previous patch from yours:
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=bf8647d1d82205934776b474fe89e5f30e03ad25>

Cheers,
Alex

> ---
>  man2/ioctl_userfaultfd.2 | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 82aee667c..40528001e 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -413,8 +413,6 @@ On error, \-1 is returned and
>  .I errno
>  is set to indicate the error.
>  Possible errors include:
> -.\" FIXME Is the following error list correct?
> -.\"
>  .TP
>  .B EBUSY
>  A mapping in the specified range is registered with another
> @@ -433,21 +431,28 @@ field; or the
>  field was zero.
>  .TP
>  .B EINVAL
> -There is no mapping in the specified address range.
> -.TP
> -.B EINVAL
> -.I range.start
> +The specified address range was invalid.
> +More specifically,
> +no mapping exists in the given range,
> +or the mapping that exists there is invalid
> +(e.g. unsupported type of memory),
> +or the range values
> +.RI ( range.start
> +or
> +.IR range.len )
> +are not multiples of the relevant page size,
>  or
>  .I range.len
> -is not a multiple of the system page size; or,
> -.I range.len
> -is zero; or these fields are otherwise invalid.
> +is zero.
>  .TP
> -.B EINVAL
> -There as an incompatible mapping in the specified address range.
> -.\" Mike Rapoport:
> -.\" ENOMEM if the process is exiting and the
> -.\" mm_struct has gone by the time userfault grabs it.
> +.B ENOMEM
> +The process is exiting,
> +and its address space has already been torn down
> +when userfaultfd attempts to reference it.
> +.TP
> +.B EPERM
> +The userfaultfd would allow writing to a file backing the mapping,
> +but the calling process lacks such write permissions.
>  .SS UFFDIO_UNREGISTER
>  (Since Linux 4.3.)
>  Unregister a memory address range from userfaultfd.
> -- 
> 2.42.0.655.g421f12c284-goog
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 3/3] ioctl_userfaultfd.2: document new UFFDIO_POISON ioctl

[Axel Rasmussen]

This is a new feature recently added to the kernel. So, document the new
ioctl the same way we do other UFFDIO_* ioctls.

Also note the corresponding new ioctl flag we can return in response to
a UFFDIO_REGISTER call.

Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 man2/ioctl_userfaultfd.2 | 124 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 6 deletions(-)

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index 40528001e..a9131515c 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -405,6 +405,11 @@ operation is supported.
 The
 .B UFFDIO_CONTINUE
 operation is supported.
+.TP
+.B 1 << _UFFDIO_POISON
+The
+.B UFFDIO_POISON
+operation is supported.
 .PP
 This
 .BR ioctl (2)
@@ -886,6 +891,12 @@ does not equal the value that was specified in the
 .I range.len
 field.
 .TP
+.B EEXIST
+One or more pages were already mapped in the given range.
+.TP
+.B EFAULT
+No existing page could be found in the page cache for the given range.
+.TP
 .B EINVAL
 Either
 .I range.start
@@ -900,9 +911,6 @@ An invalid bit was specified in the
 .I mode
 field.
 .TP
-.B EEXIST
-One or more pages were already mapped in the given range.
-.TP
 .B ENOENT
 The faulting process has changed its virtual memory layout simultaneously with
 an outstanding
@@ -912,14 +920,118 @@ operation.
 .B ENOMEM
 Allocating memory needed to setup the page table mappings failed.
 .TP
-.B EFAULT
-No existing page could be found in the page cache for the given range.
-.TP
 .B ESRCH
 The faulting process has exited at the time of a
 .B UFFDIO_CONTINUE
 operation.
 .\"
+.SS UFFDIO_POISON
+(Since Linux 6.6.)
+Mark an address range as "poisoned".
+Future accesses to these addresses will raise a
+.B SIGBUS
+signal.
+Unlike
+.B MADV_HWPOISON
+this works by installing page table entries,
+rather than "really" poisoning the underlying physical pages.
+This means it only affects this particular address space.
+.PP
+The
+.I argp
+argument is a pointer to a
+.I uffdio_poison
+structure as shown below:
+.PP
+.in +4n
+.EX
+struct uffdio_poison {
+	struct uffdio_range range;
+	                /* Range to install poison PTE markers in */
+	__u64 mode;     /* Flags controlling the behavior of poison */
+	__s64 updated;  /* Number of bytes poisoned, or negated error */
+};
+.EE
+.in
+.PP
+The following value may be bitwise ORed in
+.I mode
+to change the behavior of the
+.B UFFDIO_POISON
+operation:
+.TP
+.B UFFDIO_POISON_MODE_DONTWAKE
+Do not wake up the thread that waits for page-fault resolution.
+.PP
+The
+.I updated
+field is used by the kernel
+to return the number of bytes that were actually poisoned,
+or an error in the same manner as
+.BR UFFDIO_COPY .
+If the value returned in the
+.I updated
+field doesn't match the value that was specified in
+.IR range.len ,
+the operation fails with the error
+.BR EAGAIN .
+The
+.I updated
+field is output-only;
+it is not read by the
+.B UFFDIO_POISON
+operation.
+.PP
+This
+.BR ioctl (2)
+operation returns 0 on success.
+In this case,
+the entire area was poisoned.
+On error, \-1 is returned and
+.I errno
+is set to indicate the error.
+Possible errors include:
+.TP
+.B EAGAIN
+The number of bytes mapped
+(i.e., the value returned in the
+.I updated
+field)
+does not equal the value that was specified in the
+.I range.len
+field.
+.TP
+.B EINVAL
+Either
+.I range.start
+or
+.I range.len
+was not a multiple of the system page size; or
+.I range.len
+was zero; or the range specified was invalid.
+.TP
+.B EINVAL
+An invalid bit was specified in the
+.I mode
+field.
+.TP
+.B EEXIST
+One or more pages were already mapped in the given range.
+.TP
+.B ENOENT
+The faulting process has changed its virtual memory layout simultaneously with
+an outstanding
+.B UFFDIO_POISON
+operation.
+.TP
+.B ENOMEM
+Allocating memory for page table entries failed.
+.TP
+.B ESRCH
+The faulting process has exited at the time of a
+.B UFFDIO_POISON
+operation.
+.\"
 .SH RETURN VALUE
 See descriptions of the individual operations, above.
 .SH ERRORS
-- 
2.42.0.655.g421f12c284-goog

Re: [PATCH v3 3/3] ioctl_userfaultfd.2: document new UFFDIO_POISON ioctl

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]

On Tue, Oct 17, 2023 at 04:01:10PM -0700, Axel Rasmussen wrote:
> This is a new feature recently added to the kernel. So, document the new
> ioctl the same way we do other UFFDIO_* ioctls.
> 
> Also note the corresponding new ioctl flag we can return in response to
> a UFFDIO_REGISTER call.
> 
> Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>

Thanks for the patch and the review.  Patch applied.

Cheers,
Alex

> ---
>  man2/ioctl_userfaultfd.2 | 124 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 6 deletions(-)
> 
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 40528001e..a9131515c 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -405,6 +405,11 @@ operation is supported.
>  The
>  .B UFFDIO_CONTINUE
>  operation is supported.
> +.TP
> +.B 1 << _UFFDIO_POISON
> +The
> +.B UFFDIO_POISON
> +operation is supported.
>  .PP
>  This
>  .BR ioctl (2)
> @@ -886,6 +891,12 @@ does not equal the value that was specified in the
>  .I range.len
>  field.
>  .TP
> +.B EEXIST
> +One or more pages were already mapped in the given range.
> +.TP
> +.B EFAULT
> +No existing page could be found in the page cache for the given range.
> +.TP
>  .B EINVAL
>  Either
>  .I range.start
> @@ -900,9 +911,6 @@ An invalid bit was specified in the
>  .I mode
>  field.
>  .TP
> -.B EEXIST
> -One or more pages were already mapped in the given range.
> -.TP
>  .B ENOENT
>  The faulting process has changed its virtual memory layout simultaneously with
>  an outstanding
> @@ -912,14 +920,118 @@ operation.
>  .B ENOMEM
>  Allocating memory needed to setup the page table mappings failed.
>  .TP
> -.B EFAULT
> -No existing page could be found in the page cache for the given range.
> -.TP
>  .B ESRCH
>  The faulting process has exited at the time of a
>  .B UFFDIO_CONTINUE
>  operation.
>  .\"
> +.SS UFFDIO_POISON
> +(Since Linux 6.6.)
> +Mark an address range as "poisoned".
> +Future accesses to these addresses will raise a
> +.B SIGBUS
> +signal.
> +Unlike
> +.B MADV_HWPOISON
> +this works by installing page table entries,
> +rather than "really" poisoning the underlying physical pages.
> +This means it only affects this particular address space.
> +.PP
> +The
> +.I argp
> +argument is a pointer to a
> +.I uffdio_poison
> +structure as shown below:
> +.PP
> +.in +4n
> +.EX
> +struct uffdio_poison {
> +	struct uffdio_range range;
> +	                /* Range to install poison PTE markers in */
> +	__u64 mode;     /* Flags controlling the behavior of poison */
> +	__s64 updated;  /* Number of bytes poisoned, or negated error */
> +};
> +.EE
> +.in
> +.PP
> +The following value may be bitwise ORed in
> +.I mode
> +to change the behavior of the
> +.B UFFDIO_POISON
> +operation:
> +.TP
> +.B UFFDIO_POISON_MODE_DONTWAKE
> +Do not wake up the thread that waits for page-fault resolution.
> +.PP
> +The
> +.I updated
> +field is used by the kernel
> +to return the number of bytes that were actually poisoned,
> +or an error in the same manner as
> +.BR UFFDIO_COPY .
> +If the value returned in the
> +.I updated
> +field doesn't match the value that was specified in
> +.IR range.len ,
> +the operation fails with the error
> +.BR EAGAIN .
> +The
> +.I updated
> +field is output-only;
> +it is not read by the
> +.B UFFDIO_POISON
> +operation.
> +.PP
> +This
> +.BR ioctl (2)
> +operation returns 0 on success.
> +In this case,
> +the entire area was poisoned.
> +On error, \-1 is returned and
> +.I errno
> +is set to indicate the error.
> +Possible errors include:
> +.TP
> +.B EAGAIN
> +The number of bytes mapped
> +(i.e., the value returned in the
> +.I updated
> +field)
> +does not equal the value that was specified in the
> +.I range.len
> +field.
> +.TP
> +.B EINVAL
> +Either
> +.I range.start
> +or
> +.I range.len
> +was not a multiple of the system page size; or
> +.I range.len
> +was zero; or the range specified was invalid.
> +.TP
> +.B EINVAL
> +An invalid bit was specified in the
> +.I mode
> +field.
> +.TP
> +.B EEXIST
> +One or more pages were already mapped in the given range.
> +.TP
> +.B ENOENT
> +The faulting process has changed its virtual memory layout simultaneously with
> +an outstanding
> +.B UFFDIO_POISON
> +operation.
> +.TP
> +.B ENOMEM
> +Allocating memory for page table entries failed.
> +.TP
> +.B ESRCH
> +The faulting process has exited at the time of a
> +.B UFFDIO_POISON
> +operation.
> +.\"
>  .SH RETURN VALUE
>  See descriptions of the individual operations, above.
>  .SH ERRORS
> -- 
> 2.42.0.655.g421f12c284-goog
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]