Message ID | 20230719154137.732d8525@imladris.surriel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp2685435vqt; Wed, 19 Jul 2023 13:19:47 -0700 (PDT) X-Google-Smtp-Source: APBJJlEP3k5a4acmESA3Z8FiMCpb0+rrhPgtJf4PtSftiH4leQB/xUFTBux2Q5c9BjjSI39CMsy2 X-Received: by 2002:a17:902:ecd1:b0:1b8:6cae:4400 with SMTP id a17-20020a170902ecd100b001b86cae4400mr3494475plh.37.1689797987374; Wed, 19 Jul 2023 13:19:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689797987; cv=none; d=google.com; s=arc-20160816; b=KqJPdwRgb8gNfzj8g3DJrMJF94aES42l2zCHEvpHyvWh540T/dUu2FYXk7p49TjYg/ MDNBC1BrizOaeFkKVM48ghB4aFd/ZhC8mj4zswZiIW0G6uh/Y3N+2pLVTyTB4pi5v8pU 8igKdpxqIZq7vdugu4F8TTKiaxe4/9PnAH8+dRBoRntPYKH5YLpp5JT3RU5x5G3HSGhI U0EQ/2bNqMGd3i43EdC4W6v2jxAc3PLpSKb1snypZYzFmTJUCWKUbeti4e7W0O1SiuAy n+l7FGICLS06KIBkc/m7jlOEsDYd5LEumo2F9RS/I6dbiAZ73stfuFwYq3yAUjetbWHT /xRw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :message-id:subject:cc:to:from:date; bh=TxhwGEmi0DVq2VEH35CQcVlYItPea1jOnoeoemjif5Y=; fh=553OgrKXpjGw02ZYS/BmVvzG0IiJjcKMOrFGimh/phQ=; b=g6YkaslL+7uW1Xe+W9HxVk5XXr2sbqYYixpUK57y9W/7vRFw2vX3I44Ew24zu+r/W0 LVSpJ6KR+1kHehcuI8C/TEtcY0aUib7lPegkKCTBF5kFB9n7rMJ1dtlBs5KdRNYjkQSM iMiqvn2oIPw1hD2FyTsNjGENvunLIiMxCVWohYJaEqoilPU0NLJkrBy8qLK3GKWLgEYz 3ZGCVgFIcwwcb10+XgxRaXmftgNOBa2CA/6FstYVMO7lXxJVhysSs28SShB5SaHCgAuU yYgBVkdQEJ0RfHb6iBeqgxClVSWBN8RghZgeyOQ1gajl1Zp6bBVQEGFVBotrZxEXDaGu +GSA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id f3-20020a170902ce8300b001bb3499d8besi4156776plg.478.2023.07.19.13.19.33; Wed, 19 Jul 2023 13:19:47 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231295AbjGSTws (ORCPT <rfc822;daweilics@gmail.com> + 99 others); Wed, 19 Jul 2023 15:52:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229853AbjGSTwq (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 19 Jul 2023 15:52:46 -0400 X-Greylist: delayed 654 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 19 Jul 2023 12:52:44 PDT Received: from shelob.surriel.com (shelob.surriel.com [96.67.55.147]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BDD3CA7 for <linux-kernel@vger.kernel.org>; Wed, 19 Jul 2023 12:52:44 -0700 (PDT) Received: from [2601:18c:8180:ac39:6e0b:84ff:fee2:98bb] (helo=imladris.surriel.com) by shelob.surriel.com with esmtpsa (TLS1.2) tls TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <riel@shelob.surriel.com>) id 1qMD3N-0007gS-36; Wed, 19 Jul 2023 15:41:41 -0400 Date: Wed, 19 Jul 2023 15:41:37 -0400 From: Rik van Riel <riel@surriel.com> To: Mike Rapoport <rppt@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: [PATCH] mm,memblock: reset memblock.reserved to system init state to prevent UAF Message-ID: <20230719154137.732d8525@imladris.surriel.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: riel@surriel.com X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771881614510857682 X-GMAIL-MSGID: 1771881614510857682 |
Series |
mm,memblock: reset memblock.reserved to system init state to prevent UAF
|
|
Commit Message
Rik van Riel
July 19, 2023, 7:41 p.m. UTC
The memblock_discard function frees the memblock.reserved.regions
array, which is good.
However, if a subsequent memblock_free (or memblock_phys_free) comes
in later, from for example ima_free_kexec_buffer, that will result in
a use after free bug in memblock_isolate_range.
When running a kernel with CONFIG_KASAN enabled, this will cause a
kernel panic very early in boot. Without CONFIG_KASAN, there is
a chance that memblock_isolate_range might scribble on memory
that is now in use by somebody else.
Avoid those issues by making sure that memblock_discard points
memblock.reserved.regions back at the static buffer.
If memblock_discard is called while there is still memory
in the memblock.reserved type, that will print a warning
in memblock_remove_region.
Signed-off-by: Rik van Riel <riel@surriel.com>
---
mm/memblock.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
Hi Ric, On Wed, Jul 19, 2023 at 03:41:37PM -0400, Rik van Riel wrote: > The memblock_discard function frees the memblock.reserved.regions > array, which is good. > > However, if a subsequent memblock_free (or memblock_phys_free) comes > in later, from for example ima_free_kexec_buffer, that will result in > a use after free bug in memblock_isolate_range. The use of memblock_phys_free() in ima_free_kexec_buffer() is entirely bogus and must be fixed. It should be memblock_free_late() there. > When running a kernel with CONFIG_KASAN enabled, this will cause a > kernel panic very early in boot. Without CONFIG_KASAN, there is > a chance that memblock_isolate_range might scribble on memory > that is now in use by somebody else. This can't happen because memblock_double_array() uses kmalloc() as soon as slab_is_available(), so this sentence is misleading. > Avoid those issues by making sure that memblock_discard points > memblock.reserved.regions back at the static buffer. > > If memblock_discard is called while there is still memory > in the memblock.reserved type, that will print a warning > in memblock_remove_region. > > Signed-off-by: Rik van Riel <riel@surriel.com> > --- > mm/memblock.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 3feafea06ab2..068289a46903 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -374,6 +374,10 @@ void __init memblock_discard(void) > kfree(memblock.reserved.regions); > else > memblock_free_late(addr, size); > + /* Reset to prevent UAF from stray frees. */ > + memblock.reserved.regions = memblock_reserved_init_regions; > + memblock.reserved.cnt = 1; > + memblock_remove_region(&memblock.reserved, 0); > } > > if (memblock.memory.regions != memblock_memory_init_regions) { > -- > 2.34.1 > >
On Thu, 2023-07-20 at 08:00 +0300, Mike Rapoport wrote: > Hi Ric, > > On Wed, Jul 19, 2023 at 03:41:37PM -0400, Rik van Riel wrote: > > The memblock_discard function frees the memblock.reserved.regions > > array, which is good. > > > > However, if a subsequent memblock_free (or memblock_phys_free) > > comes > > in later, from for example ima_free_kexec_buffer, that will result > > in > > a use after free bug in memblock_isolate_range. > > The use of memblock_phys_free() in ima_free_kexec_buffer() is > entirely > bogus and must be fixed. It should be memblock_free_late() there. > I'll send in a patch for that code, then. Thank you! > > When running a kernel with CONFIG_KASAN enabled, this will cause a > > kernel panic very early in boot. Without CONFIG_KASAN, there is > > a chance that memblock_isolate_range might scribble on memory > > that is now in use by somebody else. > > This can't happen because memblock_double_array() uses kmalloc() as > soon as > slab_is_available(), so this sentence is misleading. memblock_discard() freed the array, but did not change the pointer. That resulted in memblock_isolate_range() following a stale pointer. There was no call to memblock_double_array() in the final call that crashed. I checked that by booting with memblock=debug. kind regards, Rik van Riel
On Wed, Jul 19, 2023 at 03:41:37PM -0400, Rik van Riel wrote: > The memblock_discard function frees the memblock.reserved.regions > array, which is good. > > However, if a subsequent memblock_free (or memblock_phys_free) comes > in later, from for example ima_free_kexec_buffer, that will result in > a use after free bug in memblock_isolate_range. > > When running a kernel with CONFIG_KASAN enabled, this will cause a > kernel panic very early in boot. Without CONFIG_KASAN, there is > a chance that memblock_isolate_range might scribble on memory > that is now in use by somebody else. > > Avoid those issues by making sure that memblock_discard points > memblock.reserved.regions back at the static buffer. > > If memblock_discard is called while there is still memory > in the memblock.reserved type, that will print a warning > in memblock_remove_region. I'm going to apply this with the last paragraph rewritten as If memblock_free is called after memblock memory is discarded, that will print a warning in memblock_remove_region. > Signed-off-by: Rik van Riel <riel@surriel.com> > --- > mm/memblock.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 3feafea06ab2..068289a46903 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -374,6 +374,10 @@ void __init memblock_discard(void) > kfree(memblock.reserved.regions); > else > memblock_free_late(addr, size); > + /* Reset to prevent UAF from stray frees. */ > + memblock.reserved.regions = memblock_reserved_init_regions; > + memblock.reserved.cnt = 1; > + memblock_remove_region(&memblock.reserved, 0); > } > > if (memblock.memory.regions != memblock_memory_init_regions) { > -- > 2.34.1 > >
Hi, On Wed, Jul 19, 2023 at 03:41:37PM -0400, Rik van Riel wrote: > The memblock_discard function frees the memblock.reserved.regions > array, which is good. > > However, if a subsequent memblock_free (or memblock_phys_free) comes > in later, from for example ima_free_kexec_buffer, that will result in > a use after free bug in memblock_isolate_range. > > When running a kernel with CONFIG_KASAN enabled, this will cause a > kernel panic very early in boot. Without CONFIG_KASAN, there is > a chance that memblock_isolate_range might scribble on memory > that is now in use by somebody else. > > Avoid those issues by making sure that memblock_discard points > memblock.reserved.regions back at the static buffer. > > If memblock_discard is called while there is still memory > in the memblock.reserved type, that will print a warning > in memblock_remove_region. > > Signed-off-by: Rik van Riel <riel@surriel.com> This patch results in the following WARNING backtrace when booting sparc or sparc64 images in qemu. Bisect log is attached. sparc: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at mm/memblock.c:352 memblock_remove_region+0x80/0xb0 Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 6.5.0-rc3-00350-gb917f578dc45 #1 [f0022af8 : __warn+0x9c/0xe4 ] [f0022b84 : warn_slowpath_fmt+0x44/0x54 ] [f07a16a0 : memblock_remove_region+0x80/0xb0 ] [f0787240 : memblock_discard+0x84/0x100 ] [f0784e00 : page_alloc_init_late+0xc/0x5c ] [f07782fc : kernel_init_freeable+0xb8/0x208 ] [f05ad41c : kernel_init+0x10/0x120 ] [f0007ed0 : ret_from_kernel_thread+0xc/0x38 ] [00000000 : 0x0 ] ---[ end trace 0000000000000000 ]--- sparc64: [ 1.876345] ------------[ cut here ]------------ [ 1.876852] WARNING: CPU: 0 PID: 1 at mm/memblock.c:352 memblock_remove_region+0x78/0xb4 [ 1.877912] Modules linked in: [ 1.880783] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc3+ #1 [ 1.881473] Call Trace: [ 1.881935] [<0000000000467c30>] __warn+0xf0/0x1a0 [ 1.882515] [<0000000000467d98>] warn_slowpath_fmt+0xb8/0x100 [ 1.883128] [<0000000001b8d83c>] memblock_remove_region+0x78/0xb4 [ 1.883564] [<0000000001b6df50>] memblock_discard+0x88/0x108 [ 1.883987] [<0000000001b6abc0>] page_alloc_init_late+0xc/0x94 [ 1.884594] [<0000000001b56e44>] kernel_init_freeable+0xcc/0x228 [ 1.885181] [<0000000000f673c4>] kernel_init+0x18/0x134 [ 1.885612] [<00000000004060e8>] ret_from_fork+0x1c/0x2c [ 1.886035] [<0000000000000000>] 0x0 [ 1.886697] irq event stamp: 1013 [ 1.887293] hardirqs last enabled at (1021): [<00000000004e1534>] __up_console_sem+0x74/0xa0 [ 1.887786] hardirqs last disabled at (1028): [<00000000004e1510>] __up_console_sem+0x50/0xa0 [ 1.888273] softirqs last enabled at (1004): [<0000000000f76ae4>] __do_softirq+0x4c4/0x5a0 [ 1.888760] softirqs last disabled at (997): [<000000000042b94c>] do_softirq_own_stack+0x2c/0x40 [ 1.889610] ---[ end trace 0000000000000000 ]--- Guenter --- # bad: [57012c57536f8814dec92e74197ee96c3498d24e] Merge tag 'net-6.5-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net # good: [18b44bc5a67275641fb26f2c54ba7eef80ac5950] ovl: Always reevaluate the file signature for IMA git bisect start 'HEAD' '18b44bc5a672' # good: [6c58c8816abb7b93b21fa3b1d0c1726402e5e568] net/sched: mqprio: Add length check for TCA_MQPRIO_{MAX/MIN}_RATE64 git bisect good 6c58c8816abb7b93b21fa3b1d0c1726402e5e568 # bad: [379e66711b33f9fdc0513daee6cf3dd8d2f6f435] Merge tag 'fixes-2023-07-27' of git://git.kernel.org/pub/scm/linux/kernel/git/rppt/memblock git bisect bad 379e66711b33f9fdc0513daee6cf3dd8d2f6f435 # good: [c21733754cd6ecbca346f2adf9b17d4cfa50504f] platform/x86: huawei-wmi: Silence ambient light sensor git bisect good c21733754cd6ecbca346f2adf9b17d4cfa50504f # good: [536bb492d39bb6c080c92f31e8a55fe9934f452b] ksmbd: fix out of bounds in init_smb2_rsp_hdr() git bisect good 536bb492d39bb6c080c92f31e8a55fe9934f452b # good: [0a8db05b571ad5b8d5c8774a004c0424260a90bd] Merge tag 'platform-drivers-x86-v6.5-3' of git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86 git bisect good 0a8db05b571ad5b8d5c8774a004c0424260a90bd # bad: [9e46e4dcd9d6cd88342b028dbfa5f4fb7483d39c] mm,memblock: reset memblock.reserved to system init state to prevent UAF git bisect bad 9e46e4dcd9d6cd88342b028dbfa5f4fb7483d39c # first bad commit: [9e46e4dcd9d6cd88342b028dbfa5f4fb7483d39c] mm,memblock: reset memblock.reserved to system init state to prevent UAF
On Fri, Jul 28, 2023 at 09:09:09AM -0700, Guenter Roeck wrote: > Hi, > > On Wed, Jul 19, 2023 at 03:41:37PM -0400, Rik van Riel wrote: > > The memblock_discard function frees the memblock.reserved.regions > > array, which is good. > > > > However, if a subsequent memblock_free (or memblock_phys_free) comes > > in later, from for example ima_free_kexec_buffer, that will result in > > a use after free bug in memblock_isolate_range. > > > > When running a kernel with CONFIG_KASAN enabled, this will cause a > > kernel panic very early in boot. Without CONFIG_KASAN, there is > > a chance that memblock_isolate_range might scribble on memory > > that is now in use by somebody else. > > > > Avoid those issues by making sure that memblock_discard points > > memblock.reserved.regions back at the static buffer. > > > > If memblock_discard is called while there is still memory > > in the memblock.reserved type, that will print a warning > > in memblock_remove_region. > > > > Signed-off-by: Rik van Riel <riel@surriel.com> > > This patch results in the following WARNING backtrace when booting sparc > or sparc64 images in qemu. Bisect log is attached. > Follow-up: On sparc64, this patch also results in the following backtrace. [ 2.931808] BUG: scheduling while atomic: swapper/0/1/0x00000002 [ 2.932865] no locks held by swapper/0/1. [ 2.933722] Modules linked in: [ 2.934627] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.5.0-rc3+ #1 [ 2.935604] Call Trace: [ 2.936315] [<00000000004a0610>] __schedule_bug+0x70/0x80 [ 2.937174] [<0000000000f68f50>] switch_to_pc+0x598/0x8e8 [ 2.937999] [<0000000000f69300>] schedule+0x60/0xe0 [ 2.938811] [<0000000000f72d2c>] schedule_timeout+0x10c/0x1c0 [ 2.939668] [<0000000000f69be0>] __wait_for_common+0xa0/0x1a0 [ 2.940510] [<0000000000f69d98>] wait_for_completion_killable+0x18/0x40 [ 2.941402] [<0000000000494dec>] __kthread_create_on_node+0xac/0x120 [ 2.942259] [<0000000000494e80>] kthread_create_on_node+0x20/0x40 [ 2.943023] [<0000000001b81348>] devtmpfs_init+0xb4/0x140 [ 2.943777] [<0000000001b81068>] driver_init+0x10/0x60 [ 2.944528] [<0000000001b56e4c>] kernel_init_freeable+0xd4/0x228 [ 2.945300] [<0000000000f67404>] kernel_init+0x18/0x134 [ 2.946026] [<00000000004060e8>] ret_from_fork+0x1c/0x2c [ 2.946757] [<0000000000000000>] 0x0 [ 2.959537] devtmpfs: initialized While that seemed unlikely (and I don't claim to understand it), I ran bisect separately and confirmed that both tracebacks are gone after reverting this patch. Guenter
diff --git a/mm/memblock.c b/mm/memblock.c index 3feafea06ab2..068289a46903 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -374,6 +374,10 @@ void __init memblock_discard(void) kfree(memblock.reserved.regions); else memblock_free_late(addr, size); + /* Reset to prevent UAF from stray frees. */ + memblock.reserved.regions = memblock_reserved_init_regions; + memblock.reserved.cnt = 1; + memblock_remove_region(&memblock.reserved, 0); } if (memblock.memory.regions != memblock_memory_init_regions) {