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: 12994804 X-Patchwork-Delegate: kuba@kernel.org 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 307E9C433FE for ; Fri, 30 Sep 2022 00:38:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229892AbiI3Aiu (ORCPT ); Thu, 29 Sep 2022 20:38:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58668 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229885AbiI3Aiu (ORCPT ); Thu, 29 Sep 2022 20:38:50 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ECFA7202898 for ; Thu, 29 Sep 2022 17:38:48 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id h19-20020a63e153000000b00434dfee8dbaso1845872pgk.18 for ; Thu, 29 Sep 2022 17:38:48 -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=A7X69/0wuD7b17nuF7MG1vJwOMmqCkoHfwpAclhvk0FGokX9LKGDBTIkvWz/glsFSg rouB9frGttrjPFQRfk2k64CaZvtQ93ogj1dzVzjls/Opuwu8WVH33vxVOFHm1+RdKRpP LO6rN/tTfVcuK3b5J2EbcdFH3RHXqQIkmjvQddiCvwrqDMHn7eyT24dQKHyjGtGmfGF2 3DjfitEjzgLWp4NnQOZa2fioI0V+oEFoCAzCJQJJg5z7TjDKlwBlcFoFX6aOS+n8Uh2v E1RNdZBtZb6VNW+n3obWXjqAwQY0dtnxxh3OFbKHd38UqosdpwDUYew8Num+jvyZWEtb RY6w== X-Gm-Message-State: ACrzQf34OilN8TOzC/b60FPalHviYvfSb9Ym+p0BBYXplPezMaj424tE UNnftn0QQ08ozNLNAK0QlOnqWPwLji4yfQ== 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 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org 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 Reviewed-by: Liam Howlett --- #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;