| 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
| ||
|
Date: Fri, 26 Feb 2016 09:37:41 +0100
From: Borislav Petkov <bp@...e.de>
To: Alexander Kuleshov <kuleshovmail@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
Joerg Roedel <jroedel@...e.de>, Dave Young <dyoung@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Baoquan He <bhe@...hat.com>, Mark Salter <msalter@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7] x86/setup: get ramdisk parameters only once
On Fri, Feb 26, 2016 at 01:31:45PM +0600, Alexander Kuleshov wrote:
> arch/x86/kernel/setup.c | 109 ++++++++++++++++++++++++++----------------------
> 1 file changed, 60 insertions(+), 49 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d3d80e6..449b4da 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -169,6 +169,15 @@ static struct resource bss_resource = {
> .flags = IORESOURCE_BUSY | IORESOURCE_MEM
> };
>
> +/*
> + * ramdisk setup
> + */
No need for that comment - the struct naming and members should be
sufficient.
> +struct ramdisk {
> + u64 start_addr; /* ramdisk load address */
> + u64 end_addr; /* ramdisk end address */
> + u64 size; /* ramdisk size */
> + bool reserve_ramdisk; /* do initrd provided by bootloader */
> +};
>
> #ifdef CONFIG_X86_32
> /* cpu data as detected by the assembly code in head.S */
> @@ -318,90 +327,84 @@ static u64 __init get_ramdisk_size(void)
> return ramdisk_size;
> }
>
> -static void __init relocate_initrd(void)
> +static void __init relocate_initrd(struct ramdisk *ramdisk)
> {
> - /* Assume only end is not page aligned */
> - u64 ramdisk_image = get_ramdisk_image();
> - u64 ramdisk_size = get_ramdisk_size();
> - u64 area_size = PAGE_ALIGN(ramdisk_size);
> -
> /* We need to move the initrd down into directly mapped mem */
> relocated_ramdisk = memblock_find_in_range(0, PFN_PHYS(max_pfn_mapped),
> - area_size, PAGE_SIZE);
> + ramdisk->size, PAGE_SIZE);
>
> if (!relocated_ramdisk)
> panic("Cannot find place for new RAMDISK of size %lld\n",
> - ramdisk_size);
> + ramdisk->size);
>
> /* Note: this includes all the mem currently occupied by
> the initrd, we rely on that fact to keep the data intact. */
> - memblock_reserve(relocated_ramdisk, area_size);
> + memblock_reserve(relocated_ramdisk, ramdisk->size);
> initrd_start = relocated_ramdisk + PAGE_OFFSET;
> - initrd_end = initrd_start + ramdisk_size;
> + initrd_end = initrd_start + ramdisk->size;
> printk(KERN_INFO "Allocated new RAMDISK: [mem %#010llx-%#010llx]\n",
I think all those printks talking about ramdisk should be
printk(KERN_INFO "RAMDISK: ..."
for easier grepping of dmesg about ramdisk-specific messages. This
should be another patch though.
> - relocated_ramdisk, relocated_ramdisk + ramdisk_size - 1);
> + relocated_ramdisk, relocated_ramdisk + ramdisk->size - 1);
>
> - copy_from_early_mem((void *)initrd_start, ramdisk_image, ramdisk_size);
> + copy_from_early_mem((void *)initrd_start, ramdisk->start_addr, ramdisk->size);
>
> printk(KERN_INFO "Move RAMDISK from [mem %#010llx-%#010llx] to"
> " [mem %#010llx-%#010llx]\n",
> - ramdisk_image, ramdisk_image + ramdisk_size - 1,
> - relocated_ramdisk, relocated_ramdisk + ramdisk_size - 1);
> + ramdisk->start_addr, ramdisk->start_addr + ramdisk->size - 1,
> + relocated_ramdisk, relocated_ramdisk + ramdisk->size - 1);
> }
> -
That \n should not have been deleted here.
> -static void __init early_reserve_initrd(void)
> +static void __init early_reserve_initrd(struct ramdisk *ramdisk)
> {
> - /* Assume only end is not page aligned */
> - u64 ramdisk_image = get_ramdisk_image();
> - u64 ramdisk_size = get_ramdisk_size();
> - u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size);
> -
> - if (!boot_params.hdr.type_of_loader ||
> - !ramdisk_image || !ramdisk_size)
> - return; /* No initrd provided by bootloader */
> -
> - memblock_reserve(ramdisk_image, ramdisk_end - ramdisk_image);
> + if (!boot_params.hdr.type_of_loader || !ramdisk->start_addr || !ramdisk->size)
> + ramdisk->reserve_ramdisk = false; /* No initrd provided by bootloader */
> + else
> + memblock_reserve(ramdisk->start_addr, ramdisk->size);
> }
Make this one even more readable:
static void __init early_reserve_initrd(struct ramdisk *ramdisk)
{
/* No initrd provided by bootloader */
if (!boot_params.hdr.type_of_loader ||
!ramdisk->start_addr ||
!ramdisk->size)
ramdisk->reserve_ramdisk = false;
else
memblock_reserve(ramdisk->start_addr, ramdisk->size);
}
It also has some whitespace damage.
> -static void __init reserve_initrd(void)
> +
> +static void __init reserve_initrd(struct ramdisk *ramdisk)
> {
> - /* Assume only end is not page aligned */
> - u64 ramdisk_image = get_ramdisk_image();
> - u64 ramdisk_size = get_ramdisk_size();
> - u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size);
> u64 mapped_size;
>
> - if (!boot_params.hdr.type_of_loader ||
> - !ramdisk_image || !ramdisk_size)
> - return; /* No initrd provided by bootloader */
> + if (!ramdisk->reserve_ramdisk)
> + return;
>
> initrd_start = 0;
>
> mapped_size = memblock_mem_size(max_pfn_mapped);
> - if (ramdisk_size >= (mapped_size>>1))
> + if (ramdisk->size >= (mapped_size>>1))
Space that shift out:
... mapped_size >> 1))
> panic("initrd too large to handle, "
> "disabling initrd (%lld needed, %lld available)\n",
The string in this panic() call should be a single line only for easier
grepping. With another patch though.
> - ramdisk_size, mapped_size>>1);
> + ramdisk->size, mapped_size>>1);
Space that shift out too.
>
> - printk(KERN_INFO "RAMDISK: [mem %#010llx-%#010llx]\n", ramdisk_image,
> - ramdisk_end - 1);
> + printk(KERN_INFO "RAMDISK: [mem %#010llx-%#010llx]\n",
> + ramdisk->start_addr, ramdisk->end_addr - 1);
>
> - if (pfn_range_is_mapped(PFN_DOWN(ramdisk_image),
> - PFN_DOWN(ramdisk_end))) {
> + if (pfn_range_is_mapped(PFN_DOWN(ramdisk->start_addr),
> + PFN_DOWN(ramdisk->end_addr))) {
> /* All are mapped, easy case */
> - initrd_start = ramdisk_image + PAGE_OFFSET;
> - initrd_end = initrd_start + ramdisk_size;
> + initrd_start = ramdisk->start_addr + PAGE_OFFSET;
> + initrd_end = initrd_start + ramdisk->size;
> return;
> }
>
> - relocate_initrd();
> + relocate_initrd(ramdisk);
>
> - memblock_free(ramdisk_image, ramdisk_end - ramdisk_image);
> + memblock_free(ramdisk->start_addr,
> + ramdisk->end_addr - ramdisk->start_addr);
Arg alignment should start at the function's opening brace.
> }
> #else
> -static void __init early_reserve_initrd(void)
> +static u64 __init get_ramdisk_image(void)
> {
> + return 0;
> }
> -static void __init reserve_initrd(void)
> +static u64 __init get_ramdisk_size(void)
> +{
> + return 0;
> +}
> +static void __init early_reserve_initrd(struct ramdisk *ramdisk)
> +{
> +
> +}
> +static void __init reserve_initrd(struct ramdisk *ramdisk)
> {
> }
> #endif /* CONFIG_BLK_DEV_INITRD */
> @@ -844,13 +847,21 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
> *
> * Note: On x86_64, fixmaps are ready for use even before this is called.
> */
> -
> void __init setup_arch(char **cmdline_p)
> {
> + struct ramdisk ramdisk = {
> + .start_addr = get_ramdisk_image(),
> + .size = PAGE_ALIGN(get_ramdisk_size()),
> + .reserve_ramdisk = true
> + };
More readable:
struct ramdisk ramdisk = {
.start_addr = get_ramdisk_image(),
.size = PAGE_ALIGN(get_ramdisk_size()),
.reserve_ramdisk = true,
};
> +
> + /* Assume end is not page aligned */
> + ramdisk.end_addr = PAGE_ALIGN(ramdisk.start_addr + ramdisk.size);
> +
> memblock_reserve(__pa_symbol(_text),
> (unsigned long)__bss_stop - (unsigned long)_text);
>
> - early_reserve_initrd();
> + early_reserve_initrd(&ramdisk);
>
> /*
> * At this point everything still needed from the boot loader
> @@ -1135,7 +1146,7 @@ void __init setup_arch(char **cmdline_p)
> /* Allocate bigger log buffer */
> setup_log_buf(1);
>
> - reserve_initrd();
> + reserve_initrd(&ramdisk);
>
> #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
> acpi_initrd_override((void *)initrd_start, initrd_end - initrd_start);
> --
> 2.7.2.335.g3476f70
>
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
Powered by blists - more mailing lists