From patchwork Fri Sep 30 00:38:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos Llamas X-Patchwork-Id: 12994805 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 14EEBC4332F for ; Fri, 30 Sep 2022 00:38:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7D4678D0003; Thu, 29 Sep 2022 20:38:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 783658D0001; Thu, 29 Sep 2022 20:38:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 64B268D0003; Thu, 29 Sep 2022 20:38:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 554848D0001 for ; Thu, 29 Sep 2022 20:38:51 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 15F9C1613F1 for ; Fri, 30 Sep 2022 00:38:51 +0000 (UTC) X-FDA: 79966891662.22.4D32E86 Received: from mail-pl1-f202.google.com (mail-pl1-f202.google.com [209.85.214.202]) by imf08.hostedemail.com (Postfix) with ESMTP id B8B8A160009 for ; Fri, 30 Sep 2022 00:38:49 +0000 (UTC) Received: by mail-pl1-f202.google.com with SMTP id k2-20020a170902c40200b001782bd6c416so2046508plk.20 for ; Thu, 29 Sep 2022 17:38:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date; bh=/+fx+mwCY8+Lc56RPP8V/faepECB7hZCBsaDQg4I+cc=; b=ZPIfPk960PYGAaC4JKn3VhKL1eWkgdn15xSuOq0AqzXIKYEmxriwqSGt46TymakdtA /88dDki+5epxVvUUdNpksj2JlRcU49C43GAGUYFwoxlMoMMZzcloMxlw0N58WxXjbSF5 Se1wmsCkfGkQA6wGwZIJVRBo0ov3kcqv0PJ6Mgs46fJroX+CTf5nL6QxeHAqMmNuDhlZ ZLXf2tQf5TEofLxvvoOQDpbcPGt8E2VYM2m6hfN5/opHRl41WQOHFZTv60EYrlI+6DWW Nsguytj9sRfljVBTavr1oo/pxTS20+TfcaZxA/eALqkcFI2Hr/m+qg9rLTonFipBCh/n UdqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date; bh=/+fx+mwCY8+Lc56RPP8V/faepECB7hZCBsaDQg4I+cc=; b=1V0r7uYWgV/SapAGUOmXq3nVTlDQ2CJY/YUK0m/XnbdNSLC75KTohKCJCFJeIqIJEV 3KqjFi+3JAVVLNJIw2/X/JQ/UIKxwjZBQilDhV/qmeFVP7Sl316EWNFyrNTztsioe2x0 fIRVJ7gKRxnJBa+TXuf3EFnpx3bWSMX4OHhAWJT4vqhUJuT+l35TNAFrgKOUHMxelagl XXs+hPJMuU9zq21XJtL6ohRVP/Fsf/3ih/OrdUN+raYe5ynK3BUurO3B4eS5RfPUdJLZ I4ncpov+cUwDYgOyQSN8zzTjUkSJBVjDhRecbufiltKj9LlYrLVTanxsSFC4+mSN//Bv Ocow== X-Gm-Message-State: ACrzQf2bgUzzRLQ3J7zL8/6AiAkt5oGlCOipKs6rYGI6oFiZZ/LMy9X9 Vy107Wu960I9ITp5HSIjLzkRZR8azXdgvg== X-Google-Smtp-Source: AMsMyM4lxRMX5F6XPH9FcDGPC4sqDa6OaOIE88Xx7FH+Jf5rB7ppzEvhMHium/cWA0tiotWLWDTX/zXq/h0YZw== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a05:6a00:150e:b0:548:d48:5a3c with SMTP id q14-20020a056a00150e00b005480d485a3cmr5928693pfu.27.1664498328465; Thu, 29 Sep 2022 17:38:48 -0700 (PDT) Date: Fri, 30 Sep 2022 00:38:43 +0000 Mime-Version: 1.0 X-Mailer: git-send-email 2.38.0.rc1.362.ged0d419d3c-goog Message-ID: <20220930003844.1210987-1-cmllamas@google.com> Subject: [PATCH] mm/mmap: undo ->mmap() when arch_validate_flags() fails From: Carlos Llamas To: Andrew Morton , Catalin Marinas Cc: Michal Hocko , Christian Brauner , linux-mm@kvack.org, bpf@vger.kernel.org, kernel-team@android.com, Carlos Llamas , Liam Howlett , Suren Baghdasaryan , stable@vger.kernel.org ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ZPIfPk96; spf=pass (imf08.hostedemail.com: domain of 3mDo2YwgKCAcjtsshthznvvnsl.jvtspu14-ttr2hjr.vyn@flex--cmllamas.bounces.google.com designates 209.85.214.202 as permitted sender) smtp.mailfrom=3mDo2YwgKCAcjtsshthznvvnsl.jvtspu14-ttr2hjr.vyn@flex--cmllamas.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664498329; a=rsa-sha256; cv=none; b=z2n/jOirXGgT7mCPq9thpvyBqjvWXIrJpbsg/OJIhKKvpumnYioNwzeAdrEEl8PEKyL3+m 5QUjQGyTGkMBKjRP39va2nP85om5xjt5WoZeA8AGAScMTVLDUXLF+mcxrP6WOXg9corXOz bquYbU7PYiSF5/a4veKBG2naeUMezYo= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664498329; 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: references:dkim-signature; bh=/+fx+mwCY8+Lc56RPP8V/faepECB7hZCBsaDQg4I+cc=; b=mlWI6r6eme+6ncoHD0v9YQqXIgWdkEV6OG4WqUX6GjhSlHjVx4kn7x5rQt73LjKruaQu/p 5kb6jiRzuOm9BZYmXSvRstpz0LxC4S61QppWly21HJnCNOUP4pav4z9M54Kso81swc0PbC Qm0OvJi6mENyoPc3JDA9CTSpCoPHnOM= Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ZPIfPk96; spf=pass (imf08.hostedemail.com: domain of 3mDo2YwgKCAcjtsshthznvvnsl.jvtspu14-ttr2hjr.vyn@flex--cmllamas.bounces.google.com designates 209.85.214.202 as permitted sender) smtp.mailfrom=3mDo2YwgKCAcjtsshthznvvnsl.jvtspu14-ttr2hjr.vyn@flex--cmllamas.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com X-Stat-Signature: 7hbaxi9fo3s5cp9ntfpnsfpruyo11zgu X-Rspamd-Queue-Id: B8B8A160009 X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1664498329-166641 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: Commit c462ac288f2c ("mm: Introduce arch_validate_flags()") added a late check in mmap_region() to let architectures validate vm_flags. The check needs to happen after calling ->mmap() as the flags can potentially be modified during this callback. If arch_validate_flags() check fails we unmap and free the vma. However, the error path fails to undo the ->mmap() call that previously succeeded and depending on the specific ->mmap() implementation this translates to reference increments, memory allocations and other operations what will not be cleaned up. There are several places (mainly device drivers) where this is an issue. However, one specific example is bpf_map_mmap() which keeps count of the mappings in map->writecnt. The count is incremented on ->mmap() and then decremented on vm_ops->close(). When arch_validate_flags() fails this count is off since bpf_map_mmap_close() is never called. One can reproduce this issue in arm64 devices with MTE support. Here the vm_flags are checked to only allow VM_MTE if VM_MTE_ALLOWED has been set previously. From userspace then is enough to pass the PROT_MTE flag to mmap() syscall to trigger the arch_validate_flags() failure. The following program reproduces this issue: Reviewed-by: Catalin Marinas Acked-by: Andrii Nakryiko --- #include #include #include #include #include int main(void) { union bpf_attr attr = { .map_type = BPF_MAP_TYPE_ARRAY, .key_size = sizeof(int), .value_size = sizeof(long long), .max_entries = 256, .map_flags = BPF_F_MMAPABLE, }; int fd; fd = syscall(__NR_bpf, BPF_MAP_CREATE, &attr, sizeof(attr)); mmap(NULL, 4096, PROT_WRITE | PROT_MTE, MAP_SHARED, fd, 0); return 0; } --- By manually adding some log statements to the vm_ops callbacks we can confirm that when passing PROT_MTE to mmap() the map->writecnt is off upon ->release(): With PROT_MTE flag: root@debian:~# ./bpf-test [ 111.263874] bpf_map_write_active_inc: map=9 writecnt=1 [ 111.288763] bpf_map_release: map=9 writecnt=1 Without PROT_MTE flag: root@debian:~# ./bpf-test [ 157.816912] bpf_map_write_active_inc: map=10 writecnt=1 [ 157.830442] bpf_map_write_active_dec: map=10 writecnt=0 [ 157.832396] bpf_map_release: map=10 writecnt=0 This patch fixes the above issue by calling vm_ops->close() when the arch_validate_flags() check fails, after this we can proceed to unmap and free the vma on the error path. Fixes: c462ac288f2c ("mm: Introduce arch_validate_flags()") Cc: Catalin Marinas Cc: Liam Howlett Cc: Suren Baghdasaryan Cc: # v5.10+ Signed-off-by: Carlos Llamas --- mm/mmap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mm/mmap.c b/mm/mmap.c index 9d780f415be3..36c08e2c78da 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1797,7 +1797,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, if (!arch_validate_flags(vma->vm_flags)) { error = -EINVAL; if (file) - goto unmap_and_free_vma; + goto close_and_free_vma; else goto free_vma; } @@ -1844,6 +1844,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr, return addr; +close_and_free_vma: + if (vma->vm_ops && vma->vm_ops->close) + vma->vm_ops->close(vma); unmap_and_free_vma: fput(vma->vm_file); vma->vm_file = NULL;