linux-kernel - [PATCH for 6.1 regression] mm, mremap: fix mremap() expanding for vma's with vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230117101939.9753-1-vbabka@suse.cz>
Date:   Tue, 17 Jan 2023 11:19:39 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        regressions@...mhuis.info, regressions@...ts.linux.dev,
        Vlastimil Babka <vbabka@...e.cz>, Fabian Vogt <fvogt@...e.com>,
        Jakub Matěna <matenajakub@...il.com>,
        stable@...r.kernel.org
Subject: [PATCH for 6.1 regression] mm, mremap: fix mremap() expanding for vma's with vm_ops->close()

Fabian has reported another regression in 6.1 due to ca3d76b0aa80 ("mm:
add merging after mremap resize"). The problem is that vma_merge() can
fail when vma has a vm_ops->close() method, causing is_mergeable_vma()
test to be negative. This was happening for vma mapping a file from
fuse-overlayfs, which does have the method. But when we are simply
expanding the vma, we never remove it due to the "merge" with the added
area, so the test should not prevent the expansion.

As a quick fix, check for such vmas and expand them using vma_adjust()
directly as was done before commit ca3d76b0aa80. For a more robust long
term solution we should try to limit the check for vma_ops->close only
to cases that actually result in vma removal, so that no merge would be
prevented unnecessarily.

Reported-by: Fabian Vogt <fvogt@...e.com>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1206359#c35
Fixes: ca3d76b0aa80 ("mm: add merging after mremap resize")
Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
Cc: Jakub Matěna <matenajakub@...il.com>
Cc: <stable@...r.kernel.org>
Tested-by: Fabian Vogt <fvogt@...e.com>
---
Thorsten: this should be added to the previous regression which wasn't
fully fixed by the previous patch:
https://linux-regtracking.leemhuis.info/regzbot/regression/20221216163227.24648-1-vbabka@suse.cz/
 mm/mremap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index fe587c5d6591..1e234fd95547 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1032,11 +1032,22 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 			 * the already existing vma (expand operation itself) and possibly also
 			 * with the next vma if it becomes adjacent to the expanded vma and
 			 * otherwise compatible.
+			 *
+			 * However, vma_merge() can currently fail due to is_mergeable_vma()
+			 * check for vm_ops->close (see the comment there). Yet this should not
+			 * prevent vma expanding, so perform a simple expand for such vma.
+			 * Ideally the check for close op should be only done when a vma would
+			 * be actually removed due to a merge.
 			 */
-			vma = vma_merge(mm, vma, extension_start, extension_end,
+			if (!vma->vm_ops || !vma->vm_ops->close) {
+				vma = vma_merge(mm, vma, extension_start, extension_end,
 					vma->vm_flags, vma->anon_vma, vma->vm_file,
 					extension_pgoff, vma_policy(vma),
 					vma->vm_userfaultfd_ctx, anon_vma_name(vma));
+			} else if (vma_adjust(vma, vma->vm_start, addr + new_len,
+                                   vma->vm_pgoff, NULL)) {
+				vma = NULL;
+			}
 			if (!vma) {
 				vm_unacct_memory(pages);
 				ret = -ENOMEM;
-- 
2.38.1