linux-kernel - [PATCH] mm/oom_kill: Ensure MMU notifier range_end() is paired with range

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]

Date:   Wed, 10 Mar 2021 13:31:17 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        David Rientjes <rientjes@...gle.com>,
        Ben Gardon <bgardon@...gle.com>,
        Jason Gunthorpe <jgg@...pe.ca>, Michal Hocko <mhocko@...e.com>,
        "Jérôme Glisse" <jglisse@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Dimitri Sivanich <dimitri.sivanich@....com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: [PATCH] mm/oom_kill: Ensure MMU notifier range_end() is paired with range_start()

Invoke the MMU notifier's .invalidate_range_end() callbacks even if one
of the .invalidate_range_start() callbacks failed.  If there are multiple
notifiers, the notifier that did not fail may have performed actions in
its ...start() that it expects to unwind via ...end().  Per the
mmu_notifier_ops documentation, ...start() and ...end() must be paired.

The only in-kernel usage that is fatally broken is the SGI UV GRU driver,
which effectively blocks and sleeps fault handlers during ...start(), and
unblocks/wakes the handlers during ...end().  But, the only users that
can fail ...start() are the i915 and Nouveau drivers, which are unlikely
to collide with the SGI driver.

KVM is the only other user of ...end(), and while KVM also blocks fault
handlers in ...start(), the fault handlers do not sleep and originate in
killable ioctl() calls.  So while it's possible for the i915 and Nouveau
drivers to collide with KVM, the bug is benign for KVM since the process
is dying and KVM's guest is about to be terminated.

So, as of today, the bug is likely benign.  But, that may not always be
true, e.g. there is a potential use case for blocking memslot updates in
KVM while an invalidation is in-progress, and failure to unblock would
result in said updates being blocked indefinitely and hanging.

Found by inspection.  Verified by adding a second notifier in KVM that
periodically returns -EAGAIN on non-blockable ranges, triggering OOM,
and observing that KVM exits with an elevated notifier count.

Fixes: 93065ac753e4 ("mm, oom: distinguish blockable mode for mmu notifiers")
Cc: stable@...r.kernel.org
Cc: David Rientjes <rientjes@...gle.com>
Cc: Ben Gardon <bgardon@...gle.com>
Cc: Jason Gunthorpe <jgg@...pe.ca>
Cc: Michal Hocko <mhocko@...e.com>
Cc: "Jérôme Glisse" <jglisse@...hat.com>
Cc: Andrea Arcangeli <aarcange@...hat.com>
Cc: Johannes Weiner <hannes@...xchg.org>
Cc: Dimitri Sivanich <dimitri.sivanich@....com>
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
 mm/oom_kill.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index bc65ba4f5192..acc3ba8b2ed7 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -546,12 +546,10 @@ bool __oom_reap_task_mm(struct mm_struct *mm)
 						vma, mm, vma->vm_start,
 						vma->vm_end);
 			tlb_gather_mmu(&tlb, mm);
-			if (mmu_notifier_invalidate_range_start_nonblock(&range)) {
-				tlb_finish_mmu(&tlb);
+			if (!mmu_notifier_invalidate_range_start_nonblock(&range))
+				unmap_page_range(&tlb, vma, range.start, range.end, NULL);
+			else
 				ret = false;
-				continue;
-			}
-			unmap_page_range(&tlb, vma, range.start, range.end, NULL);
 			mmu_notifier_invalidate_range_end(&range);
 			tlb_finish_mmu(&tlb);
 		}
-- 
2.30.1.766.gb4fecdf3b7-goog