Re: [PATCH 0/2] mm/userfaultfd: Fix uninitialized output field for -EAGAIN race

[David Hildenbrand]

On 25.04.25 17:45, James Houghton wrote:
> On Thu, Apr 24, 2025 at 5:57 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
> > When discussing some userfaultfd issues with Andrea, Andrea pointed out an
> > ABI issue with userfaultfd that existed for years.  Luckily the issue
> > should only be a very corner case one, and the fix (even if changing the
> > kernel ABI) should only be in the good way, IOW there should have no risk
> > breaking any userapp but only fixing.
>
> FWIW, my userspace basically looks like this:
>
> struct uffdio_continue uffdio_continue;
> int64_t target_len = /* whatever */;
> int64_t bytes_mapped = 0;
> int ioctl_ret;
> do {
>    uffdio_continue.range = /* whatever */;
>    uffdio_continue.mapped = 0;
>    ioctl_ret = ioctl(uffd, UFFDIO_CONTINUE, &uffdio_continue);
>    if (uffdio_continue.mapped < 0) { break; }
>    bytes_mapped += uffdio_continue.mapped;
> } while (bytes_mapped < target_len && errno == EAGAIN);
>
> I think your patch would indeed break this. (Perhaps I shouldn't be
> reading from `mapped` without first checking that errno == EAGAIN.)
>
> Well, that's what I would say, except in practice I never actually hit
> the mmap_changing case while invoking UFFDIO_CONTINUE. :)

Hm, but what if mfill_atomic_continue() would already return -EAGAIN 
when checking mmap_changing etc?

Wouldn't code already run into an issue there?

--
Cheers,

David / dhildenb