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 BC5B3C4167B for ; Fri, 1 Dec 2023 20:50:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4CEE26B04F1; Fri, 1 Dec 2023 15:50:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 47F436B04F2; Fri, 1 Dec 2023 15:50:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 36DD46B04F3; Fri, 1 Dec 2023 15:50:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 27B256B04F1 for ; Fri, 1 Dec 2023 15:50:44 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id F4172A0694 for ; Fri, 1 Dec 2023 20:50:43 +0000 (UTC) X-FDA: 81519443166.23.AFDA14C Received: from mail.hallyn.com (mail.hallyn.com [178.63.66.53]) by imf10.hostedemail.com (Postfix) with ESMTP id 1644FC0014 for ; Fri, 1 Dec 2023 20:50:41 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf10.hostedemail.com: domain of serge@mail.hallyn.com designates 178.63.66.53 as permitted sender) smtp.mailfrom=serge@mail.hallyn.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701463842; 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; bh=n3kB6UzvCvikMr0jgzXNNKGiCWX971q5TltnBn5Mf/I=; b=lx3Uav4jJsoxX7SZpQA2HJCe+T91BWj+pXBFgUZKiKZgmQQN1rAgZVnB2OpoGA6chKAmOy aDyKdS3gT3VD20la2uPbiLY1T1rban+iGSxWeBJR04/NkfKsH6+qK4RHuFFsrGaUgXUDTw lJARgEPV4XBTOQ5z1w4M0htIJLhVLRI= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf10.hostedemail.com: domain of serge@mail.hallyn.com designates 178.63.66.53 as permitted sender) smtp.mailfrom=serge@mail.hallyn.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701463842; a=rsa-sha256; cv=none; b=07LM3CI7Cs1I2njVx/VGV4xQd0vod3+q1f+YXuXl/9a054i8WHKlSIGgg4l7O48wFY+QKH zLoZk+eVuPXDML2XNgXCnmjaFxqaWWKL+wVzM3inuJFbnzp4W2Hby5eBSxg01sYsq6kU75 nzeZwNxhWQ2T0q0TWWZw3wwDd0oDe7c= Received: by mail.hallyn.com (Postfix, from userid 1001) id F317B788; Fri, 1 Dec 2023 14:50:39 -0600 (CST) Date: Fri, 1 Dec 2023 14:50:39 -0600 From: "Serge E. Hallyn" To: Yafang Shao Cc: akpm@linux-foundation.org, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com, omosnace@redhat.com, mhocko@suse.com, ying.huang@intel.com, linux-mm@kvack.org, linux-security-module@vger.kernel.org, bpf@vger.kernel.org, ligang.bdlg@bytedance.com Subject: Re: [PATCH v3 3/7] mm, security: Fix missed security_task_movememory() Message-ID: <20231201205039.GB109168@mail.hallyn.com> References: <20231201094636.19770-1-laoar.shao@gmail.com> <20231201094636.19770-4-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231201094636.19770-4-laoar.shao@gmail.com> X-Rspamd-Queue-Id: 1644FC0014 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: jcu4cuen8ce8ijuon7arzaxji5thz9ak X-HE-Tag: 1701463841-812934 X-HE-Meta: U2FsdGVkX1+DBRiyhLjlf4GNGSCvL/NlB0zGx0sKump3NVmrmRCmO74v5kdj681lwa7q6R++vc6Km/yR5ieXcrFrJA5m6ntl2gzVaLJ9cZbQatrAyA6GYotsxWfc5kEtC8gGcIknU4IngkinoHMi8WsgtojRNJOT4TJpewQtKp/V+jb/3QUfwDxaGxWSH5tXZLieBNx+cqhdliIebTYibWGNrKTIxV/te4pMWfmjsvH5W+AlE3peoXyQ0nSZ+pjh/yIQPP5jijIuX6a1SIx12VNE0QXzPU63BF69l+Y+HHoglafml1e+jKcYT+gk+uKKjqizICfxiPVadRJcGt62LA/kaeazZmKdx6WnVl4QQrQKhHU24CpZKwQBC6lL93lUHQhMhcqk8XThB0erisoAhWDi45roiDPTXhm2Sz184otO/OxrOggvTHxrElINWZgFvbeXCW9hZYLGmcf7beCM9SWe+yANJ5Sj7J2l//qOnBCod83CvybvmSClx1C//yZm/2qVwwSv/zgsVwru36MUbd/Xq4SI69rLsgp9jjNzHLtWkeFW56/o5NKgEh+xmvP8BJ5NnKzi7m0ksrdUyxM9rsMMiotT73YunvP5g/apKUDBqA6LkpT2kBaU+a3F6Uu9CxegLNR3D9ZsKIqaXkY/BSWrBPh648nhOMjfxaNNgujUTg9QRoYNbSdUJz+fO/PELmPEiGboQhPDzA9PNDbRMwDcIkjVhihXE7G45DpfGUS/JoubKGDUe1HbjGt9i9VrZf6iMWcx18ideBjmv/gl9z6ZIlGD4IsmEw4HHBBVdmKXhSxHhKG3yxca6GGO65/Q3d+BO+mByYrmloWWLf6EUuk8/WplwHNwRvVRAZc11N/+hM79LeDVfac4/sipDeqFILp114mQw/PUJ/Krx4C1eKrgjYgcxULVY6j+Q8vd4jr+2b96tsNwHKqX5nV7CUfsK81FiStLPHt6IWVJmCf 29Osw4lX h94KmQ1eSXvkwZgNgouuLt8ajpQQTLcmwt7OXdXG5rfL43iFy/52iGTQS1gJsmCH9/LyQfORS1ocFYztyhsI75YpEv7vl0LBEfIF3ednHhRFlWThDjvCroRCam6x9RWppQRuSw8Q8gy3JaoowlFgiQA9YCR2pT2Yt+L1vNmTjAxirsNm2SK/t6sHilvIiHFnH4TMCMvRvCEFrmWfT6tKJpeu7T9qDk6ZHa0Ns1J6kIRl0QrPF8IRAyg5ZcUsuX7Jq7NZHu1x4UQ8NUlJyR2+bXoFGyJYA6qKeHsAZi6bNpQr7huJqgjwWG36pKQ== 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: List-Subscribe: List-Unsubscribe: On Fri, Dec 01, 2023 at 09:46:32AM +0000, Yafang Shao wrote: > Considering that MPOL_F_NUMA_BALANCING or mbind(2) using either > MPOL_MF_MOVE or MPOL_MF_MOVE_ALL are capable of memory movement, it's > essential to include security_task_movememory() to cover this > functionality as well. It was identified during a code review. Hm - this doesn't have any bad side effects for you when using selinux? The selinux_task_movememory() hook checks for PROCESS__SETSCHED privs. The two existing security_task_movememory() calls are in cases where we expect the caller to be affecting another task identified by pid, so that makes sense. Is an MPOL_MV_MOVE to move your own pages actually analogous to that? Much like the concern you mentioned in your intro about requiring CAP_SYS_NICE and thereby expanding its use, it seems that here you will be regressing some mbind users unless the granting of PROCESS__SETSCHED is widened. > Signed-off-by: Yafang Shao > --- > mm/mempolicy.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 10a590ee1c89..1eafe81d782e 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1259,8 +1259,15 @@ static long do_mbind(unsigned long start, unsigned long len, > if (!new) > flags |= MPOL_MF_DISCONTIG_OK; > > - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) { MPOL_MF_MOVE_ALL already has a CAP_SYS_NICE check. Does that suffice for that one? > + err = security_task_movememory(current); > + if (err) { > + mpol_put(new); > + return err; > + } > lru_cache_disable(); > + } > + > { > NODEMASK_SCRATCH(scratch); > if (scratch) { > @@ -1450,6 +1457,8 @@ static int copy_nodes_to_user(unsigned long __user *mask, unsigned long maxnode, > /* Basic parameter sanity check used by both mbind() and set_mempolicy() */ > static inline int sanitize_mpol_flags(int *mode, unsigned short *flags) > { > + int err; > + > *flags = *mode & MPOL_MODE_FLAGS; > *mode &= ~MPOL_MODE_FLAGS; > > @@ -1460,6 +1469,9 @@ static inline int sanitize_mpol_flags(int *mode, unsigned short *flags) > if (*flags & MPOL_F_NUMA_BALANCING) { > if (*mode != MPOL_BIND) > return -EINVAL; > + err = security_task_movememory(current); > + if (err) > + return err; > *flags |= (MPOL_F_MOF | MPOL_F_MORON); > } > return 0; > -- > 2.30.1 (Apple Git-130)