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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 94E73C00140 for ; Tue, 2 Aug 2022 09:49:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3336C6B0071; Tue, 2 Aug 2022 05:49:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E23D6B0072; Tue, 2 Aug 2022 05:49:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1AA8C8E0001; Tue, 2 Aug 2022 05:49:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 08EE56B0071 for ; Tue, 2 Aug 2022 05:49:11 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id C064E80CE4 for ; Tue, 2 Aug 2022 09:49:10 +0000 (UTC) X-FDA: 79754179260.20.2C05B36 Received: from mail-oi1-f171.google.com (mail-oi1-f171.google.com [209.85.167.171]) by imf20.hostedemail.com (Postfix) with ESMTP id 4DDC01C011E for ; Tue, 2 Aug 2022 09:49:10 +0000 (UTC) Received: by mail-oi1-f171.google.com with SMTP id h125so15924661oif.8 for ; Tue, 02 Aug 2022 02:49:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Rd/MnnpubSXpiz2m3mizA1wFblV70cbg5nXLpi9fPUU=; b=LrhdbfLNPTzm+rQRir/KlWFTOP5w7dp2DeTugPU44Q7w9F+j2YfnKpe5cfHrw8ralE bKy5BBnIk/1WWdF3dgQ/lWnyvcFT5+dRorBjdP3noyEpgfJqKe3is9ZiFbMNycb6g6v4 0MRYwfSDsPLWug/dc57YhUVLFQrhQPoRYD1AivLL9QUs4VTOvsIGmHWbj9nPBlkXwueu xbaByWeHnH7xrusROFoxFuJRrdZmKdaGTgZXL5zmSyG2yv642dN+zwJTVYzJLHRddndm CxXNmEiUPl7zFls9hvBpssresB7hZSjkLKunn8joVhzt1eroMR4WxIf8rps0sGG1CyLi 54sw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Rd/MnnpubSXpiz2m3mizA1wFblV70cbg5nXLpi9fPUU=; b=pKm12EK7f9Mqh3skl5v/AY5ZjnGqOPysz7lpvSufPhwJNeP8LsZZBAZm4kY55yPPuh wh0zrYZ6KeabzTyAizTNFM+ZlAE3aS/35GDH2CVZthjIBWptvLDU0r+pUz7bpnxZKf/n fmx85SdvfUbSG2vdKMLHWdOkkbrKWZH4atyIFfSywoLx6pW4TDi51jqYJr/yQ/rv/DCd khzv2yVf1CKl6mbqfPsfw+2H0FTAD2tZapbR4Ro4UwGr7U64+OQv1Q5owZi5om6uoPgi ue/VZk3dAVhj6Ie67OPhpgA9IyvCBFIiVl2jx2RFJLpUFR0S9pC6OhZO8U/NBOUmnL4g +ztw== X-Gm-Message-State: AJIora86uKZ6ECzSgcdKWrQJCTpyD+a9/Y3ORPcKKzeTvFn/kmbSc8ZO yvp4+QxlCfNYurr0QaSut3+ip7OPvOzGZMKvsyAPTg== X-Google-Smtp-Source: AGRyM1vYxPJcIwe5cHDHLSAcsh31y1ZP8mbEtq/TG7pxpHeW2mhAu3KJQQ6G4OsCSeCIaJ4nIBs3Y1UcxGw0e5uU3tU= X-Received: by 2002:a05:6808:1446:b0:33a:c7e0:e88d with SMTP id x6-20020a056808144600b0033ac7e0e88dmr7690868oiv.123.1659433749236; Tue, 02 Aug 2022 02:49:09 -0700 (PDT) MIME-Version: 1.0 References: <20220801210946.3069083-1-zokeefe@google.com> In-Reply-To: From: "Zach O'Keefe" Date: Tue, 2 Aug 2022 02:48:33 -0700 Message-ID: Subject: Re: [PATCH mm-unstable] mm/madvise: remove CAP_SYS_ADMIN requirement for process_madvise(MADV_COLLAPSE) To: Michal Hocko Cc: linux-mm@kvack.org, Andrew Morton , linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, Axel Rasmussen , James Houghton , Hugh Dickins , Yang Shi , Miaohe Lin , David Hildenbrand , David Rientjes , Matthew Wilcox , Pasha Tatashin , Peter Xu , Rongwei Wang , SeongJae Park , Song Liu , Vlastimil Babka , Zi Yan , Andrea Arcangeli , Arnd Bergmann , Chris Kennelly , Chris Zankel , Helge Deller , Ivan Kokshaysky , "James E.J. Bottomley" , Jens Axboe , "Kirill A. Shutemov" , Matt Turner , Max Filippov , Minchan Kim , Patrick Xia , Pavel Begunkov , Thomas Bogendoerfer Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1659433750; a=rsa-sha256; cv=none; b=ZH2BiCbogW231f+4V0ah0/UoWYMAlj72mnIRazIumWcA4+pBwfN8RgpNGsWYcXCF+UQ+Md 7oNis8b5DaK2cocqt1wwb4IWhZEZ1ASapQJyM26Uy5uzT1hGfD68eQ56V+qLbntpcHXVA/ xhP80RzSCxhjAXVpRsRpdg8GPTKtRB8= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=LrhdbfLN; spf=pass (imf20.hostedemail.com: domain of zokeefe@google.com designates 209.85.167.171 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1659433750; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Rd/MnnpubSXpiz2m3mizA1wFblV70cbg5nXLpi9fPUU=; b=vVy8H1rxla6EHNia2NHMy5EAPbdt1V2kwgKe/5usGn5YoDOlZRHewliSmA6J/ai9ookDvN ZL6IAwNgeRsADkN66ehxM6iv3EdZUb5cWHuHXiiuLqsPYLe9mi1NYNDBFF4+VSGy5u+dQQ vx7Ah9o2cfrpzLw1P+J8nEOrxTAmQYA= X-Stat-Signature: 9kiuy36oc8hrr1eif33e6n51gp7y49gy X-Rspamd-Queue-Id: 4DDC01C011E X-Rspam-User: Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=LrhdbfLN; spf=pass (imf20.hostedemail.com: domain of zokeefe@google.com designates 209.85.167.171 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam03 X-HE-Tag: 1659433750-306074 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 Tue, Aug 2, 2022 at 2:09 AM Michal Hocko wrote: > > Hasn't this been discussed during the MADV_COLLAPSE submission? What has > changed? Does this need more time to settle with the consensus? > > On Mon 01-08-22 14:09:46, Zach O'Keefe wrote: > > process_madvise(MADV_COLLAPSE) currently requires CAP_SYS_ADMIN when not > > acting on the caller's own mm. This is maximally restrictive, and > > perpetuates existing issues with CAP_SYS_ADMIN. Remove this requirement. > > > > When acting on an external process' memory, the biggest concerns for > > process_madvise(MADV_COLLAPSE) are (1) being able to influence process > > performance by moving memory, possibly between nodes, that is mapped > > into the address space of external process(es), (2) defeat of > > address-space-layout randomization, and (3), being able to increase > > process RSS and memcg usage, possibly causing memcg OOM. > > > > process_madvise(2) already enforces CAP_SYS_NICE and PTRACE_MODE_READ (in > > PTRACE_MODE_FSCREDS mode). A process with these credentials can already > > accomplish (1) and (2) via move_pages(MPOL_MF_MOVE_ALL), and (3) via > > process_madvise(MADV_WILLNEED). > > > > process_madvise(MADV_COLLAPSE) may also circumvent sysfs THP settings. > > When acting on one's own memory (which is equivalent to > > madvise(MADV_COLLAPSE)), this is deemed acceptable, since aside from the > > possibility of hoarding available hugepages (which is currently already > > possible) no harm to the system can be done. When acting on an external > > process' memory, circumventing sysfs THP settings should provide no > > additional threat compared to the ones listed. As such, imposing > > additional capabilities (such as CAP_SETUID, as a way to ensure the > > caller could have just altered the sysfs THP settings themselves) > > provides no extra protection. > > > > Fixes: 7ec952341312 ("mm/madvise: add MADV_COLLAPSE to process_madvise()") > > Signed-off-by: Zach O'Keefe > > --- > > mm/madvise.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index f9e11b6c9916..af97100a0727 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -1170,16 +1170,14 @@ madvise_behavior_valid(int behavior) > > } > > } > > > > -static bool > > -process_madvise_behavior_valid(int behavior, struct task_struct *task) > > +static bool process_madvise_behavior_valid(int behavior) > > { > > switch (behavior) { > > case MADV_COLD: > > case MADV_PAGEOUT: > > case MADV_WILLNEED: > > - return true; > > case MADV_COLLAPSE: > > - return task == current || capable(CAP_SYS_ADMIN); > > + return true; > > default: > > return false; > > } > > @@ -1457,7 +1455,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec, > > goto free_iov; > > } > > > > - if (!process_madvise_behavior_valid(behavior, task)) { > > + if (!process_madvise_behavior_valid(behavior)) { > > ret = -EINVAL; > > goto release_task; > > } > > -- > > 2.37.1.455.g008518b4e5-goog > > -- > Michal Hocko > SUSE Labs Hey Michal, Thanks for taking the time to take a look at this. "mm/madvise: add MADV_COLLAPSE to process_madvise()" in the v7 series ended with me mentioning a couple options, but ultimately I didn't present a solution, and no consensus was reached[1]. After taking a closer look, this is my proposal for what I believe to be the best path forward. It should be squashed into the original patch. What do you think? Thanks again, Zach [1] https://lore.kernel.org/linux-mm/Ys4aTRqWIbjNs1mI@google.com/