Re: [Qemu-devel] [PATCH 10/23] userfaultfd: add new syscall to provide memory externalization

[Dave Hansen]

On 05/14/2015 10:31 AM, Andrea Arcangeli wrote:
> +static int userfaultfd_wake_function(wait_queue_t *wq, unsigned mode,
> +                                  int wake_flags, void *key)
> +{
> +     struct userfaultfd_wake_range *range = key;
> +     int ret;
> +     struct userfaultfd_wait_queue *uwq;
> +     unsigned long start, len;
> +
> +     uwq = container_of(wq, struct userfaultfd_wait_queue, wq);
> +     ret = 0;
> +     /* don't wake the pending ones to avoid reads to block */
> +     if (uwq->pending && !ACCESS_ONCE(uwq->ctx->released))
> +             goto out;
> +     /* len == 0 means wake all */
> +     start = range->start;
> +     len = range->len;
> +     if (len && (start > uwq->address || start + len <= uwq->address))
> +             goto out;
> +     ret = wake_up_state(wq->private, mode);
> +     if (ret)
> +             /* wake only once, autoremove behavior */
> +             list_del_init(&wq->task_list);
> +out:
> +     return ret;
> +}
...
> +static __always_inline int validate_range(struct mm_struct *mm,
> +                                       __u64 start, __u64 len)
> +{
> +     __u64 task_size = mm->task_size;
> +
> +     if (start & ~PAGE_MASK)
> +             return -EINVAL;
> +     if (len & ~PAGE_MASK)
> +             return -EINVAL;
> +     if (!len)
> +             return -EINVAL;
> +     if (start < mmap_min_addr)
> +             return -EINVAL;
> +     if (start >= task_size)
> +             return -EINVAL;
> +     if (len > task_size - start)
> +             return -EINVAL;
> +     return 0;
> +}

Hey Andrea,

Down in userfaultfd_wake_function(), it looks like you intended for a
len=0 to mean "wake all".  But the validate_range() that we do from
userspace has a !len check in it, which keeps us from passing a len=0 in
from userspace.

Was that "wake all" for some internal use, or is the check too strict?

I was trying to use the wake ioctl after an madvise() (as opposed to
filling things in using a userfd copy).