From: "Björn Töpel" <bjorn@kernel.org>
To: Nam Cao <namcao@linutronix.de>, Mike Rapoport <rppt@kernel.org>,
Andreas Dilger <adilger@dilger.ca>,
linux-riscv@lists.infradead.org,
Thomas Gleixner <tglx@linutronix.de>,
Andrew Morton <akpm@linux-foundation.org>,
"ndesaulniers @ google . com" <ndesaulniers@google.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Tejun Heo <tj@kernel.org>,
Krister Johansen <kjlx@templeofstupid.com>,
Changbin Du <changbin.du@huawei.com>,
Arnd Bergmann <arnd@arndb.de>,
Geert Uytterhoeven <geert+renesas@glider.be>,
linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Subject: Re: [PATCH] init: fix allocated page overlapping with PTR_ERR
Date: Thu, 18 Apr 2024 14:41:38 +0200 [thread overview]
Message-ID: <87edb2sv0d.fsf@all.your.base.are.belong.to.us> (raw)
In-Reply-To: <20240418131238.636bee2c@namcao>
Nam Cao <namcao@linutronix.de> writes:
> On 2024-04-18 Nam Cao wrote:
>> There is nothing preventing kernel memory allocators from allocating a
>> page that overlaps with PTR_ERR(), except for architecture-specific
>> code that setup memblock.
>>
>> It was discovered that RISCV architecture doesn't setup memblock
>> corectly, leading to a page overlapping with PTR_ERR() being allocated,
>> and subsequently crashing the kernel (link in Close: )
>>
>> The reported crash has nothing to do with PTR_ERR(): the last page
>> (at address 0xfffff000) being allocated leads to an unexpected
>> arithmetic overflow in ext4; but still, this page shouldn't be
>> allocated in the first place.
>>
>> Because PTR_ERR() is an architecture-independent thing, we shouldn't
>> ask every single architecture to set this up. There may be other
>> architectures beside RISCV that have the same problem.
>>
>> Fix this one and for all by reserving the physical memory page that
>> may be mapped to the last virtual memory page as part of low memory.
>>
>> Unfortunately, this means if there is actual memory at this reserved
>> location, that memory will become inaccessible. However, if this page
>> is not reserved, it can only be accessed as high memory, so this
>> doesn't matter if high memory is not supported. Even if high memory is
>> supported, it is still only one page.
>>
>> Closes: https://lore.kernel.org/linux-riscv/878r1ibpdn.fsf@all.your.base.are.belong.to.us
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> Cc: <stable@vger.kernel.org> # all versions
>
> Sorry, forgot to add:
> Reported-by: Björn Töpel <bjorn@kernel.org>
Hmm, can't we get rid of the whole check in arch/riscv/mm/init.c for
32b?
--8<--
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index fe8e159394d8..1e91d5728887 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -196,7 +196,6 @@ early_param("mem", early_mem);
static void __init setup_bootmem(void)
{
phys_addr_t vmlinux_end = __pa_symbol(&_end);
- phys_addr_t max_mapped_addr;
phys_addr_t phys_ram_end, vmlinux_start;
if (IS_ENABLED(CONFIG_XIP_KERNEL))
@@ -234,21 +233,6 @@ static void __init setup_bootmem(void)
if (IS_ENABLED(CONFIG_64BIT))
kernel_map.va_pa_offset = PAGE_OFFSET - phys_ram_base;
- /*
- * memblock allocator is not aware of the fact that last 4K bytes of
- * the addressable memory can not be mapped because of IS_ERR_VALUE
- * macro. Make sure that last 4k bytes are not usable by memblock
- * if end of dram is equal to maximum addressable memory. For 64-bit
- * kernel, this problem can't happen here as the end of the virtual
- * address space is occupied by the kernel mapping then this check must
- * be done as soon as the kernel mapping base address is determined.
- */
- if (!IS_ENABLED(CONFIG_64BIT)) {
- max_mapped_addr = __pa(~(ulong)0);
- if (max_mapped_addr == (phys_ram_end - 1))
- memblock_set_current_limit(max_mapped_addr - 4096);
- }
-
min_low_pfn = PFN_UP(phys_ram_base);
max_low_pfn = max_pfn = PFN_DOWN(phys_ram_end);
high_memory = (void *)(__va(PFN_PHYS(max_low_pfn)));
--8<--
Mike hints that's *not* the case
(https://lore.kernel.org/linux-riscv/ZiAkRMUfiPDUGPdL@kernel.org/).
memblock_reserve() should disallow allocation as well, no?
Thanks, and FWIW:
Tested-by: Björn Töpel <bjorn@rivosinc.com>
next prev parent reply other threads:[~2024-04-18 12:41 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-18 10:29 [PATCH] init: fix allocated page overlapping with PTR_ERR Nam Cao
2024-04-18 10:54 ` Mike Rapoport
2024-04-18 11:12 ` Nam Cao
2024-04-18 12:41 ` Björn Töpel [this message]
2024-04-18 13:01 ` Nam Cao
2024-04-18 13:07 ` Nam Cao
[not found] ` <CGME20240429125236eucas1p24219f2d332e0267794a2f87dea9f39c4@eucas1p2.samsung.com>
2024-04-29 12:52 ` Joel Granados
2024-04-30 7:31 ` Nam Cao
2024-04-30 8:37 ` Alexandre Ghiti
2024-04-30 13:35 ` Joel Granados
2024-04-30 15:42 ` Joel Granados
2024-05-10 6:35 ` Nam Cao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87edb2sv0d.fsf@all.your.base.are.belong.to.us \
--to=bjorn@kernel.org \
--cc=adilger@dilger.ca \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=changbin.du@huawei.com \
--cc=christophe.leroy@csgroup.eu \
--cc=geert+renesas@glider.be \
--cc=kjlx@templeofstupid.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mcgrof@kernel.org \
--cc=mingo@kernel.org \
--cc=namcao@linutronix.de \
--cc=ndesaulniers@google.com \
--cc=rppt@kernel.org \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).