linux-kernel - Re: userfaultfd: two-step UFFDIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJHvVcgLPhAS583Hp_To1McpvZL2U9cXt+=LKRNcikat3JgMAQ@mail.gmail.com>
Date: Mon, 25 Nov 2024 14:42:31 -0800
From: Axel Rasmussen <axelrasmussen@...gle.com>
To: Peter Xu <peterx@...hat.com>
Cc: stsp <stsp2@...dex.ru>, Linux kernel <linux-kernel@...r.kernel.org>, 
	Muhammad Usama Anjum <usama.anjum@...labora.com>, Mike Rapoport <rppt@...nel.org>
Subject: Re: userfaultfd: two-step UFFDIO_API always gives -EINVAL

On Mon, Nov 25, 2024 at 9:13 AM Peter Xu <peterx@...hat.com> wrote:
>
> On Mon, Nov 25, 2024 at 08:07:34PM +0300, stsp wrote:
> > 25.11.2024 19:58, Peter Xu пишет:
> > > On Mon, Nov 25, 2024 at 07:15:10PM +0300, stsp wrote:
> > > > Man page clearly talks about
> > > > "the userfaultfd object" (one object)
> > > > when mandating the "two-step handshake".
> > > > I spent hours of head-scratching
> > > > before went looking into the sources,
> > > > and even then I was confident the man
> > > > page is right: people should always assume
> > > > documentation is correct, code is buggy.
> > > Hmm yes.  I didn't pay much attention initially, but then after I read the
> > > latest man-pages/, especially "UFFDIO_API(2const)" I found it looks indeed
> > > wrong in the doc.
> > >
> > > In this case we can't change the code because we need to keep it working
> > > like before to not break ABI.  We may still update the doc.
> > I wonder if some non-ABI-breaker
> > is possible, like eg keep the current
> > behavior of "features=0", but allow
> > to (optionally) override that by a
> > non-0 request? Yes, I've seen kselftests
> > are trying to double-register after 0,
> > but IIRC they tried to register wrong
> > options, which would fail anyway.
>
> Old kernels will fail with -EINVAL, new will succeed.  It's already an ABI
> violation, IMHO.
>
> Not to mention I'm not sure what could happen if uffd feature flags can
> change on the fly.  Your proposal means it can happen when anon missing
> trap is enabled at least.  That's probably unwanted, and unnecessary
> complexity to maintain to the kernel.
>
> Thanks,
>
> --
> Peter Xu

I agree with Peter, we should just update the man page to mention
UFFDIO_API can only be called once, so probing requires a second
userfaultfd. I think not mentioning that was just an oversight from
the last time I updated the man page.

For what it's worth, I still don't like the two-step handshake design,
my preference is still an API like this:

1. userspace asks for the features it wants
2. kernel responds with the (possibly subset of) features it actually supports
3. userspace is free to carry on with perhaps limited features, or
exit with error, or ...

But, I think at that point the ship has already sailed. I think to
maintain compatibility with existing programs there isn't much we can
do at this point. Also from the last time we discussed this I think I
am somewhat alone preferring that design. :)

Also I agree that allowing > 1 UFFDIO_API calls is potentially tricky.
Calling it with features = 0 isn't just for probing, it also gives you
a fully initialized userfaultfd which could be used out of the box (if
you don't need any additional features).

If nobody else is interested in doing it I can send a patch to fixup
the man page at some point in the near future.

>