diff mbox series

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

Message ID 20231017230110.3170850-2-axelrasmussen@google.com
State New
Headers
Series userfaultfd man page updates |

Commit Message

Axel Rasmussen Oct. 17, 2023, 11:01 p.m. UTC 
  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(-)

Comments

Alejandro Colomar Oct. 17, 2023, 11:07 p.m. UTC | #1 
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
>
diff mbox series

Patch

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.