From patchwork Fri Oct 11 15:50:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 13832705 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 5B565D0EE05 for ; Fri, 11 Oct 2024 15:51:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CAFDE6B00AF; Fri, 11 Oct 2024 11:51:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C67D96B00B5; Fri, 11 Oct 2024 11:51:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AFD2C6B00B6; Fri, 11 Oct 2024 11:51:18 -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 90B936B00AF for ; Fri, 11 Oct 2024 11:51:18 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id ECB58AB194 for ; Fri, 11 Oct 2024 15:51:08 +0000 (UTC) X-FDA: 82661760552.21.87E639B Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by imf19.hostedemail.com (Postfix) with ESMTP id E49FB1A0011 for ; Fri, 11 Oct 2024 15:51:12 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="iwWz/kIu"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jannh@google.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728661847; a=rsa-sha256; cv=none; b=K1ilE6Iaz2iTCR+h7OIlrvPpXmZUxcvfVQ15zDEXXYn4pqKsozTTXNG3QYKH7pwPysocNI 46u4jGH0N8ubkAKGXKHs3a7l5WUpPZgvg3mnYhtW3jhMKttTFzYMAbTZWdwS874zndr4As 9OhEhCUvl40ijyc6cvdygwxQUxl/4Cw= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="iwWz/kIu"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jannh@google.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728661847; 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:content-transfer-encoding:in-reply-to: references:dkim-signature; bh=9YRE8/CzNlPy/fMHd19CbAQn1zdYlnyMbeoTyUUS2vk=; b=4kJWXBJ2oGl/e/Ym6+ystxH/ErFkVPvtWRtOBeY/pbC5rg2SX+uLWm5CHJ4Wpg5diL5MXM 5EqTogyL8SMupjCM8Uy/a9ZgVLYsoND5K/2eb4ZapqPIRDFLgQx3CBTG+lkrGCZsEd4qju ul/sKCYO3Pl4BjO+MpFUC+VaaneRzd0= Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-5398c2e4118so15142e87.0 for ; Fri, 11 Oct 2024 08:51:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728661874; x=1729266674; darn=kvack.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=9YRE8/CzNlPy/fMHd19CbAQn1zdYlnyMbeoTyUUS2vk=; b=iwWz/kIuUSRzbMPnzyRMksGPWQEmAlUdf64WJ3wlqkP3oXdRiYaE5//hUaNWA8H1G7 CeeAS/+qcoL7lIVzmjRVI3MQLD6+P73w6B5UpoOn0nrFTbdepL7vgPwKHlROgDRGKN6A RubWwpSlmhU7gP+TcN+hyN37Jk4/8n8lAWOdT1InWmHHiuEtY1WxNfsKb4h3h5S+f8T6 uvJX1I3KNECAttYBCoEpn5OcdIENjJszMyq+5hnErxtCfWGDh5hZqgEdqzDuHpvdqj2g UrZs6VBXT0OgDg5eDMggvyLzH4iP8eemZqaiNh/mejAf6v8vDRH8nEhMwur95+WFhUAM oqxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728661874; x=1729266674; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=9YRE8/CzNlPy/fMHd19CbAQn1zdYlnyMbeoTyUUS2vk=; b=fdR7W0BdTVWp18O6SM6g9H5iWKjbxTLfBXMEaVwu0rycAqf7uG8A9EOanBP50HMmov uzarYhimmNRKoKyG4Y8+dTgIhurEWnxK5FolFCsqPR+Kpy7KeaY6pljAGRZawavccFIE x8IzhcAL99MphL2zE4LgepIJeBmkD+6oih2VvE8eMR4M2Fwu1iNchLGexCQ95kIP9bHh 7dSCMGHHDqtzLRjTG+BMoHzjvjNW+qUi1v/6FeMQtw2boA1qCWQfzmNDFMSk12zmw6Dp P+dIb7Ag86pPw/gg7DH4kL7KIkaSM6n+xO4Cl4cBLoshgRDI0RFs+1s5h2SVLwOkkrq7 C2zQ== X-Forwarded-Encrypted: i=1; AJvYcCXvdy9JcDO7RsMmwvY3a6Cd2mb3/W2/Unzkku0MSTDDAKUSyCu6Cu5sz4MI9FzcfpkbooPJBo9A2Q==@kvack.org X-Gm-Message-State: AOJu0Yy3qy0C6ZZggoXUGU3Q8DF6oaXE3z1QQ1x7m7orIoth0enAQbTY pED41SOZ8RtusklyV0YAmZ+jWcsDWTKYFJPm6ohFdMAtHMiEy7MaJC78lyn+/g== X-Google-Smtp-Source: AGHT+IHbRO/TNFy7alHZxLobfYm/Oko0TRXn7KWSllCdtpS6OjkyKtxH3+hpEOvuWxvc/O2hrXqvBg== X-Received: by 2002:a05:6512:3e12:b0:538:9e44:3034 with SMTP id 2adb3069b0e04-539d6937c6cmr273779e87.6.1728661873505; Fri, 11 Oct 2024 08:51:13 -0700 (PDT) Received: from localhost ([2a00:79e0:9d:4:b0bd:4045:f14c:aaf5]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-37d4b79f9b4sm4248242f8f.86.2024.10.11.08.51.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Oct 2024 08:51:11 -0700 (PDT) From: Jann Horn Date: Fri, 11 Oct 2024 17:50:54 +0200 Subject: [PATCH RFC v2] mm: Enforce the stack gap when changing inaccessible VMAs MIME-Version: 1.0 Message-Id: <20241011-stack-gap-inaccessible-v2-1-111b6a0ee2cb@google.com> X-B4-Tracking: v=1; b=H4sIAF1JCWcC/4WNQQqDMBBFryKzboqTBoxdFQo9QLfFRUzGOFSNJ CIt4t0bvECX73/++xskikwJrsUGkVZOHKYM8lSA7c3kSbDLDLKUCstSi7QY+xbezIInYy2lxO1 AwlYXrLvKqRZbyOM5UsefQ/yC5+MOTQ57TkuI3+NsxaP6511RoNBKO+V0jZ3Emw/BD3S2YYRm3 /cft9gtJ8IAAAA= To: Andrew Morton , "Liam R. Howlett" , Lorenzo Stoakes , Vlastimil Babka Cc: Hugh Dickins , Oleg Nesterov , Michal Hocko , Helge Deller , Ben Hutchings , Willy Tarreau , Rik van Riel , linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Jann Horn X-Mailer: b4 0.15-dev X-Developer-Signature: v=1; a=ed25519-sha256; t=1728661866; l=8264; i=jannh@google.com; s=20240730; h=from:subject:message-id; bh=HUif9feDehDnmA4OSsbtnTJKJixKUTKSxisX6qbYo20=; b=L/KAPOP/fjJFNJQs+u+BvgpI0X/1jnDmAhoZeLII7xgJiUM/TUvNiOJW8hDegFH/+QXqnxHNu kasxkG6dsZTA1klbWQwFJoaRQw/4x/A/eKd8bMzIiSaBmMjRuMRsKHV X-Developer-Key: i=jannh@google.com; a=ed25519; pk=AljNtGOzXeF6khBXDJVVvwSEkVDGnnZZYqfWhP1V+C8= X-Rspam-User: X-Stat-Signature: u86th9e3qtu33functiw4rmsg56s5c1n X-Rspamd-Queue-Id: E49FB1A0011 X-Rspamd-Server: rspam02 X-HE-Tag: 1728661872-631004 X-HE-Meta: U2FsdGVkX18jSOylio/j7srV2YdDgxwYEhJWe4d6regbjM7XuxmQLN04a9dh+y6NaxOMIjfrwt6yNBuswQ+6JpMCmtLBuXXzjYr8ydsT8OWs7eDY+/oypxKVR6c0DgYg8RGX+db3BY0FC2T07sTFtcXvFus0rFpqn7va/kUkgLI5UIBF71sZqfgw2Vw3DfmAmS442CiSHvgBI730JKmbejDNg6J18nPYAV8axZyx6NfY+JqjrqlSBCUCZ3uGGUmCHpnjLQzEcJNLOHLdtYtHxckTwAR49taj6qWOFXPGWERIpdf6wKr+7o4YD+06rdFsArOLHfqflc0oy6N05VofZDmetKHHhfJpocDs+tyBgRTfoyRmOweETG0B2Pa7rvvIcGBTQMcH/1kf5BqA0aHM74soLTUrWiTu+7/1A+k6vAeAI1CfaaGnnb+1or6CyqJ/58jPam2KTT3piOqUwQmUT4B9qH+87DGfRNqphamJQGDIT3NX2XfeqQvAW9JJii05mu2WN/l5lAQfnu2KALq27/UGneDn+CHipYOZssaj37pdG3HMNVcGX9JA6/FdpPWE5YxI3NtrKw0q87fFna1wjQedpm/qi74HQM8ln2I1vLLX2YX2pDA7PhFoQsNbp9MpJgZg6IoreOKcjjQGpDXqIBzDT0/sWUMypH5X13Y50hq7WZgImcIdiSjIfL0GBZkxZ27ham9Legi14TVkh+kPD73tCPwOA5n+MjhU0gfkOBt/ZzI+OnygrfPhPcxQkMbPvvqZ3e0oqTqjjCFLGvByMBTH3siP1MVbZlvNMeQWM1lxlDnTjpqk51xZZ01nv0IvdPGTVe4+yArMIkMFVyRl+vCkhcnA38IGg9iuO/B0jgG362QOBHd+N3DGPwY1jKzRk/v2eg9GOuZTiU2B0oHSwG19UYpu+bERhnVh/qmJRB/c99XIcMbGrGqwMbnoBfXn4nzaZeQkxnC/3s824KH Dex/OPCG 1O08oUpIPgWqcuz4A9tbgDHBSukjcNbXE4JBb1BshyJAeQFc5Ov2/NcRF/3UYf4RijZwFeE9KEFKSojJyBCPUnmJuDXXsvW5pZ7/shjFZ4fuHXpPDexqYsxfBKAX0m57R9sx2c+UgOWoudFNJ+vVm8vb3eMZErEbyRzNIhamsf1NQNCy2O1WoMX64wIx7M8a+/i8XKd9F8JjgjEnvNqIRCVMnVtu8MFwd8s4X/+PdFMlM5QYEtkZ1ivxD2cK5cz2nWg8nTXgcZVVfk2H8ixeyuUFpaik14eHgyvKDi3Rv35jsRlDmMXZ86NoD2Jy8Xf3mDCbswj/J80exDKvpVJ+AKq9PaoVc99CcaEedZt7ijTKl0p5VJdw+U+9vf97uWIfO/Rgc0EgR8MlPDbMzkzML7ml1VwOsqLNWlm2a285bneJpHtQJK47Yger4Y50hTLkhAu+FXH84OFxrFN8= 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: List-Subscribe: List-Unsubscribe: As explained in the comment block this change adds, we can't tell what userspace's intent is when the stack grows towards an inaccessible VMA. We should ensure that, as long as code is compiled with something like -fstack-check, a stack overflow in this code can never cause the main stack to overflow into adjacent heap memory - so the bottom of a stack should never be directly adjacent to an accessible VMA. As suggested by Lorenzo, enforce this by blocking attempts to: - make an inaccessible VMA accessible with mprotect() when it is too close to a stack - replace an inaccessible VMA with another VMA using MAP_FIXED when it is too close to a stack I have a (highly contrived) C testcase for 32-bit x86 userspace with glibc that mixes malloc(), pthread creation, and recursion in just the right way such that the main stack overflows into malloc() arena memory, see the linked list post. I don't know of any specific scenario where this is actually exploitable, but it seems like it could be a security problem for sufficiently unlucky userspace. Link: https://lore.kernel.org/r/CAG48ez2v=r9-37JADA5DgnZdMLCjcbVxAjLt5eH5uoBohRdqsw@mail.gmail.com/ Suggested-by: Lorenzo Stoakes Fixes: 561b5e0709e4 ("mm/mmap.c: do not blow on PROT_NONE MAP_FIXED holes in the stack") Cc: stable@vger.kernel.org Signed-off-by: Jann Horn --- This is an attempt at the alternate fix approach suggested by Lorenzo. This turned into more code than I would prefer to have for a scenario like this... Also, the way the gap is enforced in my patch for MAP_FIXED_NOREPLACE is a bit ugly. In the existing code, __get_unmapped_area() normally already enforces the stack gap even when it is called with a hint; but when MAP_FIXED_NOREPLACE is used, we kinda lie to __get_unmapped_area() and tell it we'll do a MAP_FIXED mapping (introduced in commit a4ff8e8620d3f when MAP_FIXED_NOREPLACE was created), then afterwards manually reject overlapping mappings. So I ended up also doing the gap check separately for MAP_FIXED_NOREPLACE. The following test program exercises scenarios that could lead to the stack becoming directly adjacent to another accessible VMA, and passes with this patch applied: <<< int main(void) { setbuf(stdout, NULL); char *ptr = (char*)( (unsigned long)(STACK_POINTER() - (1024*1024*4)/*4MiB*/) & ~0xfffUL ); if (mmap(ptr, 0x1000, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0) != ptr) err(1, "mmap distant-from-stack"); *(volatile char *)(ptr + 0x1000); /* expand stack */ system("echo;cat /proc/$PPID/maps;echo"); /* test transforming PROT_NONE mapping adjacent to stack */ if (mprotect(ptr, 0x1000, PROT_READ|PROT_WRITE|PROT_EXEC) == 0) errx(1, "mprotect adjacent to stack allowed"); if (mmap(ptr, 0x1000, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE|MAP_FIXED, -1, 0) != MAP_FAILED) errx(1, "MAP_FIXED adjacent to stack allowed"); if (munmap(ptr, 0x1000)) err(1, "munmap failed???"); /* test creating new mapping adjacent to stack */ if (mmap(ptr, 0x1000, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE|MAP_FIXED_NOREPLACE, -1, 0) != MAP_FAILED) errx(1, "MAP_FIXED_NOREPLACE adjacent to stack allowed"); if (mmap(ptr, 0x1000, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0) == ptr) errx(1, "mmap hint adjacent to stack accepted"); printf("all tests passed\n"); } >>> --- Changes in v2: - Entirely new approach (suggested by Lorenzo) - Link to v1: https://lore.kernel.org/r/20241008-stack-gap-inaccessible-v1-1-848d4d891f21@google.com --- include/linux/mm.h | 1 + mm/mmap.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ mm/mprotect.c | 6 +++++ 3 files changed, 72 insertions(+) --- base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b change-id: 20241008-stack-gap-inaccessible-c7319f7d4b1b diff --git a/include/linux/mm.h b/include/linux/mm.h index ecf63d2b0582..ecd4afc304ca 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -3520,6 +3520,7 @@ unsigned long change_prot_numa(struct vm_area_struct *vma, struct vm_area_struct *find_extend_vma_locked(struct mm_struct *, unsigned long addr); +bool overlaps_stack_gap(struct mm_struct *mm, unsigned long addr, unsigned long len); int remap_pfn_range(struct vm_area_struct *, unsigned long addr, unsigned long pfn, unsigned long size, pgprot_t); int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr, diff --git a/mm/mmap.c b/mm/mmap.c index dd4b35a25aeb..937361be3c48 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -359,6 +359,20 @@ unsigned long do_mmap(struct file *file, unsigned long addr, return -EEXIST; } + /* + * This does two things: + * + * 1. Disallow MAP_FIXED replacing a PROT_NONE VMA adjacent to a stack + * with an accessible VMA. + * 2. Disallow MAP_FIXED_NOREPLACE creating a new accessible VMA + * adjacent to a stack. + */ + if ((flags & (MAP_FIXED_NOREPLACE | MAP_FIXED)) && + (prot & (PROT_READ | PROT_WRITE | PROT_EXEC)) && + !(vm_flags & (VM_GROWSUP|VM_GROWSDOWN)) && + overlaps_stack_gap(mm, addr, len)) + return (flags & MAP_FIXED) ? -ENOMEM : -EEXIST; + if (flags & MAP_LOCKED) if (!can_do_mlock()) return -EPERM; @@ -1341,6 +1355,57 @@ struct vm_area_struct *expand_stack(struct mm_struct *mm, unsigned long addr) return vma; } +/* + * Does the specified VA range overlap the stack gap of a preceding or following + * stack VMA? + * Overlapping stack VMAs are ignored - so if someone deliberately creates a + * MAP_FIXED mapping in the middle of a stack or such, we let that go through. + * + * This is needed partly because userspace's intent when making PROT_NONE + * mappings is unclear; there are two different reasons for creating PROT_NONE + * mappings: + * + * A) Userspace wants to create its own guard mapping, for example for stacks. + * According to + * , + * some Rust/Java programs do this with the main stack. + * Enforcing the kernel's stack gap between these userspace guard mappings and + * the main stack breaks stuff. + * + * B) Userspace wants to reserve some virtual address space for later mappings. + * This is done by memory allocators. + * In this case, we want to enforce a stack gap between the mapping and the + * stack. + * + * Because we can't tell these cases apart when a PROT_NONE mapping is created, + * we instead enforce the stack gap when a PROT_NONE mapping is made accessible + * (using mprotect()) or replaced with an accessible one (using MAP_FIXED). + */ +bool overlaps_stack_gap(struct mm_struct *mm, unsigned long addr, unsigned long len) +{ + + struct vm_area_struct *vma, *prev_vma; + + /* step 1: search for a non-overlapping following stack VMA */ + vma = find_vma(mm, addr+len); + if (vma && vma->vm_start >= addr+len) { + /* is it too close? */ + if (vma->vm_start - (addr+len) < stack_guard_start_gap(vma)) + return true; + } + + /* step 2: search for a non-overlapping preceding stack VMA */ + if (!IS_ENABLED(CONFIG_STACK_GROWSUP)) + return false; + vma = find_vma_prev(mm, addr, &prev_vma); + /* don't handle cases where the VA start overlaps a VMA */ + if (vma && vma->vm_start < addr) + return false; + if (!prev_vma || !(prev_vma->vm_flags & VM_GROWSUP)) + return false; + return addr - prev_vma->vm_end < stack_guard_gap; +} + /* do_munmap() - Wrapper function for non-maple tree aware do_munmap() calls. * @mm: The mm_struct * @start: The start address to munmap diff --git a/mm/mprotect.c b/mm/mprotect.c index 0c5d6d06107d..2300e2eff956 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -772,6 +772,12 @@ static int do_mprotect_pkey(unsigned long start, size_t len, } } + error = -ENOMEM; + if ((prot & (PROT_READ | PROT_WRITE | PROT_EXEC)) && + !(vma->vm_flags & (VM_GROWSUP|VM_GROWSDOWN)) && + overlaps_stack_gap(current->mm, start, end - start)) + goto out; + prev = vma_prev(&vmi); if (start > vma->vm_start) prev = vma;