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 CA667C3064D for ; Wed, 26 Jun 2024 18:45:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1B0006B0082; Wed, 26 Jun 2024 14:45:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 15FD16B0083; Wed, 26 Jun 2024 14:45:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 000816B0085; Wed, 26 Jun 2024 14:45:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D55006B0082 for ; Wed, 26 Jun 2024 14:45:56 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 83C071A1678 for ; Wed, 26 Jun 2024 18:45:56 +0000 (UTC) X-FDA: 82273919112.20.66E1CE0 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf01.hostedemail.com (Postfix) with ESMTP id D766740018 for ; Wed, 26 Jun 2024 18:45:54 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Z3NS5yXW; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf01.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=kees@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719427541; a=rsa-sha256; cv=none; b=v15t5i5/Re/wVeYfLAXSb1rAuZPyA0uc1Z8OE/RlthHoga7T6eEDMJdJz66q8GqmxpmX/c aGuwfwJ6COz9YOscZZ0CWyPPDWOCD5vvmGz4h+FmOCgPG0NyZwJQ66/esPOxfZyU5BowaC 7cT1VYlwqvsb2UFT2+/uBIUPVHEUUfQ= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Z3NS5yXW; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf01.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=kees@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719427541; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=LdBm/ksEhVWTMk6ASx3r6t8OonmN5ZEfz5aP5/f9IMQ=; b=biPSn1HU6L7NriOWa5JpLynN/drrNPp/8g1FKdh5tEGqIuKhFzXzLHxZor8Au7oGtz1Zau bBGzFF6Xqai2pYaeVAQLGKiq192yCMV1KIRiwLmzFeDBPIC33AxXf2RrhbyCATDGD7LG+W LLWMLH0kGHvDBv1jT56VmQ+wLmdGLaw= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id AB19F61B98; Wed, 26 Jun 2024 18:45:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 58E6CC116B1; Wed, 26 Jun 2024 18:45:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1719427553; bh=HwdKQbr1kQD3+fSN1zZl35Es6O6pU661Vi1nR9Yom9I=; h=Date:From:To:Subject:References:In-Reply-To:From; b=Z3NS5yXWgoOrNLD+AMQ6HPZZzXrZ7MolKgybpGgrPnUM5CJcTSIyqmJTETh1Y61ek lBHzsHAUuCnR2dvFf1gJtOxlD/RK2wyXqMPoWA/tby1TjrUMG0CfQP5x86nJusJJpM E+xYlLbLv57KXzWp984vXACfxRjoTrqcbB0Up+7RKuJiPtrLVUAmLKFlNuvOitbr8m KhJHUghtOSDoZOVbGoFbCsKl1gThh//fVOvy4lvZGiZdjpAND+IQi2MnzWYzAINLO+ FD1xHD/SzylIL43qwFTrxapV9jgoCVmu7w1/WOfJTCewuPU7YwwF4rthXaz1ZVxYAk VdcwnSIvNh7Bw== Date: Wed, 26 Jun 2024 11:45:52 -0700 From: Kees Cook To: "Liam R. Howlett" , linux-mm@kvack.org, Andrew Morton , Suren Baghdasaryan , Vlastimil Babka , Lorenzo Stoakes , Matthew Wilcox , sidhartha.kumar@oracle.com, "Paul E . McKenney" , Bert Karwatzki , Jiri Olsa , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [PATCH v2 14/15] mm/mmap: Use vms accounted pages in mmap_region() Message-ID: <202406261145.F1D7708@keescook> References: <20240625191145.3382793-1-Liam.Howlett@oracle.com> <20240625191145.3382793-15-Liam.Howlett@oracle.com> <202406260928.0A22BB0F0@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: D766740018 X-Stat-Signature: w98uwo9bqi473saj1dp8iyeanod3t475 X-Rspam-User: X-HE-Tag: 1719427554-775588 X-HE-Meta: U2FsdGVkX19ZnGtEuH6kRG6yVHD/gWBOSQp3RUpvYiPUtCITdB82y8SVnWht7l5+FJPpnZKoPMw+OcgMGaNUgCIEBZG9lA7DyUIMdK9bietoaNMAunxuB64rHPK1J5cH+EBmQ7tjQspk72NC6seeEg3NNeHkpVdzo8u2I7xRuXLUV8Myb9ZL7bDRcSWytKUxzswD6BywMDoVmszMGW/wf7mA2IrAqlyWdUoLQIwc1b3IHSH8iVkgZf2psujMnGKs77UzywhsgRUSd3A2Xd31HI/uRChaMF2XvI0eusrNeXXYN8whMNJqxqQSFCN/YeJaci9bXelWZ8tJNFp1bjm4mMWSWVmQ92TNlxkBHUFLcw3Q2Y6nQCxg9BydzLsK8A4ozSR4wmw+SgyhYS2vS7iZuYKsOXBrC5lCEwgsbjrcemPTO1XzoVOrOUt6H95jm8hJHtGA5l2McqU2mnKQXVoOTIcVmGNNmUEhpN7U2PIMAy+YUXomFudk4nJyTXEzGrxKt8o4YfD5ZcZIec/y7tK7lwVOXva4JOGeQa/t/plpkdCCsLwmrKZ8Piacviq4ff1UpB2Zj7prQ7APmSkIkOlgI61vU4s5mnnrgYm/yOPnzvqGUP67unbMTbnmoH9gc5zEb6WdMr4EViauA0jbIxrh5DPbh7yx16FaZmq64JO3JEqMwWcTwsqRr6m5RsjOvbvAmzi2I3gvEDxP6DMpf6Oon9u/py8qpoZiKpxUVgdNWoZVPc4r+IDLXSS/gtTcdByrfR3lfWhGUsT/K1wfSh8V9kEW8tDbR7ZqBANhE91sNgL5RD3AY1AqrqbnYKXg2IcYoQrOoUhrRjqEhvv+7Wic+RrgSQ4v8aIeaBhUw51RumnP7JhheR/pAbCr8MpRqkrK3jMLDmNcUpP55KEnYvf7RSlIDLMqy8FcKUADPNBCoiSo25x42zekhCVKzBtDyXuRzOZMW/Q5fjS7rbYq/Tt AgE73/ia N6hYtVHbvuuH3GO/ivf8Y3q4lXmA7Z2IKfsrut8T4vlg9dVeZg0QgK8NtA8l4RPriG3Vk3zuILzthUrHFOCUqBWypGmoQigDu09fJYsYFHvbzbIWA/g1kliU7yycUpPO8sMW7V/p2Z2bmV1qbYBk7nRE0oPeL/OvnqGgIMCaWdlLmZAsWAyeWQZ6e0lWeT+nrcBzVQeD+w0ak17j701GKMaTc1izNB3m2fE3gkDR/mN98Dv9JnbWrbezz2nkTti6K7ajwqNU5k2rCfKs/1ABmTpCib5SicqpfNyZolmde/niK+61TpgL6AkYre80y4tRagFPkSsAC3mWQsids4z3i/iWR2RYy8R2zxKM6 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 Wed, Jun 26, 2024 at 02:04:53PM -0400, Liam R. Howlett wrote: > * Kees Cook [240626 12:32]: > > On Tue, Jun 25, 2024 at 03:11:44PM -0400, Liam R. Howlett wrote: > > > From: "Liam R. Howlett" > > > > > > Change from nr_pages variable to vms.nr_accounted for the charged pages > > > calculation. This is necessary for a future patch. > > > > > > This also avoids checking security_vm_enough_memory_mm() if the amount > > > of memory won't change. > > > > Is there a reason for making this change? (I.e. why not leave off the > > "charged" test?) > > Before, the munmap() completed prior to mmap()'ing the MAP_FIXED vma. > If we don't remove the nr_accounted from the charged, we risk hitting > the maximum limit. > > > > > Looking at the callbacks in the LSM, only capabilities and SELinux are > > hooking this, and both are checking whether a process has elevated privs > > and are ignoring the "pages" argument entirely, so I'm not sure it's > > safe to change the logic for whether to make the call based on an unused > > argument (i.e. the LSM may want to _always_ know about this). On the > > other hand, it looks like it's purely an accounting issue, and if the > > page count didn't change, there's no reason to bother calling into all > > this to make no changes to the accounting. > > I didn't see any reason not to avoid the call, but your statement is > valid. I didn't see anything looking at the callbacks that would have > issue with skipping it - but I'd like to hear what LSM has to say. > > I don't have any objections to removing the extra check, if anyone > thinks it could be an issue. > > > > > I've added the LSM list to CC... > > Thank you, and thanks for looking at this. Sure! And barring any other feedback, I think this is safe, given the changes to the accounting logic: no change means nothing to check. Reviewed-by: Kees Cook -Kees > > > > > -Kees > > > > > > > > Signed-off-by: Liam R. Howlett > > > Cc: Kees Cook > > > --- > > > mm/mmap.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > > index f3edabf83975..adb0bb5ea344 100644 > > > --- a/mm/mmap.c > > > +++ b/mm/mmap.c > > > @@ -2970,6 +2970,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > > > } else { > > > /* Minimal setup of vms */ > > > vms.nr_pages = 0; > > > + vms.nr_accounted = 0; > > > next = vma_next(&vmi); > > > prev = vma_prev(&vmi); > > > if (prev) > > > @@ -2981,9 +2982,10 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > > > */ > > > if (accountable_mapping(file, vm_flags)) { > > > charged = pglen; > > > - charged -= nr_accounted; > > > - if (security_vm_enough_memory_mm(mm, charged)) > > > + charged -= vms.nr_accounted; > > > + if (charged && security_vm_enough_memory_mm(mm, charged)) > > > goto abort_munmap; > > > + > > > vms.nr_accounted = 0; > > > vm_flags |= VM_ACCOUNT; > > > } > > > -- > > > 2.43.0 > > > > > > > -- > > Kees Cook -- Kees Cook