On Thu, Aug 10, 2017 at 09:42:44PM +0200, Jann Horn wrote:

On Thu, Aug 10, 2017 at 8:46 PM, Andrei Vagin [off-list ref] wrote:

quoted

It is a hybrid of process_vm_readv() and vmsplice().

vmsplice can map memory from a current address space into a pipe.
process_vm_readv can read memory of another process.

[...]

quoted

+/*
+ * Map pages from a specified task into a pipe
+ */
+static int remote_single_vec_to_pipe(struct task_struct *task,
+                       struct mm_struct *mm,
+                       const struct iovec *rvec,
+                       struct pipe_inode_info *pipe,
+                       unsigned int flags,
+                       size_t *total)
+{
+       struct pipe_buffer buf = {
+               .ops = &user_page_pipe_buf_ops,
+               .flags = flags
+       };

[...]

quoted

+       while (nr_pages) {

[...]

quoted

+               /*
+                * Get the pages we're interested in.  We must
+                * access remotely because task/mm might not
+                * current/current->mm
+                */
+               down_read(&mm->mmap_sem);
+               pages = get_user_pages_remote(task, mm, pa, pages, flags,
+                                             process_pages, NULL, &locked);

This fifth "flags" argument of get_user_pages_remote() should contain
GUP flags (FOLL_*), but it looks like you're actually passing in 0 or
PIPE_BUF_FLAG_GIFT, which will be interpreted as FOLL_GET?
(See the snippets quoted below.) This looks like a bug.

Maybe use a more meaningful variable name than "flags".

Good catch. I will fix and rename the variable. get_user_pages_remote
has to be called with zero flags here. Thank you.

quoted

+static ssize_t remote_iovec_to_pipe(struct task_struct *task,
+                       struct mm_struct *mm,
+                       const struct iovec *rvec,
+                       unsigned long riovcnt,
+                       struct pipe_inode_info *pipe,
+                       unsigned int flags)
+{

[...]

quoted

+               ret = remote_single_vec_to_pipe(
+                               task, mm, &rvec[i], pipe, flags, &total);

[...]

quoted

+}
+
+static long process_vmsplice_to_pipe(struct task_struct *task,
+                               struct mm_struct *mm, struct file *file,
+                               const struct iovec __user *uiov,
+                               unsigned long nr_segs, unsigned int flags)
+{

[...]

quoted

+       unsigned int buf_flag = 0;

[...]

quoted

+       if (flags & SPLICE_F_GIFT)
+               buf_flag = PIPE_BUF_FLAG_GIFT;

[...]

quoted

+       if (!ret)
+               ret = remote_iovec_to_pipe(task, mm, iov,
+                                               nr_segs, pipe, buf_flag);

[...]

quoted

+}