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

Alejandro Colomar
2023-10-09T11:01:19Z

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

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 <axelrasmussen@google.com>
> > > ---
> > > 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) <rppt@kernel.org>

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/>

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