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 9D056C7EE22 for ; Thu, 11 May 2023 18:08:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E7B186B0072; Thu, 11 May 2023 14:08:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E2D4F6B0074; Thu, 11 May 2023 14:08:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF2E26B0075; Thu, 11 May 2023 14:08:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id BE88F6B0072 for ; Thu, 11 May 2023 14:08:47 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7A83140AA7 for ; Thu, 11 May 2023 18:08:47 +0000 (UTC) X-FDA: 80778759894.26.4F21D71 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf13.hostedemail.com (Postfix) with ESMTP id 8EC4420002 for ; Thu, 11 May 2023 18:08:44 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=C4U73fyS; spf=pass (imf13.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1683828524; 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=eNWOWMTtSya0T8qp8rVOMjrQplNvnwAPoLY4mrPXzN0=; b=USQ3P1o0FY1oAnZpZCSCy8aizTLzcj/408x/dDmdxCLV+4rQZzMkDPtIZ4Psa2X08YVIEA 8qRKf1tnwtiom1AGJp1gTOEaLC50IKAjSO57FUZcE3/Qm+fMvoBiEey/0t/+JfYTvcakZD 4LnXlRzV6nzng5dhojCh3EUGUlbJizA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1683828524; a=rsa-sha256; cv=none; b=UCGDsr0aPY+t0G+IgqmMHa0mnuYTsZ45M+DrNyEsPzcP/08Ea6puphinrvsaisxmRYH4OR VZ9875LLdNV46DiTVuX12BXKoBnkkwg4e3ti3tq0p3ORee1NjP+KFbhYLrYwj6vNmjzfgT gDrz6jYeoXr7oGEH4vrvPCGCJuJjxJo= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=C4U73fyS; spf=pass (imf13.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4D62D65089; Thu, 11 May 2023 18:08:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 76498C4339B; Thu, 11 May 2023 18:08:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1683828522; bh=a8GH8ffv6Io2e/3QEMAqEjxKKcU+R60IoxJKxKD750o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C4U73fySzrXnlvzjnI/BrXsv5SsdVqE5i7WZ1lYPG/0jlvKIENfxAVa5z+u+NE+no 1wyOUqO38EXmCK7yHRBGCiWTO4bPifTc3J6/KXWW5ZrgRZgvSG8HUMMmo2a72oYsBs uXaS4sW6Yfdvg3+qJTjFh/mXviCTMXmr2Px8HfjmNtHTLW2At1FOwLxQZ20VNmL3vM V6OpWU9T6f7LhITV3uECj07+H5guXFa6c6OFp25UzwokNEXQw7w8RilSnAVq2wxRH2 hVZxptbP3IBk73F3q+lIDIoQZk0aaEZ19w9r1QDf2NcxFkJL1ERGJK5k9MziiYMgWX qHiLmGyQsYVfQ== Date: Thu, 11 May 2023 11:08:41 -0700 From: Mike Rapoport To: Lorenzo Stoakes Cc: Mark Rutland , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , "Liam R . Howlett" , Vlastimil Babka , Peter Xu Subject: Re: [PATCH] mm/mmap/vma_merge: always check invariants Message-ID: <20230511180841.GE4135@kernel.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 8EC4420002 X-Stat-Signature: tjd13dhgderqg1k7mt7oczpe19a5ar7n X-Rspam-User: X-Rspamd-Server: rspam09 X-HE-Tag: 1683828524-462857 X-HE-Meta: U2FsdGVkX1+ODaE7N/1Nk5BRLVzM4ZPEGFNP6CJDLhvLcJz+zkO0DAY0dByXu16dYh7QB22kOFAMlUcMLF20fFroeBHZCppg3WKF59QkQeSQ1FQYcYXAYXmZxrZJACy0kVcXXwnzQXXay+SOvQkN/pV54J9TCY17Is6L4OfngeSWOzGj+UuNHiQshyUjT78mBaLG0Ea7jMSGH7kThaxk1K51NNVgs5gZ54H8eDvWWHNq/jtqyf8/KZQjfaZL9eSjtMkoMEk6tToKoXoPYo2x9h2ymQCAhgG6f2pU3hKDStV/4vXtvNR1L3BWR6klRe506Kogu63YrMXfdvrBP0bhn2TMxndsDSMUQIoAVoFVKzhmcpPjgdwNK7sJrIQQfj7ZDcE6bIwSyp77W6TQVu6eKRx5Lmbc0vOChOAAWq3fOebd9heEjDaX4pUicYsFNt3YKdxCOHG4m5EVYkR36A/Xw3mYiAwrgMgFq3a5xh+KK/KBIfpXtQa/9mIGZ+M6H0wpCKI22TOrECdIn7YWOEpInWC5l8sr7NI4+0zeVXyJtgkICcHawOTIRseYjy+yITJpExDzS1cAxNvh6wMfrk1NesozDQHGFwTr0O+EbcZxeM01I7TChuT+NqOPn7snrWC1QM2J7oqaN1cADAOrYwEdoNaOuEEYuRtU1hE+0HfpRoOOHGyKaO7henHCjK0VKHdWwx1JIGoM9kvhthPk9o4Um+8WOJyI6NlSm+wfYWiFIAC9kRU+sA61GSv+toCvalgIEjDs6Tnwdj8PhQ/Sv7L1A0dGNxlxdOiS+yVSNXBTBSmf2qI/hH6dmgHyX/B8k3ZHR9gz0VZ8QrdIKnBmN9xMpHA3cOjLQ9SObdYEvjXHm6rOQKToGuKjPM3XxhpAL7C2CBxPNTorNPX6Hikls75gNVKVPSCnxiUMqt03jvpEzc4ekTE7oQhRuxayDH0RHuJ3qU/RXzqFELsJ4v21zMu VwNTD4GC vf7RS0TgC/iVwk1X9I8fotg9okFTJ1JjAgQ7UUmmYfhMEtcSnPRtFn+aQg5fTIz1Brtm6jNPCAjDUjp7pJsIgWGGegAHQevI5rTyKXbi5pERwd0XCNi1aCUKIIue7PH1M8UuDaJGrL/rdWJ8d2Erl77j6iHR/9N/UfE3JujQXwP9uDbnXbRxpEGmaoBto6Z+v78bd7TYovMDj8G7xMbOUZcaSdmNAapbTBXloxupeiEe8l2sW0IeTZug7+xjxZ6oZgWn5djJ/S1ddMPAuAWb/DLxQs5iYTmZ7ToLXg3UiiPv9KeTPA7BNJm95KbvXQE9U2DUFeKmscelMgtbhUhFsALQUWOayglp5mBJJ4DtPu5qHh0YLzE1oBlD/0rGHMmrjXTIvV469EfPiivp735YGcwTYb8MNq6O0QA7Z 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: (adding Peter) On Wed, May 10, 2023 at 09:26:10AM -0700, Lorenzo Stoakes wrote: > On Wed, May 10, 2023 at 05:17:49PM +0100, Mark Rutland wrote: > > On Wed, May 10, 2023 at 09:04:44AM -0700, Lorenzo Stoakes wrote: > > > On Wed, May 10, 2023 at 03:15:51PM +0100, Mark Rutland wrote: > > > > Hi, > > > > > > > > On Sun, Apr 30, 2023 at 09:19:17PM +0100, Lorenzo Stoakes wrote: > > > > > We may still have inconsistent input parameters even if we choose not to > > > > > merge and the vma_merge() invariant checks are useful for checking this > > > > > with no production runtime cost (these are only relevant when > > > > > CONFIG_DEBUG_VM is specified). > > > > > > > > > > Therefore, perform these checks regardless of whether we merge. > > > > > > > > > > This is relevant, as a recent issue (addressed in commit "mm/mempolicy: > > > > > Correctly update prev when policy is equal on mbind") in the mbind logic > > > > > was only picked up in the 6.2.y stable branch where these assertions are > > > > > performed prior to determining mergeability. > > > > > > > > > > Had this remained the same in mainline this issue may have been picked up > > > > > faster, so moving forward let's always check them. > > > > > > > > > > Signed-off-by: Lorenzo Stoakes > > > > > --- > > > > > mm/mmap.c | 10 +++++----- > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > > > > index 5522130ae606..13678edaa22c 100644 > > > > > --- a/mm/mmap.c > > > > > +++ b/mm/mmap.c > > > > > @@ -960,17 +960,17 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > > > > merge_next = true; > > > > > } > > > > > > > > > > + /* Verify some invariant that must be enforced by the caller. */ > > > > > + VM_WARN_ON(prev && addr <= prev->vm_start); > > > > > + VM_WARN_ON(curr && (addr != curr->vm_start || end > curr->vm_end)); > > > > > + VM_WARN_ON(addr >= end); > > > > > + > > > > > > > > I'm seeing this fire a lot when fuzzing v6.4-rc1 on arm64 using Syzkaller. > > > > > > > > > > Thanks, from the line I suspect addr != curr->vm_start, but need to look > > > into the repro, at lsf/mm so a bit time lagged :) > > > > No problem; FWIW I can confirm your theory, the reproducer is causing: > > > > addr > curr->vm_start > > > > ... confirmed the the following hack, log below. > > Awesome thanks for that! Just been firing up qemu to do this. > > Cases 5-8 should really have addr == curr->vm_start, I wonder if it's > another case but curr is being set incorrectly, it should in theory not be > the case. AFAIU, it's a case of "adjust vma, but don't merge, because prev is not compatible". Looks like uffd first attempts to merge compatible the newly registered range with adjacent vmas relying on that there won't be no merge when addr != curr->vm_start and only after the merge attempt it splits the edges. I think that moving the split in fs/userfaultfd.c:1495 (as of v6.4-rc1) before vma_merge() will be the right fix. > (See [1] for a visualisation of merge cases as a handy reference) > > Of course userfaultfd might be the offender here and might be relying on no > merge case arising but passing dodgy parameters. > > [1]:https://ljs.io/merge_cases.png You really should put it into Documentation/mm ;-) > > > > | diff --git a/mm/mmap.c b/mm/mmap.c > > | index 13678edaa22c..2cdebba15719 100644 > > | --- a/mm/mmap.c > > | +++ b/mm/mmap.c > > | @@ -961,9 +961,21 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > | } > > | > > | /* Verify some invariant that must be enforced by the caller. */ > > | - VM_WARN_ON(prev && addr <= prev->vm_start); > > | - VM_WARN_ON(curr && (addr != curr->vm_start || end > curr->vm_end)); > > | - VM_WARN_ON(addr >= end); > > | + VM_WARN(prev && addr <= prev->vm_start, > > | + "addr = 0x%016lx, prev->vm_start = 0x%016lx\n", > > | + addr, prev->vm_start); > > | + > > | + VM_WARN(curr && addr != curr->vm_start, > > | + "addr = 0x%016lx, curr->vm_start = 0x%016lx\n", > > | + addr, curr->vm_start); > > | + > > | + VM_WARN(curr && addr > curr->vm_end, > > | + "addr = 0x%016lx, curr->vm_end = 0x%016lx\n", > > | + addr, curr->vm_end); > > | + > > | + VM_WARN(addr >= end, > > | + "addr = 0x%016lx, end = 0x%016lx\n", > > | + addr, end); > > | > > | if (!merge_prev && !merge_next) > > | return NULL; /* Not mergeable. */ > > > > ... with that applied, running the reproducer results in: > > > > | addr = 0x0000ffff99dc2000, curr->vm_start = 0x0000ffff99db2000 > > | WARNING: CPU: 0 PID: 163 at mm/mmap.c:968 vma_merge+0x3d4/0x1260 > > > > ... i.e. addr > curr->vm_start > > > > Thanks, > > Mark. >