linux-kernel - Re: [PATCH] [RFC] vm: add a syscall to map a process memory into a pipe

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]

Date:   Thu, 10 Aug 2017 21:42:44 +0200
From:   Jann Horn <jannh@...gle.com>
To:     Andrei Vagin <avagin@...nvz.org>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org,
        kernel list <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>, criu@...nvz.org,
        Arnd Bergmann <arnd@...db.de>,
        Pavel Emelyanov <xemul@...tuozzo.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] [RFC] vm: add a syscall to map a process memory into a pipe

On Thu, Aug 10, 2017 at 8:46 PM, Andrei Vagin <avagin@...nvz.org> 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);
[...]
> +}