From patchwork Wed Dec 15 06:23:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keno Fischer X-Patchwork-Id: 12677519 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC7E2C433EF for ; Wed, 15 Dec 2021 06:24:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2589A6B0071; Wed, 15 Dec 2021 01:23:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 209386B0073; Wed, 15 Dec 2021 01:23:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0D0AC6B0074; Wed, 15 Dec 2021 01:23:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0099.hostedemail.com [216.40.44.99]) by kanga.kvack.org (Postfix) with ESMTP id F18476B0071 for ; Wed, 15 Dec 2021 01:23:56 -0500 (EST) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id A6668181AC9C6 for ; Wed, 15 Dec 2021 06:23:46 +0000 (UTC) X-FDA: 78919037652.07.F53BCF8 Received: from mail-qv1-f51.google.com (mail-qv1-f51.google.com [209.85.219.51]) by imf20.hostedemail.com (Postfix) with ESMTP id 2F4841C0016 for ; Wed, 15 Dec 2021 06:23:43 +0000 (UTC) Received: by mail-qv1-f51.google.com with SMTP id a24so19466884qvb.5 for ; Tue, 14 Dec 2021 22:23:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=juliacomputing.com; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition; bh=Rc5xtFp4u75d66LLYKtxhf7lmOqThaEzRIkdwyUgUEc=; b=H7f/qfVfKBmo+iFdBKfYICARTkjIneUXudsZaaPgNIY1nXeISYJ9ZgzT8vROoaR/j6 TbeX1N45CrKCQpSJnW4myRzFVD1laJa1Cs1eVEQlhd9ilSHUn8/KXrW7O00qV1fDXJhR fsUQtqffQZ5TJ7OWAwaKVGaOFPk6HIxAZwPNBZn4cDd+7jOggIa5hcYiONsolLEUT93b UT1IXhB/gN+VWDsHCD91PHc+op6SChCDrGehX4QoTf+hpwaVAz5tOJlLTxkApvzkoOYW ht8HAvDVw0/0q1jwqNjOqJec7H1KHId3Lj/WSt4kpBXBSpG4oMyipfvSyLdgaIbW3vhq HQZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=Rc5xtFp4u75d66LLYKtxhf7lmOqThaEzRIkdwyUgUEc=; b=O35GvhA4TXYTVxlYuTKcrmxpQMfuznbo2hhvBjyThIWTZF0w/+Ch7X0v0X7qPqGsb7 k/exSWlKnrPKpctOv2CVxxkkRGI2cchFpWr/h+1dUL5p3nncQZygD6u2GvznLmGuRav8 ED4j0aGm9FoOUVdGkJV+I2/MfWS0dTiHCgQpho+Y4r+xLaetpIPYy805L6p85n5SQcVd ERLLp7dni5av4Z95cdYk0T3depWdT2jU+aYt7U2Ya7MvmsDoP7FZYR1mJ8b612yZuxzI RgVGuh4Mlv5uWbd6XdmcGHLMAIXUJo/gqtBEOeK76D7VLEcBbN6mbdmcUOcdzrhSeOy6 Dipw== X-Gm-Message-State: AOAM530JsmjfPKTMYKMeWX8sVk9l+jduJfvotRIxpDIT5wpvF5VG78l9 1MgY551oe5UHfI6qbBz3TxwmLw== X-Google-Smtp-Source: ABdhPJwrqhJXWl1YMtTct0xm5CppzumvwPiBGXjS6u4bkCZS76kogu7ZJdfk89rUy2GooCf6KVwrqQ== X-Received: by 2002:a05:6214:246d:: with SMTP id im13mr2449436qvb.44.1639549425089; Tue, 14 Dec 2021 22:23:45 -0800 (PST) Received: from juliacomputing.com ([2601:184:4780:3aef:8c6:cc82:9394:77a7]) by smtp.gmail.com with ESMTPSA id r8sm857396qtw.35.2021.12.14.22.23.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Dec 2021 22:23:44 -0800 (PST) Date: Wed, 15 Dec 2021 01:23:42 -0500 From: Keno Fischer To: linux-kernel@vger.kernel.org Cc: gorcunov@openvz.org, khlebnikov@openvz.org, oleg@redhat.com, akpm@linux-foundation.org, keescook@chromium.org, tj@kernel.org, dbueso@suse.de, matthltc@us.ibm.com, kosaki.motohiro@jp.fujitsu.com, xemul@parallels.com, linux-mm@kvack.org Subject: [PATCH] c/r: prctl: Remove PR_SET_MM_EXE_FILE old file mapping restriction Message-ID: <20211215062342.GA1548576@juliacomputing.com> MIME-Version: 1.0 Content-Disposition: inline Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=juliacomputing.com header.s=google header.b="H7f/qfVf"; dmarc=pass (policy=none) header.from=juliacomputing.com; spf=pass (imf20.hostedemail.com: domain of keno@juliacomputing.com designates 209.85.219.51 as permitted sender) smtp.mailfrom=keno@juliacomputing.com X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 2F4841C0016 X-Stat-Signature: 5apzg3kn4xyxbq6mkeo5y7ska16tbqm9 X-HE-Tag: 1639549423-402690 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: * What this patch does This patch changes the behavior of replace_mm_exe_file to remove the restriction that the current mm may not have any mappings of the previous exe_file. This restriction has a bit of a complicated history, which I will summarize below, but the upshot is that whatever value it may have had when originally introduced (and it is worth pointing out that the history does not suggest it was ever seen as a security feature) - in its current state, the restriction is essentially useless and merely forces userspace into awkward contusions (and extra system calls) to be able to use it. * Context/History The /proc//exe symlink provides access to the file that was used to execve . It is used for example by gdb to find the on-disk location of the executed binary and read its debug information. Originally, the /proc//exe symlink was immutable, set by the kernel upon execve and never changed again. However, in b32dfe377 ("c/r: prctl: add ability to set new mm_struct::exe_file"), `prctl` gained the ability to modify this symlink for use by c/r, under a couple of restrictions: 1. The process contains no mappings marked as VM_EXECUTABLE (i.e. mappings created in execve or by splitting mappings thereof). 2. The new file has appropriate access permissions 3. The call may only be made once 4. The calling process has CAP_SYS_RESOURCE The restriction we're considering here is point 1. For completeness, I will note that restriction 3 was subsequently dropped, and restriction 4 was expanded to also allow the local user namespace's root to perform the operation (as long as this was done using `PR_SET_MM_MAP`). On restriction 1, the original commit notes that: Note it allows to change /proc/$pid/exe if there are no VM_EXECUTABLE vmas present for current process, simply because this feature is a special to C/R and mm::num_exe_file_vmas become meaningless after that. The `num_exe_file_vmas` counter was a refcount for the number of mapped VMAs with the VM_EXECUTABLE flag set. It was used to drop the reference of /proc//exe to the execve'd file if all mappings to it created in sys_execve were subsequently removed. Thus, as best I can tell, this restriction was simply a convenience to avoid the additional complexity of correctly handling non-zero `num_exe_file_vmas` while updating the exe_file. However, `num_exe_file_vmas` was removed a few months later in e9714acf8c ("mm: kill vma flag VM_EXECUTABLE and mm->num_exe_file_vmas") with the justification that nobody depended on it and the functionality could be replaced by an appropriate use of PR_SET_MM_EXE_FILE. Because of this change, the restriction was updated in bafb282d ("c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal") to not allow any mappings present in the memory map of the process other than the *new* exe_file (a more strict restriction than the original restriction) and then again in 4229fb1dc ("c/r: prctl: less paranoid prctl_set_mm_exe_file()") to disallow any mappings of the old exe_file (still a more strict restriction than the original restriction on VM_EXECUTABLE mappings, as now any mapping with the same path would be forbidden not just those created in execve). It is worth noting that at this point the check for mappings of the original file and the modification to mm->exe_file were still protected by the mm's mmap_sem and thus atomic with respect to other modifications of the mm. However, this too was changed in 6e399cd14 ("prctl: avoid using mmap_sem for exe_file serialization") and the prctl now separately acquires the mm's read sema just for the purpose of enforcing the restriction (but does not enforce any sort of atomicity with respect to the update of the exe_file). Except for minor refactorings, this is essentially the state of the restriction in today's kernel. It appears to me that this was originally a technical restriction to avoid additional complexity from the interaction with VM_EXECTUABLE, but when this was removed the question of whether the restriction was still sensible was not revisited. I searched around for any additional justifications for this restriction, but could not find any, and given the lack of enforced atomicity, it does not seem that any guarantees are actually provided in practice. * The use case for dropping the restriction Apart from a general dislike for executing unnecessary code, there are some practical reasons to want to drop the restriction. In particular, it is currently awkward to call PR_SET_MM_MAP from an executable itself. In the original c/r usecase, the restorer was a ptracer and the original exe_file merely a stub that essentially did nothing, so it was no trouble to unmap it completely, However, there are a few usecases where PR_SET_MM_MAP would be useful that are not ptracers. One such use case are preloaders that run before ld.so and the main executable in order to control the memory layout. Wine has such a preloader, but they are also useful to control memory layout for debugging purposes. Another use case are non-ptrace checkpoint/restore systems (ptrace is powerful, but not particularly performant, so c/r systems that are ok with some state changing can gain performance by not using it). It is of course possible to use PR_SET_MM_MAP in these contexts by relocating the executable to private memory and unmapping the original, but this introduces additional unnecessary complexity for what appears to be no good reason. * Summary As far as I can tell, the restriction against mappings of the old exe_file in PR_SET_MM_EXE_FILE/PR_SET_MM_MAP exists for no good reason, but is simply an artifact of its development process. Because it makes it hard to use this APIs in legitimate contexts I propose that the restriction be dropped. Signed-off-by: Keno Fischer --- kernel/fork.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/kernel/fork.c b/kernel/fork.c index 3244cc56b697..11e01dae8bbc 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1203,27 +1203,9 @@ int set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file) */ int replace_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file) { - struct vm_area_struct *vma; struct file *old_exe_file; int ret = 0; - /* Forbid mm->exe_file change if old file still mapped. */ - old_exe_file = get_mm_exe_file(mm); - if (old_exe_file) { - mmap_read_lock(mm); - for (vma = mm->mmap; vma && !ret; vma = vma->vm_next) { - if (!vma->vm_file) - continue; - if (path_equal(&vma->vm_file->f_path, - &old_exe_file->f_path)) - ret = -EBUSY; - } - mmap_read_unlock(mm); - fput(old_exe_file); - if (ret) - return ret; - } - /* set the new file, lockless */ ret = deny_write_access(new_exe_file); if (ret)