| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4BBE0E18.8050903@goop.org>
Date: Thu, 08 Apr 2010 10:10:48 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Yinghai <yinghai.lu@...cle.com>
CC: Liang Li <liang.li@...driver.com>,
Rusty Russell <rusty@...tcorp.com.au>,
akpm@...ux-foundation.org, hpa@...or.com, mingo@...e.hu,
tglx@...utronix.de, wangchen@...fujitsu.com,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: + x86-fix-handling-of-the-reservetop-boot-option.patch added
to -mm tree
On 04/07/2010 11:57 PM, Yinghai wrote:
> On 04/07/2010 09:59 PM, Liang Li wrote:
>
>> On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
>>
>>> On 04/07/2010 06:53 PM, Liang Li wrote:
>>>
>>>> Does this similar modification like this is more preferred?
>>>>
>>>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>>>> index a1dcfa3..30a3e97 100644
>>>> --- a/arch/x86/include/asm/io.h
>>>> +++ b/arch/x86/include/asm/io.h
>>>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>>>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>>>> unsigned long size);
>>>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>>>> +extern void fixup_early_ioremap(void);
>>>>
>>>> #define IO_SPACE_LIMIT 0xffff
>>>>
>>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>>> index ea82ef0..fe06296 100644
>>>> --- a/arch/x86/mm/ioremap.c
>>>> +++ b/arch/x86/mm/ioremap.c
>>>> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>>>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>>>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>>>
>>>> +void __init fixup_early_ioremap(void)
>>>> +{
>>>> + int i;
>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>>>> + if (prev_map[i])
>>>> + break;
>>>> + }
>>>> +
>>>> + if (i == FIX_BTMAPS_SLOTS)
>>>>
>>> <
>>>
>>>> + WARN_ON(1);
>>>>
>>> BUG_ON()
>>>
>>>> +
>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
>>>> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
>>>> +
>>>>
>>> need to clear the old PMD, and set new PMD.
>>>
>>> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
>>>
>> Call early_ioremap_init will do the update PMD work. So the preferred
>> patch would be:
>> ---------------
>> From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
>> From: Liang Li <liang.li@...driver.com>
>> Date: Mon, 22 Mar 2010 18:38:14 +0800
>> Subject: [PATCH] x86: let 'reservetop' functioning right
>>
>> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
>> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>>
>> The root cause of boot failure problem is the value of 'slot_virt[i]'
>> was initialized in setup_arch->early_ioremap_init. But later in
>> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
>> when 'reservetop=0xbadc0de' being specified.
>>
>> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
>> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>>
>> Signed-off-by: Liang Li <liang.li@...driver.com>
>> Cc: Wang Chen <wangchen@...fujitsu.com>
>> Cc: Ingo Molnar <mingo@...e.hu>
>> Cc: Thomas Gleixner <tglx@...utronix.de>
>> Cc: "H. Peter Anvin" <hpa@...or.com>
>> Cc: Yinghai Lu <yinghai@...nel.org>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> ---
>> arch/x86/include/asm/io.h | 1 +
>> arch/x86/mm/ioremap.c | 15 +++++++++++++++
>> arch/x86/mm/pgtable.c | 2 ++
>> 3 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>> index a1dcfa3..30a3e97 100644
>> --- a/arch/x86/include/asm/io.h
>> +++ b/arch/x86/include/asm/io.h
>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>> unsigned long size);
>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>> +extern void fixup_early_ioremap(void);
>>
>> #define IO_SPACE_LIMIT 0xffff
>>
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index 5eb1ba7..e4ab706 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>
>> +void __init fixup_early_ioremap(void)
>> +{
>> + int i;
>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>> + if (prev_map[i])
>> + break;
>> + }
>> +
>> + if (i < FIX_BTMAPS_SLOTS)
>> + BUG_ON(1);
>> +
>> + early_ioremap_init();
>> + return;
>> +}
>> +
>> static int __init check_early_ioremap_leak(void)
>> {
>> int count = 0;
>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>> index 5c4ee42..ea4d54c 100644
>> --- a/arch/x86/mm/pgtable.c
>> +++ b/arch/x86/mm/pgtable.c
>> @@ -4,6 +4,7 @@
>> #include <asm/pgtable.h>
>> #include <asm/tlb.h>
>> #include <asm/fixmap.h>
>> +#include <asm/io.h>
>>
>> #define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
>>
>> @@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
>> printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
>> (int)-reserve);
>> __FIXADDR_TOP = -reserve - PAGE_SIZE;
>> + fixup_early_ioremap();
>> #endif
>> }
>>
>> -----------
>> Acceptable?
>>
>>
> good to me.
>
> may need to ask xen/lguest/vmi related to check that too.
>
> arch/x86/kernel/vmi_32.c: reserve_top_address(-vmi_rom->virtual_top);
> arch/x86/lguest/boot.c: reserve_top_address(lguest_data.reserve_mem);
> arch/x86/mm/pgtable_32.c: reserve_top_address(address);
> arch/x86/xen/mmu.c: reserve_top_address(-top);
>
This looks troublesome for us. We're calling reserve_top_address() very
early - before start_kernel - to make sure the address space for the
hypervisor has been reserved. Calling fixup_early_ioremap() will
probably fail horribly.
Can you make it so that it only calls fixup_early_ioremap() if ioremap
init has already happened?
Also, do we actually need reservetop= any more. It looks like Zach
added it for VMI, but VMI has been deprecated. Are there any other use
cases?
Thanks,
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists