Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753324AbdHJTnJ (ORCPT ); Thu, 10 Aug 2017 15:43:09 -0400 Received: from mail-it0-f44.google.com ([209.85.214.44]:36151 "EHLO mail-it0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752894AbdHJTnF (ORCPT ); Thu, 10 Aug 2017 15:43:05 -0400 MIME-Version: 1.0 In-Reply-To: <20170810184616.22726-1-avagin@openvz.org> References: <20170810184616.22726-1-avagin@openvz.org> From: Jann Horn Date: Thu, 10 Aug 2017 21:42:44 +0200 Message-ID: Subject: Re: [PATCH] [RFC] vm: add a syscall to map a process memory into a pipe To: Andrei Vagin Cc: Alexander Viro , linux-fsdevel@vger.kernel.org, kernel list , Linux API , linux-arch , criu@openvz.org, Arnd Bergmann , Pavel Emelyanov , Michael Kerrisk , Thomas Gleixner , Andrew Morton Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2602 Lines: 68 On Thu, Aug 10, 2017 at 8:46 PM, Andrei Vagin wrote: > 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. [...] > +/* > + * 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 > + }; [...] > + while (nr_pages) { [...] > + /* > + * 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". > +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) > +{ [...] > + ret = remote_single_vec_to_pipe( > + task, mm, &rvec[i], pipe, flags, &total); [...] > +} > + > +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) > +{ [...] > + unsigned int buf_flag = 0; [...] > + if (flags & SPLICE_F_GIFT) > + buf_flag = PIPE_BUF_FLAG_GIFT; [...] > + if (!ret) > + ret = remote_iovec_to_pipe(task, mm, iov, > + nr_segs, pipe, buf_flag); [...] > +}