2023-09-19 20:05:33

by Axel Rasmussen

[permalink] [raw]
Subject: [PATCH 07/10] ioctl_userfaultfd.2: correct and update UFFDIO_API ioctl error codes

First, it is not correct that repeated UFFDIO_API calls result in
EINVAL. This is true *if both calls enable features*, but in the case
where we're doing a two-step feature detection handshake, the kernel
explicitly expects 2 calls (one with no features set). So, correct this
description.

Then, some new error cases have been added to the kernel recently, and
the man page wasn't updated to note these. So, add in descriptions of
these new error cases.

Signed-off-by: Axel Rasmussen <[email protected]>
---
man2/ioctl_userfaultfd.2 | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
index 53b1f473f..1aa9654be 100644
--- a/man2/ioctl_userfaultfd.2
+++ b/man2/ioctl_userfaultfd.2
@@ -280,17 +280,31 @@ refers to an address that is outside the calling process's
accessible address space.
.TP
.B EINVAL
-The userfaultfd has already been enabled by a previous
-.B UFFDIO_API
-operation.
-.TP
-.B EINVAL
The API version requested in the
.I api
field is not supported by this kernel, or the
.I features
field passed to the kernel includes feature bits that are not supported
by the current kernel version.
+.TP
+.B EPERM
+The
+.B UFFD_FEATURE_EVENT_FORK
+feature was enabled,
+but the calling process doesn't have the
+.B CAP_SYS_PTRACE
+capability.
+.TP
+.B EINVAL
+A previous
+.B UFFDIO_API
+call already enabled one or more features for this userfaultfd.
+Calling
+.B UFFDIO_API
+twice,
+the first time with no features set,
+is explicitly allowed
+as per the two-step feature detection handshake.
.\" 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.
.\"
--
2.42.0.459.ge4e396fd5e-goog


2023-09-25 23:53:29

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 07/10] ioctl_userfaultfd.2: correct and update UFFDIO_API ioctl error codes

Hi Axel,

On Tue, Sep 19, 2023 at 12:02:03PM -0700, Axel Rasmussen wrote:
> First, it is not correct that repeated UFFDIO_API calls result in
> EINVAL. This is true *if both calls enable features*, but in the case
> where we're doing a two-step feature detection handshake, the kernel
> explicitly expects 2 calls (one with no features set). So, correct this
> description.
>
> Then, some new error cases have been added to the kernel recently, and
> the man page wasn't updated to note these. So, add in descriptions of
> these new error cases.
>
> Signed-off-by: Axel Rasmussen <[email protected]>
> ---
> man2/ioctl_userfaultfd.2 | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> index 53b1f473f..1aa9654be 100644
> --- a/man2/ioctl_userfaultfd.2
> +++ b/man2/ioctl_userfaultfd.2
> @@ -280,17 +280,31 @@ refers to an address that is outside the calling process's
> accessible address space.
> .TP
> .B EINVAL
> -The userfaultfd has already been enabled by a previous
> -.B UFFDIO_API
> -operation.
> -.TP
> -.B EINVAL
> The API version requested in the
> .I api
> field is not supported by this kernel, or the
> .I features
> field passed to the kernel includes feature bits that are not supported
> by the current kernel version.
> +.TP
> +.B EPERM

This EPERM should probably be at the end. Unless you have a good reason
to break alphabetic order.

Thanks,
Alex

> +The
> +.B UFFD_FEATURE_EVENT_FORK
> +feature was enabled,
> +but the calling process doesn't have the
> +.B CAP_SYS_PTRACE
> +capability.
> +.TP
> +.B EINVAL
> +A previous
> +.B UFFDIO_API
> +call already enabled one or more features for this userfaultfd.
> +Calling
> +.B UFFDIO_API
> +twice,
> +the first time with no features set,
> +is explicitly allowed
> +as per the two-step feature detection handshake.
> .\" 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.
> .\"
> --
> 2.42.0.459.ge4e396fd5e-goog
>


Attachments:
(No filename) (2.13 kB)
signature.asc (849.00 B)
Download all attachments

2023-10-09 08:50:25

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH 07/10] ioctl_userfaultfd.2: correct and update UFFDIO_API ioctl error codes

On Tue, Sep 26, 2023 at 01:52:34AM +0200, Alejandro Colomar wrote:
> Hi Axel,
>
> On Tue, Sep 19, 2023 at 12:02:03PM -0700, Axel Rasmussen wrote:
> > First, it is not correct that repeated UFFDIO_API calls result in
> > EINVAL. This is true *if both calls enable features*, but in the case
> > where we're doing a two-step feature detection handshake, the kernel
> > explicitly expects 2 calls (one with no features set). So, correct this
> > description.
> >
> > Then, some new error cases have been added to the kernel recently, and
> > the man page wasn't updated to note these. So, add in descriptions of
> > these new error cases.
> >
> > Signed-off-by: Axel Rasmussen <[email protected]>
> > ---
> > man2/ioctl_userfaultfd.2 | 24 +++++++++++++++++++-----
> > 1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> > index 53b1f473f..1aa9654be 100644
> > --- a/man2/ioctl_userfaultfd.2
> > +++ b/man2/ioctl_userfaultfd.2
> > @@ -280,17 +280,31 @@ refers to an address that is outside the calling process's
> > accessible address space.
> > .TP
> > .B EINVAL
> > -The userfaultfd has already been enabled by a previous
> > -.B UFFDIO_API
> > -operation.
> > -.TP
> > -.B EINVAL
> > The API version requested in the
> > .I api
> > field is not supported by this kernel, or the
> > .I features
> > field passed to the kernel includes feature bits that are not supported
> > by the current kernel version.
> > +.TP
> > +.B EPERM
>
> This EPERM should probably be at the end. Unless you have a good reason
> to break alphabetic order.

I agree with Alex here, other than that feel free to add

Reviewed-by: Mike Rapoport (IBM) <[email protected]>


> Thanks,
> Alex
>
> > +The
> > +.B UFFD_FEATURE_EVENT_FORK
> > +feature was enabled,
> > +but the calling process doesn't have the
> > +.B CAP_SYS_PTRACE
> > +capability.
> > +.TP
> > +.B EINVAL
> > +A previous
> > +.B UFFDIO_API
> > +call already enabled one or more features for this userfaultfd.
> > +Calling
> > +.B UFFDIO_API
> > +twice,
> > +the first time with no features set,
> > +is explicitly allowed
> > +as per the two-step feature detection handshake.
> > .\" 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.
> > .\"
> > --
> > 2.42.0.459.ge4e396fd5e-goog
> >



--
Sincerely yours,
Mike.

2023-10-09 11:01:46

by Alejandro Colomar

[permalink] [raw]
Subject: Re: [PATCH 07/10] ioctl_userfaultfd.2: correct and update UFFDIO_API ioctl error codes

Hi Mike,

On Mon, Oct 09, 2023 at 11:49:11AM +0300, Mike Rapoport wrote:
> On Tue, Sep 26, 2023 at 01:52:34AM +0200, Alejandro Colomar wrote:
> > Hi Axel,
> >
> > On Tue, Sep 19, 2023 at 12:02:03PM -0700, Axel Rasmussen wrote:
> > > First, it is not correct that repeated UFFDIO_API calls result in
> > > EINVAL. This is true *if both calls enable features*, but in the case
> > > where we're doing a two-step feature detection handshake, the kernel
> > > explicitly expects 2 calls (one with no features set). So, correct this
> > > description.
> > >
> > > Then, some new error cases have been added to the kernel recently, and
> > > the man page wasn't updated to note these. So, add in descriptions of
> > > these new error cases.
> > >
> > > Signed-off-by: Axel Rasmussen <[email protected]>
> > > ---
> > > man2/ioctl_userfaultfd.2 | 24 +++++++++++++++++++-----
> > > 1 file changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2
> > > index 53b1f473f..1aa9654be 100644
> > > --- a/man2/ioctl_userfaultfd.2
> > > +++ b/man2/ioctl_userfaultfd.2
> > > @@ -280,17 +280,31 @@ refers to an address that is outside the calling process's
> > > accessible address space.
> > > .TP
> > > .B EINVAL
> > > -The userfaultfd has already been enabled by a previous
> > > -.B UFFDIO_API
> > > -operation.
> > > -.TP
> > > -.B EINVAL
> > > The API version requested in the
> > > .I api
> > > field is not supported by this kernel, or the
> > > .I features
> > > field passed to the kernel includes feature bits that are not supported
> > > by the current kernel version.
> > > +.TP
> > > +.B EPERM
> >
> > This EPERM should probably be at the end. Unless you have a good reason
> > to break alphabetic order.
>
> I agree with Alex here, other than that feel free to add
>
> Reviewed-by: Mike Rapoport (IBM) <[email protected]>

Thanks. Since v2 only reorders these, I've added your tag.

Cheers,
Alex

>
>
> > Thanks,
> > Alex
> >
> > > +The
> > > +.B UFFD_FEATURE_EVENT_FORK
> > > +feature was enabled,
> > > +but the calling process doesn't have the
> > > +.B CAP_SYS_PTRACE
> > > +capability.
> > > +.TP
> > > +.B EINVAL
> > > +A previous
> > > +.B UFFDIO_API
> > > +call already enabled one or more features for this userfaultfd.
> > > +Calling
> > > +.B UFFDIO_API
> > > +twice,
> > > +the first time with no features set,
> > > +is explicitly allowed
> > > +as per the two-step feature detection handshake.
> > > .\" 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.
> > > .\"
> > > --
> > > 2.42.0.459.ge4e396fd5e-goog
> > >
>
>
>
> --
> Sincerely yours,
> Mike.

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


Attachments:
(No filename) (2.83 kB)
signature.asc (849.00 B)
Download all attachments