From patchwork Thu Nov 5 17:41:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: William Breathitt Gray X-Patchwork-Id: 1395172 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CRrRr4KXNz9sTv; Fri, 6 Nov 2020 04:41:44 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1kajGW-00004u-II; Thu, 05 Nov 2020 17:41:40 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kajGT-0008V6-RV for kernel-team@lists.ubuntu.com; Thu, 05 Nov 2020 17:41:37 +0000 Received: from mail-qv1-f69.google.com ([209.85.219.69]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kajGT-0000VO-Gz for kernel-team@lists.ubuntu.com; Thu, 05 Nov 2020 17:41:37 +0000 Received: by mail-qv1-f69.google.com with SMTP id dd7so1299359qvb.6 for ; Thu, 05 Nov 2020 09:41:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=deHXIyVlGqO29Gcgb/BP+X0dHdQGY1/T+0K6BycGcIY=; b=c7XlRqkzq6EtbFMWj5GbADRVKrt2PKamgC71USjjl3jcz4ZBrlvluZqAIjLTQ/qX+e YdevHZIYk05kHbq3U4kClysACJqXDB+elEB6uE/5jh/EckZ9BxkPtHArF2dGKg9uff0h 8wAAL0B6wlil1mTpDOki0gqaRT+XA0JjBNeY0S8otcJX8gjlrcTTKfDnyz70a7XNo6VA 376em+pu+zB/eq/FaRE1Z6vYFgKH3cJpTL/JzBxwj6UMygQBsvOZGCAhzmJYsdy4EU/H 0E+A0UQpwvbW9wjODzeeNrSzSyyNyP82gH0RGnVAklWlLQHqMzRsD1BIM0K9h48Wpj5m GJew== X-Gm-Message-State: AOAM532XBBmTdc4xzRbIBXIqXgTjlpSDE+QN8G+SZO1NRWQr0z2f/cOF Shd33TKw7+ixrLP+F0r0Ti7Wtt4TQqgkLf76P23rsUnnRYVPpNpKVKBH+j8PWIsH2pyuc/8v2DI ZUNyPb+JepAr2e0bOOMk71yOCeQqsNcEmYB+BU/59cg== X-Received: by 2002:a05:620a:408f:: with SMTP id f15mr3138870qko.276.1604598096430; Thu, 05 Nov 2020 09:41:36 -0800 (PST) X-Google-Smtp-Source: ABdhPJzCHp7B/9ViSYclTAIjLopI1lnA375wynB3HFFTCFXPI+gC8fvxDGRDnWAdjifPRa5VWzCWcA== X-Received: by 2002:a05:620a:408f:: with SMTP id f15mr3138849qko.276.1604598096195; Thu, 05 Nov 2020 09:41:36 -0800 (PST) Received: from localhost.localdomain (072-189-064-225.res.spectrum.com. [72.189.64.225]) by smtp.gmail.com with ESMTPSA id o20sm1246820qtw.30.2020.11.05.09.41.34 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Nov 2020 09:41:35 -0800 (PST) From: William Breathitt Gray To: kernel-team@lists.ubuntu.com Subject: [SRU][CVE-2020-14351][X][PATCH 2/2] perf/core: Fix race in the perf_mmap_close() function Date: Thu, 5 Nov 2020 12:41:32 -0500 Message-Id: <20201105174132.80472-3-william.gray@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201105174132.80472-1-william.gray@canonical.com> References: <20201105174132.80472-1-william.gray@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jiri Olsa There's a possible race in perf_mmap_close() when checking ring buffer's mmap_count refcount value. The problem is that the mmap_count check is not atomic because we call atomic_dec() and atomic_read() separately. perf_mmap_close: ... atomic_dec(&rb->mmap_count); ... if (atomic_read(&rb->mmap_count)) goto out_put; free_uid out_put: ring_buffer_put(rb); /* could be last */ The race can happen when we have two (or more) events sharing same ring buffer and they go through atomic_dec() and then they both see 0 as refcount value later in atomic_read(). Then both will go on and execute code which is meant to be run just once. The code that detaches ring buffer is probably fine to be executed more than once, but the problem is in calling free_uid(), which will later on demonstrate in related crashes and refcount warnings, like: refcount_t: addition on 0; use-after-free. ... RIP: 0010:refcount_warn_saturate+0x6d/0xf ... Call Trace: prepare_creds+0x190/0x1e0 copy_creds+0x35/0x172 copy_process+0x471/0x1a80 _do_fork+0x83/0x3a0 __do_sys_wait4+0x83/0x90 __do_sys_clone+0x85/0xa0 do_syscall_64+0x5b/0x1e0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Using atomic decrease and check instead of separated calls. Tested-by: Michael Petlan Signed-off-by: Jiri Olsa Signed-off-by: Ingo Molnar Acked-by: Peter Zijlstra Acked-by: Namhyung Kim Acked-by: Wade Mealing Fixes: 9bb5d40cd93c ("perf: Fix mmap() accounting hole"); Link: https://lore.kernel.org/r/20200916115311.GE2301783@krava CVE-2020-14351 (backported from commit f91072ed1b7283b13ca57fcfbece5a3b92726143) [ vilhelmgray: context adjustment ] Signed-off-by: William Breathitt Gray --- kernel/events/core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 388ce03155b4..d399748ea86b 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4664,11 +4664,11 @@ static void perf_mmap_open(struct vm_area_struct *vma) static void perf_mmap_close(struct vm_area_struct *vma) { struct perf_event *event = vma->vm_file->private_data; - struct ring_buffer *rb = ring_buffer_get(event); struct user_struct *mmap_user = rb->mmap_user; int mmap_locked = rb->mmap_locked; unsigned long size = perf_data_size(rb); + bool detach_rest = false; if (event->pmu->event_unmapped) event->pmu->event_unmapped(event); @@ -4687,7 +4687,8 @@ static void perf_mmap_close(struct vm_area_struct *vma) mutex_unlock(&event->mmap_mutex); } - atomic_dec(&rb->mmap_count); + if (atomic_dec_and_test(&rb->mmap_count)) + detach_rest = true; if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) goto out_put; @@ -4696,7 +4697,7 @@ static void perf_mmap_close(struct vm_area_struct *vma) mutex_unlock(&event->mmap_mutex); /* If there's still other mmap()s of this buffer, we're done. */ - if (atomic_read(&rb->mmap_count)) + if (!detach_rest) goto out_put; /*