From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D08E4C11F6B for ; Fri, 2 Jul 2021 11:51:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0E5061413 for ; Fri, 2 Jul 2021 11:51:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232033AbhGBLyK (ORCPT ); Fri, 2 Jul 2021 07:54:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34654 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231936AbhGBLyJ (ORCPT ); Fri, 2 Jul 2021 07:54:09 -0400 Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7C4EFC061764 for ; Fri, 2 Jul 2021 04:51:36 -0700 (PDT) Received: by mail-lf1-x135.google.com with SMTP id a15so17569392lfr.6 for ; Fri, 02 Jul 2021 04:51:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=itw4ZSFZPbB5jMtAcW94A5cIi28Zbh7A4MY1L25gGO0=; b=skWOQ+tjB/RGXCRgCh/oX+pSqn801BpaewaioJ94g7BDJUoKJyzmNIJxfF6R7k4OVV FkwksQciuoAEGOrZtrGFPyYLCXxLZXMJQsbTDphZ25i2JhWuw1YkS/xEcgUdDDbf9BEz j7+1ZY18y7jKX0p1sfgvxy86H6YhI4LvjICQmgerkbW0Z3rzuCrlxHcOSbVsgrfKEMkm twF/uAlLn/bNXF+depqyu2UMF8dk7nIYWit4IrjecjAjTjt4ERd3UudzQ1TbM8bnmPv4 qcu2ePgioE8YFrrKk17qWsiRJ2RNd9GS4629An1dwAydO/asFBTQAYsYJLt0DMpXfTAL jsNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=itw4ZSFZPbB5jMtAcW94A5cIi28Zbh7A4MY1L25gGO0=; b=fWhk2AHGG9wJrsGZ2LyieSj/HAuYlVShtynLVLWvR/Dr+PkpHWtoSfDx9zLcMt6Y0B x4aK0zGURPv9x3PQdVbuDtTX9wAIoWib0n+qIco+m3klEIpsnGHVTmXYu76vwjF6UsSL EGYTqKnutw5YDZyoLwk08Jy5am5puvRXZ/vo0LwXYBLcr8LXkKatopiFx5Mh1jVeD2P+ UNUrchChiXfeIY5v+H9Af+2k1XkuIiw5Rtx8AipXLtSxPx0FXGLHHFjURGuUxFvsW/yS YF+ZDvvQwlvtwvJIRV4qKzc5GxCHSL9kbHTF9fzQxoLApx0K2zkCbNMPJVJX1pWmjnj/ 5l4w== X-Gm-Message-State: AOAM531BI9DpxdfaQDPdKdgZPkmrXyyuR5mCXxLvEgj/Gpyy6pPW2suC tiNtqsKF6JntBRbcWcWNwRuqaUIU1N6xkJrCC+TBOg== X-Google-Smtp-Source: ABdhPJzv91oZ5IBQE4TVWTsbh26N4tL41MOZOBkez4YnKnXUpCp87craAVl4JAJ/NuKAFHVkgR9wdySOTWnDC2Tli5g= X-Received: by 2002:a19:f51a:: with SMTP id j26mr3666613lfb.390.1625226694605; Fri, 02 Jul 2021 04:51:34 -0700 (PDT) MIME-Version: 1.0 References: <20210414055217.543246-1-avagin@gmail.com> <20210414055217.543246-3-avagin@gmail.com> In-Reply-To: From: Jann Horn Date: Fri, 2 Jul 2021 13:51:08 +0200 Message-ID: Subject: Re: [PATCH 2/4] arch/x86: implement the process_vm_exec syscall To: Andrei Vagin Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, linux-um@lists.infradead.org, criu@openvz.org, avagin@google.com, Andrew Morton , Andy Lutomirski , Anton Ivanov , Christian Brauner , Dmitry Safonov <0x7f454c46@gmail.com>, Ingo Molnar , Jeff Dike , Mike Rapoport , Michael Kerrisk , Oleg Nesterov , Peter Zijlstra , Richard Weinberger , Thomas Gleixner , linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 2, 2021 at 8:25 AM Andrei Vagin wrote: > On Mon, Jun 28, 2021 at 06:13:29PM +0200, Jann Horn wrote: > > On Wed, Apr 14, 2021 at 7:59 AM Andrei Vagin wrote: > > > +static void swap_mm(struct mm_struct *prev_mm, struct mm_struct *target_mm) > > > +{ > > > + struct task_struct *tsk = current; > > > + struct mm_struct *active_mm; > > > + > > > + task_lock(tsk); > > > + /* Hold off tlb flush IPIs while switching mm's */ > > > + local_irq_disable(); > > > + > > > + sync_mm_rss(prev_mm); > > > + > > > + vmacache_flush(tsk); > > > + > > > + active_mm = tsk->active_mm; > > > + if (active_mm != target_mm) { > > > + mmgrab(target_mm); > > > + tsk->active_mm = target_mm; > > > + } > > > + tsk->mm = target_mm; > > > > I'm pretty sure you're not currently allowed to overwrite the ->mm > > pointer of a userspace thread. For example, zap_threads() assumes that > > all threads running under a process have the same ->mm. (And if you're > > fiddling with ->mm stuff, you should probably CC linux-mm@.) > > > > As far as I understand, only kthreads are allowed to do this (as > > implemented in kthread_use_mm()). > > kthread_use_mm() was renamed from use_mm in the v5.8 kernel. Before > that, it wasn't used for user processes in the kernel, but it was > exported for modules, and we used it without any visible problems. We > understood that there could be some issues like zap_threads and it was > one of reasons why we decided to introduce this system call. > > I understand that there are no places in the kernel where we change mm > of user threads back and forth, but are there any real concerns why we > should not do that? I agree that zap_threads should be fixed, but it > will the easy one. My point is that if you break a preexisting assumption like this, you'll have to go through the kernel and search for places that rely on this assumption, and fix them up, which may potentially require thinking about what kinds of semantics would actually be appropriate there. Like the MCE killing logic (collect_procs_anon() and such). And current_is_single_threaded(), in which the current patch probably leads to logic security bugs. And __uprobe_perf_filter(). Before my refactoring of the ELF coredump logic in kernel 5.10 (commit b2767d97f5ff75 and the ones before it), you'd have also probably created memory corruption bugs in races between elf_core_dump() and syscalls like mmap()/munmap(). (Note that this is not necessarily an exhaustive list.) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 677DFC11F69 for ; Fri, 2 Jul 2021 11:51:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 01AD861416 for ; Fri, 2 Jul 2021 11:51:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 01AD861416 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 86E616B0011; Fri, 2 Jul 2021 07:51:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 81F066B0036; Fri, 2 Jul 2021 07:51:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6BE746B005D; Fri, 2 Jul 2021 07:51:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0238.hostedemail.com [216.40.44.238]) by kanga.kvack.org (Postfix) with ESMTP id 493F76B0011 for ; Fri, 2 Jul 2021 07:51:37 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id E420B18404017 for ; Fri, 2 Jul 2021 11:51:36 +0000 (UTC) X-FDA: 78317482992.17.09D62D2 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf22.hostedemail.com (Postfix) with ESMTP id 862861982 for ; Fri, 2 Jul 2021 11:51:36 +0000 (UTC) Received: by mail-lf1-f53.google.com with SMTP id k10so17505800lfv.13 for ; Fri, 02 Jul 2021 04:51:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=itw4ZSFZPbB5jMtAcW94A5cIi28Zbh7A4MY1L25gGO0=; b=skWOQ+tjB/RGXCRgCh/oX+pSqn801BpaewaioJ94g7BDJUoKJyzmNIJxfF6R7k4OVV FkwksQciuoAEGOrZtrGFPyYLCXxLZXMJQsbTDphZ25i2JhWuw1YkS/xEcgUdDDbf9BEz j7+1ZY18y7jKX0p1sfgvxy86H6YhI4LvjICQmgerkbW0Z3rzuCrlxHcOSbVsgrfKEMkm twF/uAlLn/bNXF+depqyu2UMF8dk7nIYWit4IrjecjAjTjt4ERd3UudzQ1TbM8bnmPv4 qcu2ePgioE8YFrrKk17qWsiRJ2RNd9GS4629An1dwAydO/asFBTQAYsYJLt0DMpXfTAL jsNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=itw4ZSFZPbB5jMtAcW94A5cIi28Zbh7A4MY1L25gGO0=; b=QX+ZjHF5c4WMDSCc3xFc/yD3jmNr6nOLixr7iRGXWK8iY1PmyHqUvevddGPG2uC4sS KCbw5AAcR7Q0vuNBSxbWwRbY506IXppO5mixpsB+kgzq0GWjdhRX/fMSxn7nTyE/chgI R81/oaOh4Qi53t60L1pIrW3XybNEIXW5D2dmtTfKy/RiuM8YC55xUCY5opzFF04RUSwr nvkq8O5hYxRxnPkSzOJffJgMS7+lF8tkHnaOiCjOwHlGBTsAWtfu6rVuu/R+MU0Chjv0 /y9wduZlR60FL4Xi1RQUxyYVfQaQgF5ssk6HnAzgNpcIm2MiuJZng8gsE/9gBNNemsdA R4TA== X-Gm-Message-State: AOAM5304C4Hnmkx3ihdpP+tKW4gCI3hhFv4ih/FWJ6YvAmG65i4c4mfl zYtvn74mSS7LXgr1LaZ69zrv9BYPnm56TxbxUtiz+g== X-Google-Smtp-Source: ABdhPJzv91oZ5IBQE4TVWTsbh26N4tL41MOZOBkez4YnKnXUpCp87craAVl4JAJ/NuKAFHVkgR9wdySOTWnDC2Tli5g= X-Received: by 2002:a19:f51a:: with SMTP id j26mr3666613lfb.390.1625226694605; Fri, 02 Jul 2021 04:51:34 -0700 (PDT) MIME-Version: 1.0 References: <20210414055217.543246-1-avagin@gmail.com> <20210414055217.543246-3-avagin@gmail.com> In-Reply-To: From: Jann Horn Date: Fri, 2 Jul 2021 13:51:08 +0200 Message-ID: Subject: Re: [PATCH 2/4] arch/x86: implement the process_vm_exec syscall To: Andrei Vagin Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, linux-um@lists.infradead.org, criu@openvz.org, avagin@google.com, Andrew Morton , Andy Lutomirski , Anton Ivanov , Christian Brauner , Dmitry Safonov <0x7f454c46@gmail.com>, Ingo Molnar , Jeff Dike , Mike Rapoport , Michael Kerrisk , Oleg Nesterov , Peter Zijlstra , Richard Weinberger , Thomas Gleixner , linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=skWOQ+tj; spf=pass (imf22.hostedemail.com: domain of jannh@google.com designates 209.85.167.53 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com X-Stat-Signature: pb5tubtx9on9jatxb3fosy6twe13aytr X-Rspamd-Queue-Id: 862861982 X-Rspamd-Server: rspam06 X-HE-Tag: 1625226696-853442 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Jul 2, 2021 at 8:25 AM Andrei Vagin wrote: > On Mon, Jun 28, 2021 at 06:13:29PM +0200, Jann Horn wrote: > > On Wed, Apr 14, 2021 at 7:59 AM Andrei Vagin wrote: > > > +static void swap_mm(struct mm_struct *prev_mm, struct mm_struct *target_mm) > > > +{ > > > + struct task_struct *tsk = current; > > > + struct mm_struct *active_mm; > > > + > > > + task_lock(tsk); > > > + /* Hold off tlb flush IPIs while switching mm's */ > > > + local_irq_disable(); > > > + > > > + sync_mm_rss(prev_mm); > > > + > > > + vmacache_flush(tsk); > > > + > > > + active_mm = tsk->active_mm; > > > + if (active_mm != target_mm) { > > > + mmgrab(target_mm); > > > + tsk->active_mm = target_mm; > > > + } > > > + tsk->mm = target_mm; > > > > I'm pretty sure you're not currently allowed to overwrite the ->mm > > pointer of a userspace thread. For example, zap_threads() assumes that > > all threads running under a process have the same ->mm. (And if you're > > fiddling with ->mm stuff, you should probably CC linux-mm@.) > > > > As far as I understand, only kthreads are allowed to do this (as > > implemented in kthread_use_mm()). > > kthread_use_mm() was renamed from use_mm in the v5.8 kernel. Before > that, it wasn't used for user processes in the kernel, but it was > exported for modules, and we used it without any visible problems. We > understood that there could be some issues like zap_threads and it was > one of reasons why we decided to introduce this system call. > > I understand that there are no places in the kernel where we change mm > of user threads back and forth, but are there any real concerns why we > should not do that? I agree that zap_threads should be fixed, but it > will the easy one. My point is that if you break a preexisting assumption like this, you'll have to go through the kernel and search for places that rely on this assumption, and fix them up, which may potentially require thinking about what kinds of semantics would actually be appropriate there. Like the MCE killing logic (collect_procs_anon() and such). And current_is_single_threaded(), in which the current patch probably leads to logic security bugs. And __uprobe_perf_filter(). Before my refactoring of the ELF coredump logic in kernel 5.10 (commit b2767d97f5ff75 and the ones before it), you'd have also probably created memory corruption bugs in races between elf_core_dump() and syscalls like mmap()/munmap(). (Note that this is not necessarily an exhaustive list.) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x130.google.com ([2a00:1450:4864:20::130]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lzHhq-002wkO-DY for linux-um@lists.infradead.org; Fri, 02 Jul 2021 11:51:39 +0000 Received: by mail-lf1-x130.google.com with SMTP id bq39so4977162lfb.12 for ; Fri, 02 Jul 2021 04:51:36 -0700 (PDT) MIME-Version: 1.0 References: <20210414055217.543246-1-avagin@gmail.com> <20210414055217.543246-3-avagin@gmail.com> In-Reply-To: From: Jann Horn Date: Fri, 2 Jul 2021 13:51:08 +0200 Message-ID: Subject: Re: [PATCH 2/4] arch/x86: implement the process_vm_exec syscall List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Andrei Vagin Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, linux-um@lists.infradead.org, criu@openvz.org, avagin@google.com, Andrew Morton , Andy Lutomirski , Anton Ivanov , Christian Brauner , Dmitry Safonov <0x7f454c46@gmail.com>, Ingo Molnar , Jeff Dike , Mike Rapoport , Michael Kerrisk , Oleg Nesterov , Peter Zijlstra , Richard Weinberger , Thomas Gleixner , linux-mm@kvack.org On Fri, Jul 2, 2021 at 8:25 AM Andrei Vagin wrote: > On Mon, Jun 28, 2021 at 06:13:29PM +0200, Jann Horn wrote: > > On Wed, Apr 14, 2021 at 7:59 AM Andrei Vagin wrote: > > > +static void swap_mm(struct mm_struct *prev_mm, struct mm_struct *target_mm) > > > +{ > > > + struct task_struct *tsk = current; > > > + struct mm_struct *active_mm; > > > + > > > + task_lock(tsk); > > > + /* Hold off tlb flush IPIs while switching mm's */ > > > + local_irq_disable(); > > > + > > > + sync_mm_rss(prev_mm); > > > + > > > + vmacache_flush(tsk); > > > + > > > + active_mm = tsk->active_mm; > > > + if (active_mm != target_mm) { > > > + mmgrab(target_mm); > > > + tsk->active_mm = target_mm; > > > + } > > > + tsk->mm = target_mm; > > > > I'm pretty sure you're not currently allowed to overwrite the ->mm > > pointer of a userspace thread. For example, zap_threads() assumes that > > all threads running under a process have the same ->mm. (And if you're > > fiddling with ->mm stuff, you should probably CC linux-mm@.) > > > > As far as I understand, only kthreads are allowed to do this (as > > implemented in kthread_use_mm()). > > kthread_use_mm() was renamed from use_mm in the v5.8 kernel. Before > that, it wasn't used for user processes in the kernel, but it was > exported for modules, and we used it without any visible problems. We > understood that there could be some issues like zap_threads and it was > one of reasons why we decided to introduce this system call. > > I understand that there are no places in the kernel where we change mm > of user threads back and forth, but are there any real concerns why we > should not do that? I agree that zap_threads should be fixed, but it > will the easy one. My point is that if you break a preexisting assumption like this, you'll have to go through the kernel and search for places that rely on this assumption, and fix them up, which may potentially require thinking about what kinds of semantics would actually be appropriate there. Like the MCE killing logic (collect_procs_anon() and such). And current_is_single_threaded(), in which the current patch probably leads to logic security bugs. And __uprobe_perf_filter(). Before my refactoring of the ELF coredump logic in kernel 5.10 (commit b2767d97f5ff75 and the ones before it), you'd have also probably created memory corruption bugs in races between elf_core_dump() and syscalls like mmap()/munmap(). (Note that this is not necessarily an exhaustive list.) _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um