* [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling
@ 2015-03-04 8:00 Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size Yinghai Lu
` (14 more replies)
0 siblings, 15 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
patch 1-7: are kasl related.
1. make ZO: arch/x86/boot/compressed/vmlinux data region is not
overwritten by final VO: vmlinux after decompress.
so could pass data from ZO to VO
2. create new ident mapping for kasl 64bit, so we can cover
above 4G random kernel base, also don't need to track pagetable
for 64bit bootloader (patched grub2 or kexec).
that will make mem_avoid handling simple.
patch 8-15: setup_data related.
Now we setup_data is reserved via memblock and e820 and different
handlers have different ways, and it is confusing.
1. SETUP_E820_EXT: is consumed early and will not copy or access again.
have memory wasted.
2. SETUP_EFI: is accessed via ioremap every time at early stage.
have memory wasted.
3. SETUP_DTB: is copied locally.
have memory wasted.
4. SETUP_PCI: is accessed via ioremap for every pci devices, even run-time.
5. SETUP_KASLR: is accessed early, will not copy or access again.
have memory wasted.
Also setup_data is exported to debugfs for debug purpose.
Here will convert to let every handler to decide how to handle it.
and will not reserve the setup_data generally, so will not
waste memory and also make memblock/e820 keep page aligned.
1. not touch E820 anymore.
2. copy SETUP_EFI to __initdata variable and access it without ioremap.
3. SETUP_DTB: reserver and copy to local and free.
4. SETUP_PCI: reverve localy and convert to list, to avoid keeping ioremap.
5. SETUP_KASLR: fix accessing kaslr_enabled accessing...
6. export SETUP_PCI via sysfs.
Yinghai Lu (15):
x86, kaslr: Use init_size instead of run_size
x86, boot: move ZO to end of buffer
x86, boot: keep data from ZO boot stage to VO kernel stage.
x86, kaslr: get kaslr_enabled back correctly
x86, kaslr: consolidate the mem_avoid filling
x86, boot: split kernel_ident_mapping_init into another file
x86, kaslr, 64bit: set new or extra ident_mapping
x86: Kill E820_RESERVED_KERN
x86, efi: copy SETUP_EFI data and access directly
x86, of: let add_dtb reserve by itself
x86, boot: Add add_pci handler for SETUP_PCI
x86: kill not used setup_data handling code
x86, pci: convert SETUP_PCI data to list
x86, boot: copy rom to kernel space
x86, pci: export SETUP_PCI data via sysfs
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/compressed/Makefile | 4 +-
arch/x86/boot/compressed/aslr.c | 55 ++++--
arch/x86/boot/compressed/head_32.S | 16 +-
arch/x86/boot/compressed/head_64.S | 17 +-
arch/x86/boot/compressed/misc.c | 15 +-
arch/x86/boot/compressed/misc_pgt.c | 96 ++++++++++
arch/x86/boot/compressed/mkpiggy.c | 12 +-
arch/x86/boot/compressed/vmlinux.lds.S | 2 +
arch/x86/boot/header.S | 7 +-
arch/x86/include/asm/boot.h | 13 ++
arch/x86/include/asm/efi.h | 2 +-
arch/x86/include/asm/page.h | 5 +
arch/x86/include/asm/pci.h | 4 +
arch/x86/include/asm/prom.h | 9 +-
arch/x86/include/uapi/asm/e820.h | 9 -
arch/x86/kernel/asm-offsets.c | 1 +
arch/x86/kernel/devicetree.c | 39 ++--
arch/x86/kernel/e820.c | 6 +-
arch/x86/kernel/kdebugfs.c | 142 ---------------
arch/x86/kernel/setup.c | 60 ++-----
arch/x86/kernel/tboot.c | 3 +-
arch/x86/mm/ident_map.c | 74 ++++++++
arch/x86/mm/init_64.c | 85 +--------
arch/x86/pci/common.c | 313 +++++++++++++++++++++++++++++++--
arch/x86/platform/efi/efi.c | 13 +-
arch/x86/platform/efi/efi_64.c | 13 +-
arch/x86/platform/efi/quirks.c | 23 +--
arch/x86/tools/calc_run_size.sh | 42 -----
29 files changed, 640 insertions(+), 442 deletions(-)
create mode 100644 arch/x86/boot/compressed/misc_pgt.c
create mode 100644 arch/x86/mm/ident_map.c
delete mode 100644 arch/x86/tools/calc_run_size.sh
--
1.8.4.5
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-06 13:55 ` Borislav Petkov
2015-03-04 8:00 ` [PATCH v2 02/15] x86, boot: move ZO to end of buffer Yinghai Lu
` (13 subsequent siblings)
14 siblings, 1 reply; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Josh Triplett, Kees Cook, Andrew Morton, Ard Biesheuvel,
Junjie Mao
commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
introduced one run_size for kaslr.
We do not need to have home grown run_size.
We should use real runtime size (include copy/decompress) aka init_size
Please check arch/x86/boot/header.S about init_size for detail.
Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Junjie Mao <eternal.n08@gmail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/boot/compressed/Makefile | 4 +---
arch/x86/boot/compressed/head_32.S | 5 ++---
arch/x86/boot/compressed/head_64.S | 5 +----
arch/x86/boot/compressed/misc.c | 15 +++++++-------
arch/x86/boot/compressed/mkpiggy.c | 9 ++------
arch/x86/tools/calc_run_size.sh | 42 --------------------------------------
6 files changed, 13 insertions(+), 67 deletions(-)
delete mode 100644 arch/x86/tools/calc_run_size.sh
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 0a291cd..70cc92c 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz
suffix-$(CONFIG_KERNEL_LZO) := lzo
suffix-$(CONFIG_KERNEL_LZ4) := lz4
-RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
- $(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
quiet_cmd_mkpiggy = MKPIGGY $@
- cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
+ cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
targets += piggy.S
$(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 1d7fbbc..cbed140 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -207,8 +207,7 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
/* push arguments for decompress_kernel: */
- pushl $z_run_size /* size of kernel with .bss and .brk */
- pushl $z_output_len /* decompressed length, end of relocs */
+ pushl $z_output_len /* decompressed length */
leal z_extract_offset_negative(%ebx), %ebp
pushl %ebp /* output address */
pushl $z_input_len /* input_len */
@@ -218,7 +217,7 @@ relocated:
pushl %eax /* heap area */
pushl %esi /* real mode pointer */
call decompress_kernel /* returns kernel location in %eax */
- addl $28, %esp
+ addl $24, %esp
/*
* Jump to the decompressed kernel.
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 6b1766c..2884e0c 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -402,16 +402,13 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
pushq %rsi /* Save the real mode argument */
- movq $z_run_size, %r9 /* size of kernel with .bss and .brk */
- pushq %r9
movq %rsi, %rdi /* real mode address */
leaq boot_heap(%rip), %rsi /* malloc area for uncompression */
leaq input_data(%rip), %rdx /* input_data */
movl $z_input_len, %ecx /* input_len */
movq %rbp, %r8 /* output target address */
- movq $z_output_len, %r9 /* decompressed length, end of relocs */
+ movq $z_output_len, %r9 /* decompressed length */
call decompress_kernel /* returns kernel location in %rax */
- popq %r9
popq %rsi
/*
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 5903089..51e9e54 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -370,10 +370,10 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
unsigned char *input_data,
unsigned long input_len,
unsigned char *output,
- unsigned long output_len,
- unsigned long run_size)
+ unsigned long output_len)
{
unsigned char *output_orig = output;
+ unsigned long init_size;
real_mode = rmode;
@@ -396,15 +396,14 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
free_mem_ptr = heap; /* Heap */
free_mem_end_ptr = heap + BOOT_HEAP_SIZE;
+ init_size = real_mode->hdr.init_size;
+
/*
- * The memory hole needed for the kernel is the larger of either
- * the entire decompressed kernel plus relocation table, or the
- * entire decompressed kernel plus .bss and .brk sections.
+ * The memory hole needed for the kernel is init_size for running
+ * and init_size is bigger than output_len always.
*/
output = choose_kernel_location(real_mode, input_data, input_len,
- output,
- output_len > run_size ? output_len
- : run_size);
+ output, init_size);
/* Validate memory location choices. */
if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index d8222f2..b669ab6 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -36,13 +36,11 @@ int main(int argc, char *argv[])
uint32_t olen;
long ilen;
unsigned long offs;
- unsigned long run_size;
FILE *f = NULL;
int retval = 1;
- if (argc < 3) {
- fprintf(stderr, "Usage: %s compressed_file run_size\n",
- argv[0]);
+ if (argc < 2) {
+ fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
goto bail;
}
@@ -76,7 +74,6 @@ int main(int argc, char *argv[])
offs += olen >> 12; /* Add 8 bytes for each 32K block */
offs += 64*1024 + 128; /* Add 64K + 128 bytes slack */
offs = (offs+4095) & ~4095; /* Round to a 4K boundary */
- run_size = atoi(argv[2]);
printf(".section \".rodata..compressed\",\"a\",@progbits\n");
printf(".globl z_input_len\n");
@@ -88,8 +85,6 @@ int main(int argc, char *argv[])
/* z_extract_offset_negative allows simplification of head_32.S */
printf(".globl z_extract_offset_negative\n");
printf("z_extract_offset_negative = -0x%lx\n", offs);
- printf(".globl z_run_size\n");
- printf("z_run_size = %lu\n", run_size);
printf(".globl input_data, input_data_end\n");
printf("input_data:\n");
diff --git a/arch/x86/tools/calc_run_size.sh b/arch/x86/tools/calc_run_size.sh
deleted file mode 100644
index 1a4c17b..0000000
--- a/arch/x86/tools/calc_run_size.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/bin/sh
-#
-# Calculate the amount of space needed to run the kernel, including room for
-# the .bss and .brk sections.
-#
-# Usage:
-# objdump -h a.out | sh calc_run_size.sh
-
-NUM='\([0-9a-fA-F]*[ \t]*\)'
-OUT=$(sed -n 's/^[ \t0-9]*.b[sr][sk][ \t]*'"$NUM$NUM$NUM$NUM"'.*/\1\4/p')
-if [ -z "$OUT" ] ; then
- echo "Never found .bss or .brk file offset" >&2
- exit 1
-fi
-
-OUT=$(echo ${OUT# })
-sizeA=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-offsetA=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-sizeB=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-offsetB=$(printf "%d" 0x${OUT%% *})
-
-run_size=$(( $offsetA + $sizeA + $sizeB ))
-
-# BFD linker shows the same file offset in ELF.
-if [ "$offsetA" -ne "$offsetB" ] ; then
- # Gold linker shows them as consecutive.
- endB=$(( $offsetB + $sizeB ))
- if [ "$endB" != "$run_size" ] ; then
- printf "sizeA: 0x%x\n" $sizeA >&2
- printf "offsetA: 0x%x\n" $offsetA >&2
- printf "sizeB: 0x%x\n" $sizeB >&2
- printf "offsetB: 0x%x\n" $offsetB >&2
- echo ".bss and .brk are non-contiguous" >&2
- exit 1
- fi
-fi
-
-printf "%d\n" $run_size
-exit 0
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 02/15] x86, boot: move ZO to end of buffer
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-06 13:58 ` Borislav Petkov
2015-03-04 8:00 ` [PATCH v2 03/15] x86, boot: keep data from ZO boot stage to VO kernel stage Yinghai Lu
` (12 subsequent siblings)
14 siblings, 1 reply; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Kees Cook
bp found data from boot stage can not be used kernel stage.
Actually those data area is overlapped with VO kernel bss stage, and clear_bss()
clear them before code in arch/x86/kernel/setup.c access them.
To make the data survive that later, we should avoid the overlapping.
At first move compressed kernel close the end of buffer instead of middle
of the buffer.
Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/boot/compressed/head_32.S | 11 +++++++++--
arch/x86/boot/compressed/head_64.S | 8 ++++++--
arch/x86/boot/compressed/mkpiggy.c | 3 ---
arch/x86/kernel/asm-offsets.c | 1 +
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index cbed140..a9b56f1 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -147,7 +147,9 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movl BP_init_size(%esi), %eax
+ subl $_end, %eax
+ addl %eax, %ebx
/* Set up the stack */
leal boot_stack_end(%ebx), %esp
@@ -208,8 +210,13 @@ relocated:
*/
/* push arguments for decompress_kernel: */
pushl $z_output_len /* decompressed length */
- leal z_extract_offset_negative(%ebx), %ebp
+
+ movl BP_init_size(%esi), %eax
+ subl $_end, %eax
+ movl %ebx, %ebp
+ subl %eax, %ebp
pushl %ebp /* output address */
+
pushl $z_input_len /* input_len */
leal input_data(%ebx), %eax
pushl %eax /* input_data */
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 2884e0c..69015b5 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -101,7 +101,9 @@ ENTRY(startup_32)
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movl BP_init_size(%esi), %eax
+ subl $_end, %eax
+ addl %eax, %ebx
/*
* Prepare for entering 64 bit mode
@@ -329,7 +331,9 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- leaq z_extract_offset(%rbp), %rbx
+ movl BP_init_size(%rsi), %ebx
+ subl $_end, %ebx
+ addq %rbp, %rbx
/* Set up the stack */
leaq boot_stack_end(%rbx), %rsp
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index b669ab6..652879b 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -82,9 +82,6 @@ int main(int argc, char *argv[])
printf("z_output_len = %lu\n", (unsigned long)olen);
printf(".globl z_extract_offset\n");
printf("z_extract_offset = 0x%lx\n", offs);
- /* z_extract_offset_negative allows simplification of head_32.S */
- printf(".globl z_extract_offset_negative\n");
- printf("z_extract_offset_negative = -0x%lx\n", offs);
printf(".globl input_data, input_data_end\n");
printf("input_data:\n");
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 9f6b934..0e8e4f7 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -66,6 +66,7 @@ void common(void) {
OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
OFFSET(BP_version, boot_params, hdr.version);
OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
+ OFFSET(BP_init_size, boot_params, hdr.init_size);
OFFSET(BP_pref_address, boot_params, hdr.pref_address);
OFFSET(BP_code32_start, boot_params, hdr.code32_start);
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 03/15] x86, boot: keep data from ZO boot stage to VO kernel stage.
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 02/15] x86, boot: move ZO to end of buffer Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
` (11 subsequent siblings)
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Kees Cook
bp found data from boot stage can not be used kernel stage.
Actually those data area is overlapped with kernel bss stage, and clear_bss()
clear them before code in arch/x86/kernel/setup.c access them.
To make the data survive that later, we should avoid the overlapping.
We already move compressed kernel close the end of buffer instead of middle of
buffer. But there will have overlapping beween VO BRK with ZO data/bss range.
Extend init_size so no one from kernel bss and brk will touch the data
region of boot/compressed/misc.c
The increase is from _rodata to _end in arch/x86/boot/compressed/vmlinux.
-v2: add init_size in arch/x86/boot/header.S instead of BRK.
-v3: split code that move Zo to end of buffer to another patch.
Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/compressed/vmlinux.lds.S | 2 ++
| 7 +++++--
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 57bbf2f..863ef25 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -86,7 +86,7 @@ targets += voffset.h
$(obj)/voffset.h: vmlinux FORCE
$(call if_changed,voffset)
-sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|z_.*\)$$/\#define ZO_\2 0x\1/p'
+sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|_rodata\|z_.*\)$$/\#define ZO_\2 0x\1/p'
quiet_cmd_zoffset = ZOFFSET $@
cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 34d047c..805d6ad 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -35,6 +35,7 @@ SECTIONS
*(.text.*)
_etext = . ;
}
+ . = ALIGN(PAGE_SIZE); /* keep ADDON_ZO_SIZE page aligned */
.rodata : {
_rodata = . ;
*(.rodata) /* read-only data */
@@ -70,5 +71,6 @@ SECTIONS
_epgtable = . ;
}
#endif
+ . = ALIGN(PAGE_SIZE); /* keep ADDON_ZO_SIZE page aligned */
_end = .;
}
--git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 16ef025..44359bd 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -440,12 +440,15 @@ setup_data: .quad 0 # 64-bit physical pointer to
pref_address: .quad LOAD_PHYSICAL_ADDR # preferred load addr
+# don't overlap data area of ZO with VO
+#define ADDON_ZO_SIZE (ZO__end - ZO__rodata)
+
#define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
#define VO_INIT_SIZE (VO__end - VO__text)
#if ZO_INIT_SIZE > VO_INIT_SIZE
-#define INIT_SIZE ZO_INIT_SIZE
+#define INIT_SIZE (ZO_INIT_SIZE + ADDON_ZO_SIZE)
#else
-#define INIT_SIZE VO_INIT_SIZE
+#define INIT_SIZE (VO_INIT_SIZE + ADDON_ZO_SIZE)
#endif
init_size: .long INIT_SIZE # kernel initialization size
handover_offset: .long 0 # Filled in by build.c
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Kees Cook
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
is using address as value for kaslr_enabled.
That will random kaslr_enabled get that set or cleared.
Will have problem for system really have kaslr enabled.
-v2: update changelog.
Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Jiri Kosina <jkosina@suse.cz>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/kernel/setup.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ static void __init reserve_initrd(void)
static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
{
- kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Yinghai Lu, Kees Cook
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
is using address as value for kaslr_enabled.
That will random kaslr_enabled get that set or cleared.
Will have problem for system really have kaslr enabled.
-v2: update changelog.
Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
Acked-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/x86/kernel/setup.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ static void __init reserve_initrd(void)
static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
{
- kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 05/15] x86, kaslr: consolidate the mem_avoid filling
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Kees Cook
Now ZO sit end of the buffer, we can find out where is ZO text
and data/bss etc.
[input, input+input_size) is copied compressed kernel, not the whole ZO.
[output, output+init_size) is the buffer for VO.
[input+input_size, output+init_size) is [_text, _end) for ZO.
that will be first range in mem_avoid. we don't need to guess that anymore.
That area aleady include heap and stack. So we don't need to put
them into mem_avoid array.
Also we need to put boot_params into the mem_avoid too.
as with 64bit boot loader will put them above 4G.
Also change output_size to init_size, as we pass that instead already.
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/boot/compressed/aslr.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 7083c16..e8486a5 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -116,7 +116,7 @@ struct mem_vector {
unsigned long size;
};
-#define MEM_AVOID_MAX 5
+#define MEM_AVOID_MAX 4
static struct mem_vector mem_avoid[MEM_AVOID_MAX];
static bool mem_contains(struct mem_vector *region, struct mem_vector *item)
@@ -142,7 +142,7 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
}
static void mem_avoid_init(unsigned long input, unsigned long input_size,
- unsigned long output, unsigned long output_size)
+ unsigned long output, unsigned long init_size)
{
u64 initrd_start, initrd_size;
u64 cmd_line, cmd_line_size;
@@ -151,10 +151,13 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
/*
* Avoid the region that is unsafe to overlap during
- * decompression (see calculations at top of misc.c).
+ * decompression.
+ * As we already move compressed to the end of buffer,
+ * [input+input_size, output+init_size) has [_text, _end)
+ * for arch/x86/boot/compressed/vmlinux.
*/
- unsafe_len = (output_size >> 12) + 32768 + 18;
- unsafe = (unsigned long)input + input_size - unsafe_len;
+ unsafe_len = output + init_size - (input + input_size);
+ unsafe = (unsigned long)input + input_size;
mem_avoid[0].start = unsafe;
mem_avoid[0].size = unsafe_len;
@@ -176,13 +179,9 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
mem_avoid[2].start = cmd_line;
mem_avoid[2].size = cmd_line_size;
- /* Avoid heap memory. */
- mem_avoid[3].start = (unsigned long)free_mem_ptr;
- mem_avoid[3].size = BOOT_HEAP_SIZE;
-
- /* Avoid stack memory. */
- mem_avoid[4].start = (unsigned long)free_mem_end_ptr;
- mem_avoid[4].size = BOOT_STACK_SIZE;
+ /* Avoid params */
+ mem_avoid[3].start = (unsigned long)real_mode;
+ mem_avoid[3].size = sizeof(*real_mode);
}
/* Does this memory vector overlap a known avoided area? */
@@ -327,7 +326,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
unsigned char *input,
unsigned long input_size,
unsigned char *output,
- unsigned long output_size)
+ unsigned long init_size)
{
unsigned long choice = (unsigned long)output;
unsigned long random;
@@ -349,10 +348,10 @@ unsigned char *choose_kernel_location(struct boot_params *params,
/* Record the various known unsafe memory ranges. */
mem_avoid_init((unsigned long)input, input_size,
- (unsigned long)output, output_size);
+ (unsigned long)output, init_size);
/* Walk e820 and find a random address. */
- random = find_random_addr(choice, output_size);
+ random = find_random_addr(choice, init_size);
if (!random) {
debug_putstr("KASLR could not find suitable E820 region...\n");
goto out;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 05/15] x86, kaslr: consolidate the mem_avoid filling
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Yinghai Lu, Kees Cook
Now ZO sit end of the buffer, we can find out where is ZO text
and data/bss etc.
[input, input+input_size) is copied compressed kernel, not the whole ZO.
[output, output+init_size) is the buffer for VO.
[input+input_size, output+init_size) is [_text, _end) for ZO.
that will be first range in mem_avoid. we don't need to guess that anymore.
That area aleady include heap and stack. So we don't need to put
them into mem_avoid array.
Also we need to put boot_params into the mem_avoid too.
as with 64bit boot loader will put them above 4G.
Also change output_size to init_size, as we pass that instead already.
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/x86/boot/compressed/aslr.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 7083c16..e8486a5 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -116,7 +116,7 @@ struct mem_vector {
unsigned long size;
};
-#define MEM_AVOID_MAX 5
+#define MEM_AVOID_MAX 4
static struct mem_vector mem_avoid[MEM_AVOID_MAX];
static bool mem_contains(struct mem_vector *region, struct mem_vector *item)
@@ -142,7 +142,7 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
}
static void mem_avoid_init(unsigned long input, unsigned long input_size,
- unsigned long output, unsigned long output_size)
+ unsigned long output, unsigned long init_size)
{
u64 initrd_start, initrd_size;
u64 cmd_line, cmd_line_size;
@@ -151,10 +151,13 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
/*
* Avoid the region that is unsafe to overlap during
- * decompression (see calculations at top of misc.c).
+ * decompression.
+ * As we already move compressed to the end of buffer,
+ * [input+input_size, output+init_size) has [_text, _end)
+ * for arch/x86/boot/compressed/vmlinux.
*/
- unsafe_len = (output_size >> 12) + 32768 + 18;
- unsafe = (unsigned long)input + input_size - unsafe_len;
+ unsafe_len = output + init_size - (input + input_size);
+ unsafe = (unsigned long)input + input_size;
mem_avoid[0].start = unsafe;
mem_avoid[0].size = unsafe_len;
@@ -176,13 +179,9 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
mem_avoid[2].start = cmd_line;
mem_avoid[2].size = cmd_line_size;
- /* Avoid heap memory. */
- mem_avoid[3].start = (unsigned long)free_mem_ptr;
- mem_avoid[3].size = BOOT_HEAP_SIZE;
-
- /* Avoid stack memory. */
- mem_avoid[4].start = (unsigned long)free_mem_end_ptr;
- mem_avoid[4].size = BOOT_STACK_SIZE;
+ /* Avoid params */
+ mem_avoid[3].start = (unsigned long)real_mode;
+ mem_avoid[3].size = sizeof(*real_mode);
}
/* Does this memory vector overlap a known avoided area? */
@@ -327,7 +326,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
unsigned char *input,
unsigned long input_size,
unsigned char *output,
- unsigned long output_size)
+ unsigned long init_size)
{
unsigned long choice = (unsigned long)output;
unsigned long random;
@@ -349,10 +348,10 @@ unsigned char *choose_kernel_location(struct boot_params *params,
/* Record the various known unsafe memory ranges. */
mem_avoid_init((unsigned long)input, input_size,
- (unsigned long)output, output_size);
+ (unsigned long)output, init_size);
/* Walk e820 and find a random address. */
- random = find_random_addr(choice, output_size);
+ random = find_random_addr(choice, init_size);
if (!random) {
debug_putstr("KASLR could not find suitable E820 region...\n");
goto out;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 06/15] x86, boot: split kernel_ident_mapping_init into another file
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (4 preceding siblings ...)
2015-03-04 8:00 ` Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 07/15] x86, kaslr, 64bit: set new or extra ident_mapping Yinghai Lu
` (8 subsequent siblings)
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
We need to include that in boot::decompress_kernel stage to set new mapping.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/include/asm/page.h | 5 +++
arch/x86/mm/ident_map.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
arch/x86/mm/init_64.c | 74 +--------------------------------------------
3 files changed, 80 insertions(+), 73 deletions(-)
create mode 100644 arch/x86/mm/ident_map.c
diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index 802dde3..cf8f619 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -37,7 +37,10 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#ifndef __pa
#define __pa(x) __phys_addr((unsigned long)(x))
+#endif
+
#define __pa_nodebug(x) __phys_addr_nodebug((unsigned long)(x))
/* __pa_symbol should be used for C visible symbols.
This seems to be the official gcc blessed way to do such arithmetic. */
@@ -51,7 +54,9 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
#define __pa_symbol(x) \
__phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))
+#ifndef __va
#define __va(x) ((void *)((unsigned long)(x)+PAGE_OFFSET))
+#endif
#define __boot_va(x) __va(x)
#define __boot_pa(x) __pa(x)
diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
new file mode 100644
index 0000000..751ca92
--- /dev/null
+++ b/arch/x86/mm/ident_map.c
@@ -0,0 +1,74 @@
+
+static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
+ unsigned long addr, unsigned long end)
+{
+ addr &= PMD_MASK;
+ for (; addr < end; addr += PMD_SIZE) {
+ pmd_t *pmd = pmd_page + pmd_index(addr);
+
+ if (!pmd_present(*pmd))
+ set_pmd(pmd, __pmd(addr | pmd_flag));
+ }
+}
+static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
+ unsigned long addr, unsigned long end)
+{
+ unsigned long next;
+
+ for (; addr < end; addr = next) {
+ pud_t *pud = pud_page + pud_index(addr);
+ pmd_t *pmd;
+
+ next = (addr & PUD_MASK) + PUD_SIZE;
+ if (next > end)
+ next = end;
+
+ if (pud_present(*pud)) {
+ pmd = pmd_offset(pud, 0);
+ ident_pmd_init(info->pmd_flag, pmd, addr, next);
+ continue;
+ }
+ pmd = (pmd_t *)info->alloc_pgt_page(info->context);
+ if (!pmd)
+ return -ENOMEM;
+ ident_pmd_init(info->pmd_flag, pmd, addr, next);
+ set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
+ }
+
+ return 0;
+}
+
+int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
+ unsigned long addr, unsigned long end)
+{
+ unsigned long next;
+ int result;
+ int off = info->kernel_mapping ? pgd_index(__PAGE_OFFSET) : 0;
+
+ for (; addr < end; addr = next) {
+ pgd_t *pgd = pgd_page + pgd_index(addr) + off;
+ pud_t *pud;
+
+ next = (addr & PGDIR_MASK) + PGDIR_SIZE;
+ if (next > end)
+ next = end;
+
+ if (pgd_present(*pgd)) {
+ pud = pud_offset(pgd, 0);
+ result = ident_pud_init(info, pud, addr, next);
+ if (result)
+ return result;
+ continue;
+ }
+
+ pud = (pud_t *)info->alloc_pgt_page(info->context);
+ if (!pud)
+ return -ENOMEM;
+ result = ident_pud_init(info, pud, addr, next);
+ if (result)
+ return result;
+ set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
+ }
+
+ return 0;
+}
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 30eb05a..c30efb6 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -56,79 +56,7 @@
#include "mm_internal.h"
-static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
- unsigned long addr, unsigned long end)
-{
- addr &= PMD_MASK;
- for (; addr < end; addr += PMD_SIZE) {
- pmd_t *pmd = pmd_page + pmd_index(addr);
-
- if (!pmd_present(*pmd))
- set_pmd(pmd, __pmd(addr | pmd_flag));
- }
-}
-static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
- unsigned long addr, unsigned long end)
-{
- unsigned long next;
-
- for (; addr < end; addr = next) {
- pud_t *pud = pud_page + pud_index(addr);
- pmd_t *pmd;
-
- next = (addr & PUD_MASK) + PUD_SIZE;
- if (next > end)
- next = end;
-
- if (pud_present(*pud)) {
- pmd = pmd_offset(pud, 0);
- ident_pmd_init(info->pmd_flag, pmd, addr, next);
- continue;
- }
- pmd = (pmd_t *)info->alloc_pgt_page(info->context);
- if (!pmd)
- return -ENOMEM;
- ident_pmd_init(info->pmd_flag, pmd, addr, next);
- set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
- }
-
- return 0;
-}
-
-int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
- unsigned long addr, unsigned long end)
-{
- unsigned long next;
- int result;
- int off = info->kernel_mapping ? pgd_index(__PAGE_OFFSET) : 0;
-
- for (; addr < end; addr = next) {
- pgd_t *pgd = pgd_page + pgd_index(addr) + off;
- pud_t *pud;
-
- next = (addr & PGDIR_MASK) + PGDIR_SIZE;
- if (next > end)
- next = end;
-
- if (pgd_present(*pgd)) {
- pud = pud_offset(pgd, 0);
- result = ident_pud_init(info, pud, addr, next);
- if (result)
- return result;
- continue;
- }
-
- pud = (pud_t *)info->alloc_pgt_page(info->context);
- if (!pud)
- return -ENOMEM;
- result = ident_pud_init(info, pud, addr, next);
- if (result)
- return result;
- set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
- }
-
- return 0;
-}
+#include "ident_map.c"
static int __init parse_direct_gbpages_off(char *arg)
{
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 07/15] x86, kaslr, 64bit: set new or extra ident_mapping
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (5 preceding siblings ...)
2015-03-04 8:00 ` [PATCH v2 06/15] x86, boot: split kernel_ident_mapping_init into another file Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
` (7 subsequent siblings)
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Kees Cook
aslr will support to put random VO above 4G, so we need to set ident
mapping for the range even we come from startup_32 path.
At the same time, when boot from 64bit bootloader, bootloader will
set ident mapping, and boot via ZO startup_64.
Then pages for pagetable need to be avoided when selecting new random VO base.
otherwise decompressor will overwrite the pgtable.
One solution: go through pagetable and find out every page is
used by pagetable for every mem_aovid checking.
but kexec could put those page anywhere, and we will need extra code.
Other solution: create new ident mapping instead, and pages for pagetable
will sit in _pagetable area of ZO, and they are in mem_avoid array already.
so the _pgtable will be shared 32bit and 64bit path to reduce init_size.
Need to increase buffer size. As we need to cover old VO, params, cmdline
and new VO, in extreme case we could have all cross 512G boundary, will need
1+(2+2)*4 pages with 2M mapping.
Cc: Kees Cook <keescook@chromium.org>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/boot/compressed/aslr.c | 28 +++++++++++
arch/x86/boot/compressed/head_64.S | 4 +-
arch/x86/boot/compressed/misc_pgt.c | 96 +++++++++++++++++++++++++++++++++++++
arch/x86/include/asm/boot.h | 13 +++++
4 files changed, 139 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/boot/compressed/misc_pgt.c
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index e8486a5..10ed3c7 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -1,3 +1,8 @@
+#ifdef CONFIG_X86_64
+#define __pa(x) ((unsigned long)(x))
+#define __va(x) ((void *)((unsigned long)(x)))
+#endif
+
#include "misc.h"
#include <asm/msr.h>
@@ -21,6 +26,8 @@ struct kaslr_setup_data {
__u8 data[1];
} kaslr_setup_data;
+#include "misc_pgt.c"
+
#define I8254_PORT_CONTROL 0x43
#define I8254_PORT_COUNTER0 0x40
#define I8254_CMD_READBACK 0xC0
@@ -160,6 +167,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
unsafe = (unsigned long)input + input_size;
mem_avoid[0].start = unsafe;
mem_avoid[0].size = unsafe_len;
+ fill_linux64_pagetable(output, init_size);
/* Avoid initrd. */
initrd_start = (u64)real_mode->ext_ramdisk_image << 32;
@@ -168,6 +176,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
initrd_size |= real_mode->hdr.ramdisk_size;
mem_avoid[1].start = initrd_start;
mem_avoid[1].size = initrd_size;
+ /* don't need to set mapping for initrd */
/* Avoid kernel command line. */
cmd_line = (u64)real_mode->ext_cmd_line_ptr << 32;
@@ -178,10 +187,25 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
;
mem_avoid[2].start = cmd_line;
mem_avoid[2].size = cmd_line_size;
+ fill_linux64_pagetable(cmd_line, cmd_line_size);
/* Avoid params */
mem_avoid[3].start = (unsigned long)real_mode;
mem_avoid[3].size = sizeof(*real_mode);
+ fill_linux64_pagetable((unsigned long)real_mode, sizeof(*real_mode));
+}
+
+static void init_linux64_pagetable(void)
+{
+ struct setup_data *ptr;
+
+ ptr = (struct setup_data *)(unsigned long)real_mode->hdr.setup_data;
+ while (ptr) {
+ fill_linux64_pagetable((unsigned long)ptr,
+ sizeof(*ptr) + ptr->len);
+
+ ptr = (struct setup_data *)(unsigned long)ptr->next;
+ }
}
/* Does this memory vector overlap a known avoided area? */
@@ -346,6 +370,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
#endif
add_kaslr_setup_data(params, 1);
+ init_linux64_pagetable();
/* Record the various known unsafe memory ranges. */
mem_avoid_init((unsigned long)input, input_size,
(unsigned long)output, init_size);
@@ -362,6 +387,9 @@ unsigned char *choose_kernel_location(struct boot_params *params,
goto out;
choice = random;
+
+ fill_linux64_pagetable(choice, init_size);
+ switch_linux64_pagetable();
out:
return (unsigned char *)choice;
}
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 69015b5..1b6e34a 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -125,7 +125,7 @@ ENTRY(startup_32)
/* Initialize Page tables to 0 */
leal pgtable(%ebx), %edi
xorl %eax, %eax
- movl $((4096*6)/4), %ecx
+ movl $(BOOT_INIT_PGT_SIZE/4), %ecx
rep stosl
/* Build Level 4 */
@@ -477,4 +477,4 @@ boot_stack_end:
.section ".pgtable","a",@nobits
.balign 4096
pgtable:
- .fill 6*4096, 1, 0
+ .fill BOOT_PGT_SIZE, 1, 0
diff --git a/arch/x86/boot/compressed/misc_pgt.c b/arch/x86/boot/compressed/misc_pgt.c
new file mode 100644
index 0000000..afc73bf
--- /dev/null
+++ b/arch/x86/boot/compressed/misc_pgt.c
@@ -0,0 +1,96 @@
+
+#ifdef CONFIG_X86_64
+#include <asm/init.h>
+#include <asm/pgtable.h>
+
+#include "../../mm/ident_map.c"
+
+struct alloc_pgt_data {
+ unsigned char *pgt_buf;
+ unsigned long pgt_buf_size;
+ unsigned long pgt_buf_offset;
+};
+
+static void *alloc_pgt_page(void *context)
+{
+ struct alloc_pgt_data *d = (struct alloc_pgt_data *)context;
+ unsigned char *p = (unsigned char *)d->pgt_buf;
+
+ if (d->pgt_buf_offset >= d->pgt_buf_size) {
+ debug_putstr("out of pgt_buf in misc.c\n");
+ return NULL;
+ }
+
+ p += d->pgt_buf_offset;
+ d->pgt_buf_offset += PAGE_SIZE;
+
+ return p;
+}
+
+/*
+ * Use a normal definition of memset() from string.c. There are already
+ * included header files which expect a definition of memset() and by
+ * the time we define memset macro, it is too late.
+ */
+#undef memset
+#define memzero(s, n) memset((s), 0, (n))
+
+unsigned long __force_order;
+static struct alloc_pgt_data pgt_data;
+static struct x86_mapping_info mapping_info;
+static pgd_t *level4p;
+
+extern unsigned char _pgtable[];
+static void fill_linux64_pagetable(unsigned long start, unsigned long size)
+{
+ unsigned long end = start + size;
+
+ if (!level4p) {
+ pgt_data.pgt_buf_offset = 0;
+ mapping_info.alloc_pgt_page = alloc_pgt_page;
+ mapping_info.context = &pgt_data;
+ mapping_info.pmd_flag = __PAGE_KERNEL_LARGE_EXEC;
+
+ /*
+ * come from startup_32 ?
+ * then cr3 is _pgtable, we can reuse it.
+ */
+ level4p = (pgd_t *)read_cr3();
+ if ((unsigned long)level4p == (unsigned long)_pgtable) {
+ pgt_data.pgt_buf = (unsigned char *)_pgtable +
+ BOOT_INIT_PGT_SIZE;
+ pgt_data.pgt_buf_size = BOOT_PGT_SIZE -
+ BOOT_INIT_PGT_SIZE;
+
+ debug_putstr("boot via startup_32\n");
+ } else {
+ pgt_data.pgt_buf = (unsigned char *)_pgtable;
+ pgt_data.pgt_buf_size = BOOT_PGT_SIZE;
+
+ debug_putstr("boot via startup_64\n");
+ level4p = (pgd_t *)alloc_pgt_page(&pgt_data);
+ }
+ memset((unsigned char *)pgt_data.pgt_buf, 0,
+ pgt_data.pgt_buf_size);
+ }
+
+ /* align boundary to 2M */
+ start = round_down(start, PMD_SIZE);
+ end = round_up(end, PMD_SIZE);
+ if (start < end)
+ kernel_ident_mapping_init(&mapping_info, level4p, start, end);
+}
+
+static void switch_linux64_pagetable(void)
+{
+ write_cr3((unsigned long)level4p);
+}
+
+#else
+static void fill_linux64_pagetable(unsigned long start, unsigned long size)
+{
+}
+static void switch_linux64_pagetable(void)
+{
+}
+#endif
diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
index 4fa687a..3795a77 100644
--- a/arch/x86/include/asm/boot.h
+++ b/arch/x86/include/asm/boot.h
@@ -32,7 +32,20 @@
#endif /* !CONFIG_KERNEL_BZIP2 */
#ifdef CONFIG_X86_64
+
#define BOOT_STACK_SIZE 0x4000
+
+#define BOOT_INIT_PGT_SIZE (6*4096)
+#ifdef CONFIG_RANDOMIZE_BASE
+/*
+ * 17 pages to cover for kernel, param, cmd_line, random kernel
+ * if all cross 512G boundary.
+ */
+#define BOOT_PGT_SIZE (BOOT_INIT_PGT_SIZE + (11*4096))
+#else
+#define BOOT_PGT_SIZE BOOT_INIT_PGT_SIZE
+#endif
+
#else
#define BOOT_STACK_SIZE 0x1000
#endif
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 08/15] x86: Kill E820_RESERVED_KERN
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu, Lee,
Chun-Yi, stable
Now we are using memblock to do early resource reserver/allocation
instead of using e820 map directly, and setup_data is reserved in
memblock early already.
Also kexec will generate setup_data and pass pointer to second kernel,
so second kernel will reserve setup_data by their own.
We can kill E820_RESERVED_KERN and not touch e820 map at all.
That will fix bug in mark_nonsave_region that can not handle that
case: E820_RAM and E820_RESERVED_KERN ranges are continuous and
boundary is not page aligned.
Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=913885
Reported-by: "Lee, Chun-Yi" <jlee@suse.com>
Tested-by: "Lee, Chun-Yi" <jlee@suse.com>
Cc: "Lee, Chun-Yi" <jlee@suse.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: stable@vger.kernel.org
---
arch/x86/include/uapi/asm/e820.h | 9 ---------
arch/x86/kernel/e820.c | 6 ++----
arch/x86/kernel/setup.c | 26 --------------------------
arch/x86/kernel/tboot.c | 3 +--
arch/x86/mm/init_64.c | 11 ++++-------
5 files changed, 7 insertions(+), 48 deletions(-)
diff --git a/arch/x86/include/uapi/asm/e820.h b/arch/x86/include/uapi/asm/e820.h
index d993e33..edc8a71 100644
--- a/arch/x86/include/uapi/asm/e820.h
+++ b/arch/x86/include/uapi/asm/e820.h
@@ -33,15 +33,6 @@
#define E820_NVS 4
#define E820_UNUSABLE 5
-
-/*
- * reserved RAM used by kernel itself
- * if CONFIG_INTEL_TXT is enabled, memory of this type will be
- * included in the S3 integrity calculation and so should not include
- * any memory that BIOS might alter over the S3 transition
- */
-#define E820_RESERVED_KERN 128
-
#ifndef __ASSEMBLY__
#include <linux/types.h>
struct e820entry {
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 46201de..2a6bed9 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -134,7 +134,6 @@ static void __init e820_print_type(u32 type)
{
switch (type) {
case E820_RAM:
- case E820_RESERVED_KERN:
printk(KERN_CONT "usable");
break;
case E820_RESERVED:
@@ -688,7 +687,7 @@ void __init e820_mark_nosave_regions(unsigned long limit_pfn)
register_nosave_region(pfn, PFN_UP(ei->addr));
pfn = PFN_DOWN(ei->addr + ei->size);
- if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
+ if (ei->type != E820_RAM)
register_nosave_region(PFN_UP(ei->addr), pfn);
if (pfn >= limit_pfn)
@@ -902,7 +901,6 @@ void __init finish_e820_parsing(void)
static inline const char *e820_type_to_string(int e820_type)
{
switch (e820_type) {
- case E820_RESERVED_KERN:
case E820_RAM: return "System RAM";
case E820_ACPI: return "ACPI Tables";
case E820_NVS: return "ACPI Non-volatile Storage";
@@ -1077,7 +1075,7 @@ void __init memblock_x86_fill(void)
if (end != (resource_size_t)end)
continue;
- if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
+ if (ei->type != E820_RAM)
continue;
memblock_add(ei->addr, ei->size);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 05d444f..c9b3e2f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -473,30 +473,6 @@ static void __init parse_setup_data(void)
}
}
-static void __init e820_reserve_setup_data(void)
-{
- struct setup_data *data;
- u64 pa_data;
- int found = 0;
-
- pa_data = boot_params.hdr.setup_data;
- while (pa_data) {
- data = early_memremap(pa_data, sizeof(*data));
- e820_update_range(pa_data, sizeof(*data)+data->len,
- E820_RAM, E820_RESERVED_KERN);
- found = 1;
- pa_data = data->next;
- early_iounmap(data, sizeof(*data));
- }
- if (!found)
- return;
-
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
- memcpy(&e820_saved, &e820, sizeof(struct e820map));
- printk(KERN_INFO "extended physical RAM map:\n");
- e820_print_map("reserve setup_data");
-}
-
static void __init memblock_x86_reserve_range_setup_data(void)
{
struct setup_data *data;
@@ -1032,8 +1008,6 @@ void __init setup_arch(char **cmdline_p)
early_dump_pci_devices();
#endif
- /* update the e820_saved too */
- e820_reserve_setup_data();
finish_e820_parsing();
if (efi_enabled(EFI_BOOT))
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 91a4496..3c2752a 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -195,8 +195,7 @@ static int tboot_setup_sleep(void)
tboot->num_mac_regions = 0;
for (i = 0; i < e820.nr_map; i++) {
- if ((e820.map[i].type != E820_RAM)
- && (e820.map[i].type != E820_RESERVED_KERN))
+ if (e820.map[i].type != E820_RAM)
continue;
add_mac_region(e820.map[i].addr, e820.map[i].size);
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index c30efb6..63520ec 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -354,8 +354,7 @@ phys_pte_init(pte_t *pte_page, unsigned long addr, unsigned long end,
next = (addr & PAGE_MASK) + PAGE_SIZE;
if (addr >= end) {
if (!after_bootmem &&
- !e820_any_mapped(addr & PAGE_MASK, next, E820_RAM) &&
- !e820_any_mapped(addr & PAGE_MASK, next, E820_RESERVED_KERN))
+ !e820_any_mapped(addr & PAGE_MASK, next, E820_RAM))
set_pte(pte, __pte(0));
continue;
}
@@ -401,9 +400,8 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long address, unsigned long end,
next = (address & PMD_MASK) + PMD_SIZE;
if (address >= end) {
- if (!after_bootmem &&
- !e820_any_mapped(address & PMD_MASK, next, E820_RAM) &&
- !e820_any_mapped(address & PMD_MASK, next, E820_RESERVED_KERN))
+ if (!after_bootmem && !e820_any_mapped(
+ address & PMD_MASK, next, E820_RAM))
set_pmd(pmd, __pmd(0));
continue;
}
@@ -476,8 +474,7 @@ phys_pud_init(pud_t *pud_page, unsigned long addr, unsigned long end,
next = (addr & PUD_MASK) + PUD_SIZE;
if (addr >= end) {
if (!after_bootmem &&
- !e820_any_mapped(addr & PUD_MASK, next, E820_RAM) &&
- !e820_any_mapped(addr & PUD_MASK, next, E820_RESERVED_KERN))
+ !e820_any_mapped(addr & PUD_MASK, next, E820_RAM))
set_pud(pud, __pud(0));
continue;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 08/15] x86: Kill E820_RESERVED_KERN
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Yinghai Lu, Lee, Chun-Yi,
stable-u79uwXL29TY76Z2rM5mHXA
Now we are using memblock to do early resource reserver/allocation
instead of using e820 map directly, and setup_data is reserved in
memblock early already.
Also kexec will generate setup_data and pass pointer to second kernel,
so second kernel will reserve setup_data by their own.
We can kill E820_RESERVED_KERN and not touch e820 map at all.
That will fix bug in mark_nonsave_region that can not handle that
case: E820_RAM and E820_RESERVED_KERN ranges are continuous and
boundary is not page aligned.
Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=913885
Reported-by: "Lee, Chun-Yi" <jlee-IBi9RG/b67k@public.gmane.org>
Tested-by: "Lee, Chun-Yi" <jlee-IBi9RG/b67k@public.gmane.org>
Cc: "Lee, Chun-Yi" <jlee-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
arch/x86/include/uapi/asm/e820.h | 9 ---------
arch/x86/kernel/e820.c | 6 ++----
arch/x86/kernel/setup.c | 26 --------------------------
arch/x86/kernel/tboot.c | 3 +--
arch/x86/mm/init_64.c | 11 ++++-------
5 files changed, 7 insertions(+), 48 deletions(-)
diff --git a/arch/x86/include/uapi/asm/e820.h b/arch/x86/include/uapi/asm/e820.h
index d993e33..edc8a71 100644
--- a/arch/x86/include/uapi/asm/e820.h
+++ b/arch/x86/include/uapi/asm/e820.h
@@ -33,15 +33,6 @@
#define E820_NVS 4
#define E820_UNUSABLE 5
-
-/*
- * reserved RAM used by kernel itself
- * if CONFIG_INTEL_TXT is enabled, memory of this type will be
- * included in the S3 integrity calculation and so should not include
- * any memory that BIOS might alter over the S3 transition
- */
-#define E820_RESERVED_KERN 128
-
#ifndef __ASSEMBLY__
#include <linux/types.h>
struct e820entry {
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 46201de..2a6bed9 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -134,7 +134,6 @@ static void __init e820_print_type(u32 type)
{
switch (type) {
case E820_RAM:
- case E820_RESERVED_KERN:
printk(KERN_CONT "usable");
break;
case E820_RESERVED:
@@ -688,7 +687,7 @@ void __init e820_mark_nosave_regions(unsigned long limit_pfn)
register_nosave_region(pfn, PFN_UP(ei->addr));
pfn = PFN_DOWN(ei->addr + ei->size);
- if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
+ if (ei->type != E820_RAM)
register_nosave_region(PFN_UP(ei->addr), pfn);
if (pfn >= limit_pfn)
@@ -902,7 +901,6 @@ void __init finish_e820_parsing(void)
static inline const char *e820_type_to_string(int e820_type)
{
switch (e820_type) {
- case E820_RESERVED_KERN:
case E820_RAM: return "System RAM";
case E820_ACPI: return "ACPI Tables";
case E820_NVS: return "ACPI Non-volatile Storage";
@@ -1077,7 +1075,7 @@ void __init memblock_x86_fill(void)
if (end != (resource_size_t)end)
continue;
- if (ei->type != E820_RAM && ei->type != E820_RESERVED_KERN)
+ if (ei->type != E820_RAM)
continue;
memblock_add(ei->addr, ei->size);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 05d444f..c9b3e2f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -473,30 +473,6 @@ static void __init parse_setup_data(void)
}
}
-static void __init e820_reserve_setup_data(void)
-{
- struct setup_data *data;
- u64 pa_data;
- int found = 0;
-
- pa_data = boot_params.hdr.setup_data;
- while (pa_data) {
- data = early_memremap(pa_data, sizeof(*data));
- e820_update_range(pa_data, sizeof(*data)+data->len,
- E820_RAM, E820_RESERVED_KERN);
- found = 1;
- pa_data = data->next;
- early_iounmap(data, sizeof(*data));
- }
- if (!found)
- return;
-
- sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
- memcpy(&e820_saved, &e820, sizeof(struct e820map));
- printk(KERN_INFO "extended physical RAM map:\n");
- e820_print_map("reserve setup_data");
-}
-
static void __init memblock_x86_reserve_range_setup_data(void)
{
struct setup_data *data;
@@ -1032,8 +1008,6 @@ void __init setup_arch(char **cmdline_p)
early_dump_pci_devices();
#endif
- /* update the e820_saved too */
- e820_reserve_setup_data();
finish_e820_parsing();
if (efi_enabled(EFI_BOOT))
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 91a4496..3c2752a 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -195,8 +195,7 @@ static int tboot_setup_sleep(void)
tboot->num_mac_regions = 0;
for (i = 0; i < e820.nr_map; i++) {
- if ((e820.map[i].type != E820_RAM)
- && (e820.map[i].type != E820_RESERVED_KERN))
+ if (e820.map[i].type != E820_RAM)
continue;
add_mac_region(e820.map[i].addr, e820.map[i].size);
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index c30efb6..63520ec 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -354,8 +354,7 @@ phys_pte_init(pte_t *pte_page, unsigned long addr, unsigned long end,
next = (addr & PAGE_MASK) + PAGE_SIZE;
if (addr >= end) {
if (!after_bootmem &&
- !e820_any_mapped(addr & PAGE_MASK, next, E820_RAM) &&
- !e820_any_mapped(addr & PAGE_MASK, next, E820_RESERVED_KERN))
+ !e820_any_mapped(addr & PAGE_MASK, next, E820_RAM))
set_pte(pte, __pte(0));
continue;
}
@@ -401,9 +400,8 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long address, unsigned long end,
next = (address & PMD_MASK) + PMD_SIZE;
if (address >= end) {
- if (!after_bootmem &&
- !e820_any_mapped(address & PMD_MASK, next, E820_RAM) &&
- !e820_any_mapped(address & PMD_MASK, next, E820_RESERVED_KERN))
+ if (!after_bootmem && !e820_any_mapped(
+ address & PMD_MASK, next, E820_RAM))
set_pmd(pmd, __pmd(0));
continue;
}
@@ -476,8 +474,7 @@ phys_pud_init(pud_t *pud_page, unsigned long addr, unsigned long end,
next = (addr & PUD_MASK) + PUD_SIZE;
if (addr >= end) {
if (!after_bootmem &&
- !e820_any_mapped(addr & PUD_MASK, next, E820_RAM) &&
- !e820_any_mapped(addr & PUD_MASK, next, E820_RESERVED_KERN))
+ !e820_any_mapped(addr & PUD_MASK, next, E820_RAM))
set_pud(pud, __pud(0));
continue;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 09/15] x86, efi: copy SETUP_EFI data and access directly
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
the copy will be in __initdata, and it is small.
We can use pointer to access the setup_data instead of keeping on
early_memmap and early_memunmap everywhere.
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: linux-efi@vger.kernel.org
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/include/asm/efi.h | 2 +-
arch/x86/platform/efi/efi.c | 13 ++-----------
arch/x86/platform/efi/efi_64.c | 13 ++++++++++++-
arch/x86/platform/efi/quirks.c | 23 ++++++-----------------
4 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 25bce45..edbecd6 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -114,7 +114,7 @@ struct efi_setup_data {
u64 reserved[8];
};
-extern u64 efi_setup;
+extern struct efi_setup_data *efi_setup;
#ifdef CONFIG_EFI
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627..1cd38e8 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -68,7 +68,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
{NULL_GUID, NULL, NULL},
};
-u64 efi_setup; /* efi setup_data physical address */
+struct efi_setup_data *efi_setup __initdata; /* cached efi setup_data pointer */
static int add_efi_memmap __initdata;
static int __init setup_add_efi_memmap(char *arg)
@@ -225,20 +225,13 @@ static int __init efi_systab_init(void *phys)
{
if (efi_enabled(EFI_64BIT)) {
efi_system_table_64_t *systab64;
- struct efi_setup_data *data = NULL;
+ struct efi_setup_data *data = efi_setup;
u64 tmp = 0;
- if (efi_setup) {
- data = early_memremap(efi_setup, sizeof(*data));
- if (!data)
- return -ENOMEM;
- }
systab64 = early_memremap((unsigned long)phys,
sizeof(*systab64));
if (systab64 == NULL) {
pr_err("Couldn't map the system table!\n");
- if (data)
- early_memunmap(data, sizeof(*data));
return -ENOMEM;
}
@@ -271,8 +264,6 @@ static int __init efi_systab_init(void *phys)
tmp |= data ? data->tables : systab64->tables;
early_memunmap(systab64, sizeof(*systab64));
- if (data)
- early_memunmap(data, sizeof(*data));
#ifdef CONFIG_X86_32
if (tmp >> 32) {
pr_err("EFI data located above 4GB, disabling EFI.\n");
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 17e80d8..a541c6c 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -292,9 +292,20 @@ void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
return (void __iomem *)__va(phys_addr);
}
+static struct efi_setup_data efi_setup_data __initdata;
+
void __init parse_efi_setup(u64 phys_addr, u32 data_len)
{
- efi_setup = phys_addr + sizeof(struct setup_data);
+ struct efi_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data)
+ return;
+
+ efi_setup_data = *data;
+ early_memunmap(data, sizeof(*data));
+ efi_setup = &efi_setup_data;
}
void __init efi_runtime_mkexec(void)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 1c7380d..45fec7d 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -203,9 +203,8 @@ void __init efi_free_boot_services(void)
*/
int __init efi_reuse_config(u64 tables, int nr_tables)
{
- int i, sz, ret = 0;
+ int i, sz;
void *p, *tablep;
- struct efi_setup_data *data;
if (!efi_setup)
return 0;
@@ -213,22 +212,15 @@ int __init efi_reuse_config(u64 tables, int nr_tables)
if (!efi_enabled(EFI_64BIT))
return 0;
- data = early_memremap(efi_setup, sizeof(*data));
- if (!data) {
- ret = -ENOMEM;
- goto out;
- }
-
- if (!data->smbios)
- goto out_memremap;
+ if (!efi_setup->smbios)
+ return 0;
sz = sizeof(efi_config_table_64_t);
p = tablep = early_memremap(tables, nr_tables * sz);
if (!p) {
pr_err("Could not map Configuration table!\n");
- ret = -ENOMEM;
- goto out_memremap;
+ return -ENOMEM;
}
for (i = 0; i < efi.systab->nr_tables; i++) {
@@ -237,15 +229,12 @@ int __init efi_reuse_config(u64 tables, int nr_tables)
guid = ((efi_config_table_64_t *)p)->guid;
if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
- ((efi_config_table_64_t *)p)->table = data->smbios;
+ ((efi_config_table_64_t *)p)->table = efi_setup->smbios;
p += sz;
}
early_memunmap(tablep, nr_tables * sz);
-out_memremap:
- early_memunmap(data, sizeof(*data));
-out:
- return ret;
+ return 0;
}
void __init efi_apply_memmap_quirks(void)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 09/15] x86, efi: copy SETUP_EFI data and access directly
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Yinghai Lu
the copy will be in __initdata, and it is small.
We can use pointer to access the setup_data instead of keeping on
early_memmap and early_memunmap everywhere.
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/x86/include/asm/efi.h | 2 +-
arch/x86/platform/efi/efi.c | 13 ++-----------
arch/x86/platform/efi/efi_64.c | 13 ++++++++++++-
arch/x86/platform/efi/quirks.c | 23 ++++++-----------------
4 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 25bce45..edbecd6 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -114,7 +114,7 @@ struct efi_setup_data {
u64 reserved[8];
};
-extern u64 efi_setup;
+extern struct efi_setup_data *efi_setup;
#ifdef CONFIG_EFI
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627..1cd38e8 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -68,7 +68,7 @@ static efi_config_table_type_t arch_tables[] __initdata = {
{NULL_GUID, NULL, NULL},
};
-u64 efi_setup; /* efi setup_data physical address */
+struct efi_setup_data *efi_setup __initdata; /* cached efi setup_data pointer */
static int add_efi_memmap __initdata;
static int __init setup_add_efi_memmap(char *arg)
@@ -225,20 +225,13 @@ static int __init efi_systab_init(void *phys)
{
if (efi_enabled(EFI_64BIT)) {
efi_system_table_64_t *systab64;
- struct efi_setup_data *data = NULL;
+ struct efi_setup_data *data = efi_setup;
u64 tmp = 0;
- if (efi_setup) {
- data = early_memremap(efi_setup, sizeof(*data));
- if (!data)
- return -ENOMEM;
- }
systab64 = early_memremap((unsigned long)phys,
sizeof(*systab64));
if (systab64 == NULL) {
pr_err("Couldn't map the system table!\n");
- if (data)
- early_memunmap(data, sizeof(*data));
return -ENOMEM;
}
@@ -271,8 +264,6 @@ static int __init efi_systab_init(void *phys)
tmp |= data ? data->tables : systab64->tables;
early_memunmap(systab64, sizeof(*systab64));
- if (data)
- early_memunmap(data, sizeof(*data));
#ifdef CONFIG_X86_32
if (tmp >> 32) {
pr_err("EFI data located above 4GB, disabling EFI.\n");
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 17e80d8..a541c6c 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -292,9 +292,20 @@ void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
return (void __iomem *)__va(phys_addr);
}
+static struct efi_setup_data efi_setup_data __initdata;
+
void __init parse_efi_setup(u64 phys_addr, u32 data_len)
{
- efi_setup = phys_addr + sizeof(struct setup_data);
+ struct efi_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data)
+ return;
+
+ efi_setup_data = *data;
+ early_memunmap(data, sizeof(*data));
+ efi_setup = &efi_setup_data;
}
void __init efi_runtime_mkexec(void)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 1c7380d..45fec7d 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -203,9 +203,8 @@ void __init efi_free_boot_services(void)
*/
int __init efi_reuse_config(u64 tables, int nr_tables)
{
- int i, sz, ret = 0;
+ int i, sz;
void *p, *tablep;
- struct efi_setup_data *data;
if (!efi_setup)
return 0;
@@ -213,22 +212,15 @@ int __init efi_reuse_config(u64 tables, int nr_tables)
if (!efi_enabled(EFI_64BIT))
return 0;
- data = early_memremap(efi_setup, sizeof(*data));
- if (!data) {
- ret = -ENOMEM;
- goto out;
- }
-
- if (!data->smbios)
- goto out_memremap;
+ if (!efi_setup->smbios)
+ return 0;
sz = sizeof(efi_config_table_64_t);
p = tablep = early_memremap(tables, nr_tables * sz);
if (!p) {
pr_err("Could not map Configuration table!\n");
- ret = -ENOMEM;
- goto out_memremap;
+ return -ENOMEM;
}
for (i = 0; i < efi.systab->nr_tables; i++) {
@@ -237,15 +229,12 @@ int __init efi_reuse_config(u64 tables, int nr_tables)
guid = ((efi_config_table_64_t *)p)->guid;
if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
- ((efi_config_table_64_t *)p)->table = data->smbios;
+ ((efi_config_table_64_t *)p)->table = efi_setup->smbios;
p += sz;
}
early_memunmap(tablep, nr_tables * sz);
-out_memremap:
- early_memunmap(data, sizeof(*data));
-out:
- return ret;
+ return 0;
}
void __init efi_apply_memmap_quirks(void)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 10/15] x86, of: let add_dtb reserve by itself
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (8 preceding siblings ...)
2015-03-04 8:00 ` Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 11/15] x86, boot: Add add_pci handler for SETUP_PCI Yinghai Lu
` (4 subsequent siblings)
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu,
Rob Herring, David Vrabel
We will not reserve setup_data in general code. Every handler
need to reserve and copy.
Current dtd handling already have code copying, just add reserve code ...
also simplify code a bit with storing real dtb size.
Cc: Rob Herring <robh@kernel.org>
Cc: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/include/asm/prom.h | 9 ++++++---
arch/x86/kernel/devicetree.c | 39 +++++++++++++++++++++------------------
2 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
index 1d081ac..fb716eddc 100644
--- a/arch/x86/include/asm/prom.h
+++ b/arch/x86/include/asm/prom.h
@@ -24,17 +24,20 @@
#ifdef CONFIG_OF
extern int of_ioapic;
-extern u64 initial_dtb;
-extern void add_dtb(u64 data);
void x86_of_pci_init(void);
void x86_dtb_init(void);
#else
-static inline void add_dtb(u64 data) { }
static inline void x86_of_pci_init(void) { }
static inline void x86_dtb_init(void) { }
#define of_ioapic 0
#endif
+#ifdef CONFIG_OF_FLATTREE
+extern void add_dtb(u64 data);
+#else
+static inline void add_dtb(u64 data) { }
+#endif
+
extern char cmd_line[COMMAND_LINE_SIZE];
#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 3d35033..cc2fb61 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -2,6 +2,7 @@
* Architecture specific OF callbacks.
*/
#include <linux/bootmem.h>
+#include <linux/memblock.h>
#include <linux/export.h>
#include <linux/io.h>
#include <linux/irqdomain.h>
@@ -23,7 +24,6 @@
#include <asm/setup.h>
#include <asm/i8259.h>
-__initdata u64 initial_dtb;
char __initdata cmd_line[COMMAND_LINE_SIZE];
int __initdata of_ioapic;
@@ -43,11 +43,23 @@ void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
return __alloc_bootmem(size, align, __pa(MAX_DMA_ADDRESS));
}
+#ifdef CONFIG_OF_FLATTREE
+static u64 initial_dtb __initdata;
+static u32 initial_dtb_size __initdata;
void __init add_dtb(u64 data)
{
+ u32 map_len;
+
initial_dtb = data + offsetof(struct setup_data, data);
-}
+ map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128);
+ initial_boot_params = early_memremap(initial_dtb, map_len);
+ initial_dtb_size = of_get_flat_dt_size();
+ early_memunmap(initial_boot_params, map_len);
+ initial_boot_params = NULL;
+ memblock_reserve(initial_dtb, initial_dtb_size);
+}
+#endif
/*
* CE4100 ids. Will be moved to machine_device_initcall() once we have it.
*/
@@ -272,31 +284,22 @@ static void __init dtb_apic_setup(void)
dtb_ioapic_setup();
}
-#ifdef CONFIG_OF_FLATTREE
static void __init x86_flattree_get_config(void)
{
- u32 size, map_len;
+#ifdef CONFIG_OF_FLATTREE
void *dt;
if (!initial_dtb)
return;
- map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128);
-
- initial_boot_params = dt = early_memremap(initial_dtb, map_len);
- size = of_get_flat_dt_size();
- if (map_len < size) {
- early_iounmap(dt, map_len);
- initial_boot_params = dt = early_memremap(initial_dtb, size);
- map_len = size;
- }
-
+ initial_boot_params = dt = early_memremap(initial_dtb,
+ initial_dtb_size);
unflatten_and_copy_device_tree();
- early_iounmap(dt, map_len);
-}
-#else
-static inline void x86_flattree_get_config(void) { }
+ early_memunmap(dt, initial_dtb_size);
+
+ memblock_free(initial_dtb, initial_dtb_size);
#endif
+}
void __init x86_dtb_init(void)
{
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 11/15] x86, boot: Add add_pci handler for SETUP_PCI
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (9 preceding siblings ...)
2015-03-04 8:00 ` [PATCH v2 10/15] x86, of: let add_dtb reserve by itself Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
` (3 subsequent siblings)
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
Let it reserve setup_data, and keep it's own list.
Also clear the hdr.setup_data, as all handler will handle or
reserve setup_data locally already.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/include/asm/pci.h | 2 ++
arch/x86/kernel/setup.c | 8 ++++++++
arch/x86/pci/common.c | 42 ++++++++++++++++++++++++++++--------------
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 4e370a5..7fbd5f3 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -87,8 +87,10 @@ static inline void pci_dma_burst_advice(struct pci_dev *pdev,
*strat = PCI_DMA_BURST_INFINITY;
*strategy_parameter = ~0UL;
}
+void add_pci(u64 pa_data);
#else
static inline void early_quirks(void) { }
+static inline void add_pci(u64 pa_data) { }
#endif
extern void pci_iommu_alloc(void);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c9b3e2f..b1296d3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -453,6 +453,8 @@ static void __init parse_setup_data(void)
pa_next = data->next;
early_iounmap(data, sizeof(*data));
+ printk(KERN_DEBUG "setup_data type: %d @ %#010llx\n",
+ data_type, pa_data);
switch (data_type) {
case SETUP_E820_EXT:
parse_e820_ext(pa_data, data_len);
@@ -460,6 +462,9 @@ static void __init parse_setup_data(void)
case SETUP_DTB:
add_dtb(pa_data);
break;
+ case SETUP_PCI:
+ add_pci(pa_data);
+ break;
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
@@ -467,10 +472,13 @@ static void __init parse_setup_data(void)
parse_kaslr_setup(pa_data, data_len);
break;
default:
+ pr_warn("Unknown setup_data type: %d @ %#010llx ignored!\n",
+ data_type, pa_data);
break;
}
pa_data = pa_next;
}
+ boot_params.hdr.setup_data = 0; /* all done */
}
static void __init memblock_x86_reserve_range_setup_data(void)
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 3d2612b..4846db7 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -9,6 +9,7 @@
#include <linux/pci-acpi.h>
#include <linux/ioport.h>
#include <linux/init.h>
+#include <linux/memblock.h>
#include <linux/dmi.h>
#include <linux/slab.h>
@@ -667,31 +668,44 @@ unsigned int pcibios_assign_all_busses(void)
return (pci_probe & PCI_ASSIGN_ALL_BUSSES) ? 1 : 0;
}
+static u64 pci_setup_data;
+void __init add_pci(u64 pa_data)
+{
+ struct setup_data *data;
+
+ data = early_memremap(pa_data, sizeof(*data));
+ memblock_reserve(pa_data, sizeof(*data) + data->len);
+ data->next = pci_setup_data;
+ pci_setup_data = pa_data;
+ early_memunmap(data, sizeof(*data));
+}
+
int pcibios_add_device(struct pci_dev *dev)
{
struct setup_data *data;
struct pci_setup_rom *rom;
u64 pa_data;
- pa_data = boot_params.hdr.setup_data;
+ pa_data = pci_setup_data;
while (pa_data) {
data = ioremap(pa_data, sizeof(*rom));
if (!data)
return -ENOMEM;
- if (data->type == SETUP_PCI) {
- rom = (struct pci_setup_rom *)data;
-
- if ((pci_domain_nr(dev->bus) == rom->segment) &&
- (dev->bus->number == rom->bus) &&
- (PCI_SLOT(dev->devfn) == rom->device) &&
- (PCI_FUNC(dev->devfn) == rom->function) &&
- (dev->vendor == rom->vendor) &&
- (dev->device == rom->devid)) {
- dev->rom = pa_data +
- offsetof(struct pci_setup_rom, romdata);
- dev->romlen = rom->pcilen;
- }
+ rom = (struct pci_setup_rom *)data;
+
+ if ((pci_domain_nr(dev->bus) == rom->segment) &&
+ (dev->bus->number == rom->bus) &&
+ (PCI_SLOT(dev->devfn) == rom->device) &&
+ (PCI_FUNC(dev->devfn) == rom->function) &&
+ (dev->vendor == rom->vendor) &&
+ (dev->device == rom->devid)) {
+ dev->rom = pa_data +
+ offsetof(struct pci_setup_rom, romdata);
+ dev->romlen = rom->pcilen;
+ dev_printk(KERN_DEBUG, &dev->dev, "set rom to [%#010lx, %#010lx] via SETUP_PCI\n",
+ (unsigned long)dev->rom,
+ (unsigned long)(dev->rom + dev->romlen - 1));
}
pa_data = data->next;
iounmap(data);
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 12/15] x86: kill not used setup_data handling code
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/kernel/kdebugfs.c | 142 ---------------------------------------------
arch/x86/kernel/setup.c | 17 ------
2 files changed, 159 deletions(-)
diff --git a/arch/x86/kernel/kdebugfs.c b/arch/x86/kernel/kdebugfs.c
index dc1404b..c8ca86c 100644
--- a/arch/x86/kernel/kdebugfs.c
+++ b/arch/x86/kernel/kdebugfs.c
@@ -21,142 +21,6 @@ struct dentry *arch_debugfs_dir;
EXPORT_SYMBOL(arch_debugfs_dir);
#ifdef CONFIG_DEBUG_BOOT_PARAMS
-struct setup_data_node {
- u64 paddr;
- u32 type;
- u32 len;
-};
-
-static ssize_t setup_data_read(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct setup_data_node *node = file->private_data;
- unsigned long remain;
- loff_t pos = *ppos;
- struct page *pg;
- void *p;
- u64 pa;
-
- if (pos < 0)
- return -EINVAL;
-
- if (pos >= node->len)
- return 0;
-
- if (count > node->len - pos)
- count = node->len - pos;
-
- pa = node->paddr + sizeof(struct setup_data) + pos;
- pg = pfn_to_page((pa + count - 1) >> PAGE_SHIFT);
- if (PageHighMem(pg)) {
- p = ioremap_cache(pa, count);
- if (!p)
- return -ENXIO;
- } else
- p = __va(pa);
-
- remain = copy_to_user(user_buf, p, count);
-
- if (PageHighMem(pg))
- iounmap(p);
-
- if (remain)
- return -EFAULT;
-
- *ppos = pos + count;
-
- return count;
-}
-
-static const struct file_operations fops_setup_data = {
- .read = setup_data_read,
- .open = simple_open,
- .llseek = default_llseek,
-};
-
-static int __init
-create_setup_data_node(struct dentry *parent, int no,
- struct setup_data_node *node)
-{
- struct dentry *d, *type, *data;
- char buf[16];
-
- sprintf(buf, "%d", no);
- d = debugfs_create_dir(buf, parent);
- if (!d)
- return -ENOMEM;
-
- type = debugfs_create_x32("type", S_IRUGO, d, &node->type);
- if (!type)
- goto err_dir;
-
- data = debugfs_create_file("data", S_IRUGO, d, node, &fops_setup_data);
- if (!data)
- goto err_type;
-
- return 0;
-
-err_type:
- debugfs_remove(type);
-err_dir:
- debugfs_remove(d);
- return -ENOMEM;
-}
-
-static int __init create_setup_data_nodes(struct dentry *parent)
-{
- struct setup_data_node *node;
- struct setup_data *data;
- int error;
- struct dentry *d;
- struct page *pg;
- u64 pa_data;
- int no = 0;
-
- d = debugfs_create_dir("setup_data", parent);
- if (!d)
- return -ENOMEM;
-
- pa_data = boot_params.hdr.setup_data;
-
- while (pa_data) {
- node = kmalloc(sizeof(*node), GFP_KERNEL);
- if (!node) {
- error = -ENOMEM;
- goto err_dir;
- }
-
- pg = pfn_to_page((pa_data+sizeof(*data)-1) >> PAGE_SHIFT);
- if (PageHighMem(pg)) {
- data = ioremap_cache(pa_data, sizeof(*data));
- if (!data) {
- kfree(node);
- error = -ENXIO;
- goto err_dir;
- }
- } else
- data = __va(pa_data);
-
- node->paddr = pa_data;
- node->type = data->type;
- node->len = data->len;
- error = create_setup_data_node(d, no, node);
- pa_data = data->next;
-
- if (PageHighMem(pg))
- iounmap(data);
- if (error)
- goto err_dir;
- no++;
- }
-
- return 0;
-
-err_dir:
- debugfs_remove(d);
- return error;
-}
-
static struct debugfs_blob_wrapper boot_params_blob = {
.data = &boot_params,
.size = sizeof(boot_params),
@@ -181,14 +45,8 @@ static int __init boot_params_kdebugfs_init(void)
if (!data)
goto err_version;
- error = create_setup_data_nodes(dbp);
- if (error)
- goto err_data;
-
return 0;
-err_data:
- debugfs_remove(data);
err_version:
debugfs_remove(version);
err_dir:
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index b1296d3..9a18078 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -481,20 +481,6 @@ static void __init parse_setup_data(void)
boot_params.hdr.setup_data = 0; /* all done */
}
-static void __init memblock_x86_reserve_range_setup_data(void)
-{
- struct setup_data *data;
- u64 pa_data;
-
- pa_data = boot_params.hdr.setup_data;
- while (pa_data) {
- data = early_memremap(pa_data, sizeof(*data));
- memblock_reserve(pa_data, sizeof(*data) + data->len);
- pa_data = data->next;
- early_iounmap(data, sizeof(*data));
- }
-}
-
/*
* --------- Crashkernel reservation ------------------------------
*/
@@ -1001,9 +987,6 @@ void __init setup_arch(char **cmdline_p)
x86_report_nx();
- /* after early param, so could get panic from serial */
- memblock_x86_reserve_range_setup_data();
-
if (acpi_mps_check()) {
#ifdef CONFIG_X86_LOCAL_APIC
disable_apic = 1;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 12/15] x86: kill not used setup_data handling code
@ 2015-03-04 8:00 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Yinghai Lu
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/x86/kernel/kdebugfs.c | 142 ---------------------------------------------
arch/x86/kernel/setup.c | 17 ------
2 files changed, 159 deletions(-)
diff --git a/arch/x86/kernel/kdebugfs.c b/arch/x86/kernel/kdebugfs.c
index dc1404b..c8ca86c 100644
--- a/arch/x86/kernel/kdebugfs.c
+++ b/arch/x86/kernel/kdebugfs.c
@@ -21,142 +21,6 @@ struct dentry *arch_debugfs_dir;
EXPORT_SYMBOL(arch_debugfs_dir);
#ifdef CONFIG_DEBUG_BOOT_PARAMS
-struct setup_data_node {
- u64 paddr;
- u32 type;
- u32 len;
-};
-
-static ssize_t setup_data_read(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct setup_data_node *node = file->private_data;
- unsigned long remain;
- loff_t pos = *ppos;
- struct page *pg;
- void *p;
- u64 pa;
-
- if (pos < 0)
- return -EINVAL;
-
- if (pos >= node->len)
- return 0;
-
- if (count > node->len - pos)
- count = node->len - pos;
-
- pa = node->paddr + sizeof(struct setup_data) + pos;
- pg = pfn_to_page((pa + count - 1) >> PAGE_SHIFT);
- if (PageHighMem(pg)) {
- p = ioremap_cache(pa, count);
- if (!p)
- return -ENXIO;
- } else
- p = __va(pa);
-
- remain = copy_to_user(user_buf, p, count);
-
- if (PageHighMem(pg))
- iounmap(p);
-
- if (remain)
- return -EFAULT;
-
- *ppos = pos + count;
-
- return count;
-}
-
-static const struct file_operations fops_setup_data = {
- .read = setup_data_read,
- .open = simple_open,
- .llseek = default_llseek,
-};
-
-static int __init
-create_setup_data_node(struct dentry *parent, int no,
- struct setup_data_node *node)
-{
- struct dentry *d, *type, *data;
- char buf[16];
-
- sprintf(buf, "%d", no);
- d = debugfs_create_dir(buf, parent);
- if (!d)
- return -ENOMEM;
-
- type = debugfs_create_x32("type", S_IRUGO, d, &node->type);
- if (!type)
- goto err_dir;
-
- data = debugfs_create_file("data", S_IRUGO, d, node, &fops_setup_data);
- if (!data)
- goto err_type;
-
- return 0;
-
-err_type:
- debugfs_remove(type);
-err_dir:
- debugfs_remove(d);
- return -ENOMEM;
-}
-
-static int __init create_setup_data_nodes(struct dentry *parent)
-{
- struct setup_data_node *node;
- struct setup_data *data;
- int error;
- struct dentry *d;
- struct page *pg;
- u64 pa_data;
- int no = 0;
-
- d = debugfs_create_dir("setup_data", parent);
- if (!d)
- return -ENOMEM;
-
- pa_data = boot_params.hdr.setup_data;
-
- while (pa_data) {
- node = kmalloc(sizeof(*node), GFP_KERNEL);
- if (!node) {
- error = -ENOMEM;
- goto err_dir;
- }
-
- pg = pfn_to_page((pa_data+sizeof(*data)-1) >> PAGE_SHIFT);
- if (PageHighMem(pg)) {
- data = ioremap_cache(pa_data, sizeof(*data));
- if (!data) {
- kfree(node);
- error = -ENXIO;
- goto err_dir;
- }
- } else
- data = __va(pa_data);
-
- node->paddr = pa_data;
- node->type = data->type;
- node->len = data->len;
- error = create_setup_data_node(d, no, node);
- pa_data = data->next;
-
- if (PageHighMem(pg))
- iounmap(data);
- if (error)
- goto err_dir;
- no++;
- }
-
- return 0;
-
-err_dir:
- debugfs_remove(d);
- return error;
-}
-
static struct debugfs_blob_wrapper boot_params_blob = {
.data = &boot_params,
.size = sizeof(boot_params),
@@ -181,14 +45,8 @@ static int __init boot_params_kdebugfs_init(void)
if (!data)
goto err_version;
- error = create_setup_data_nodes(dbp);
- if (error)
- goto err_data;
-
return 0;
-err_data:
- debugfs_remove(data);
err_version:
debugfs_remove(version);
err_dir:
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index b1296d3..9a18078 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -481,20 +481,6 @@ static void __init parse_setup_data(void)
boot_params.hdr.setup_data = 0; /* all done */
}
-static void __init memblock_x86_reserve_range_setup_data(void)
-{
- struct setup_data *data;
- u64 pa_data;
-
- pa_data = boot_params.hdr.setup_data;
- while (pa_data) {
- data = early_memremap(pa_data, sizeof(*data));
- memblock_reserve(pa_data, sizeof(*data) + data->len);
- pa_data = data->next;
- early_iounmap(data, sizeof(*data));
- }
-}
-
/*
* --------- Crashkernel reservation ------------------------------
*/
@@ -1001,9 +987,6 @@ void __init setup_arch(char **cmdline_p)
x86_report_nx();
- /* after early param, so could get panic from serial */
- memblock_x86_reserve_range_setup_data();
-
if (acpi_mps_check()) {
#ifdef CONFIG_X86_LOCAL_APIC
disable_apic = 1;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 13/15] x86, pci: convert SETUP_PCI data to list
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (11 preceding siblings ...)
2015-03-04 8:00 ` Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 14/15] x86, boot: copy rom to kernel space Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 15/15] x86, pci: export SETUP_PCI data via sysfs Yinghai Lu
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
So we could avoid ioremap every time later.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/include/asm/pci.h | 2 ++
arch/x86/kernel/setup.c | 1 +
arch/x86/pci/common.c | 77 +++++++++++++++++++++++++++++++++++++---------
3 files changed, 65 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 7fbd5f3..99b261f 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -88,9 +88,11 @@ static inline void pci_dma_burst_advice(struct pci_dev *pdev,
*strategy_parameter = ~0UL;
}
void add_pci(u64 pa_data);
+int fill_setup_pci_entries(void);
#else
static inline void early_quirks(void) { }
static inline void add_pci(u64 pa_data) { }
+static inline int fill_setup_pci_entries(void) { }
#endif
extern void pci_iommu_alloc(void);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9a18078..215bea8 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1190,6 +1190,7 @@ void __init setup_arch(char **cmdline_p)
acpi_boot_init();
sfi_init();
x86_dtb_init();
+ fill_setup_pci_entries();
/*
* get boot-time SMP configuration:
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 4846db7..93577fb 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -668,7 +668,7 @@ unsigned int pcibios_assign_all_busses(void)
return (pci_probe & PCI_ASSIGN_ALL_BUSSES) ? 1 : 0;
}
-static u64 pci_setup_data;
+static u64 pci_setup_data __initdata;
void __init add_pci(u64 pa_data)
{
struct setup_data *data;
@@ -680,36 +680,83 @@ void __init add_pci(u64 pa_data)
early_memunmap(data, sizeof(*data));
}
-int pcibios_add_device(struct pci_dev *dev)
+struct firmware_setup_pci_entry {
+ struct list_head list;
+ uint16_t vendor;
+ uint16_t devid;
+ uint64_t pcilen;
+ unsigned long segment;
+ unsigned long bus;
+ unsigned long device;
+ unsigned long function;
+ phys_addr_t romdata;
+};
+
+static LIST_HEAD(setup_pci_entries);
+
+int __init fill_setup_pci_entries(void)
{
struct setup_data *data;
struct pci_setup_rom *rom;
+ struct firmware_setup_pci_entry *entry;
+ phys_addr_t pa_entry;
u64 pa_data;
pa_data = pci_setup_data;
while (pa_data) {
- data = ioremap(pa_data, sizeof(*rom));
+ data = early_memremap(pa_data, sizeof(*rom));
if (!data)
return -ENOMEM;
-
rom = (struct pci_setup_rom *)data;
- if ((pci_domain_nr(dev->bus) == rom->segment) &&
- (dev->bus->number == rom->bus) &&
- (PCI_SLOT(dev->devfn) == rom->device) &&
- (PCI_FUNC(dev->devfn) == rom->function) &&
- (dev->vendor == rom->vendor) &&
- (dev->device == rom->devid)) {
- dev->rom = pa_data +
- offsetof(struct pci_setup_rom, romdata);
- dev->romlen = rom->pcilen;
+ pa_entry = memblock_alloc(sizeof(*entry), sizeof(long));
+ if (!pa_entry) {
+ early_memunmap(data, sizeof(*rom));
+ return -ENOMEM;
+ }
+
+ entry = phys_to_virt(pa_entry);
+ entry->segment = rom->segment;
+ entry->bus = rom->bus;
+ entry->device = rom->device;
+ entry->function = rom->function;
+ entry->vendor = rom->vendor;
+ entry->devid = rom->devid;
+ entry->pcilen = rom->pcilen;
+ entry->romdata = pa_data +
+ offsetof(struct pci_setup_rom, romdata);
+
+ list_add(&entry->list, &setup_pci_entries);
+
+ memblock_free(pa_data, sizeof(*rom));
+ pa_data = data->next;
+ early_memunmap(data, sizeof(*rom));
+ }
+
+ pci_setup_data = 0;
+
+ return 0;
+}
+
+int pcibios_add_device(struct pci_dev *dev)
+{
+ struct firmware_setup_pci_entry *entry;
+
+ list_for_each_entry(entry, &setup_pci_entries, list) {
+ if ((pci_domain_nr(dev->bus) == entry->segment) &&
+ (dev->bus->number == entry->bus) &&
+ (PCI_SLOT(dev->devfn) == entry->device) &&
+ (PCI_FUNC(dev->devfn) == entry->function) &&
+ (dev->vendor == entry->vendor) &&
+ (dev->device == entry->devid)) {
+ dev->rom = entry->romdata;
+ dev->romlen = entry->pcilen;
dev_printk(KERN_DEBUG, &dev->dev, "set rom to [%#010lx, %#010lx] via SETUP_PCI\n",
(unsigned long)dev->rom,
(unsigned long)(dev->rom + dev->romlen - 1));
}
- pa_data = data->next;
- iounmap(data);
}
+
return 0;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 14/15] x86, boot: copy rom to kernel space
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (12 preceding siblings ...)
2015-03-04 8:00 ` [PATCH v2 13/15] x86, pci: convert SETUP_PCI data to list Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 15/15] x86, pci: export SETUP_PCI data via sysfs Yinghai Lu
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
As EFI stub code could put them high when on 32bit or with exactmap=
on 64bit conf.
Check is the range is mapped, otherwise allocate new one and have
the rom data copied. So we could really avoid ioremap.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/pci/common.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 93577fb..a5de331 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -694,6 +694,48 @@ struct firmware_setup_pci_entry {
static LIST_HEAD(setup_pci_entries);
+static phys_addr_t check_copy(phys_addr_t start, unsigned long size)
+{
+ unsigned long start_pfn = PFN_DOWN(start);
+ unsigned long end_pfn = PFN_UP(start + size);
+ unsigned char *p, *q;
+ phys_addr_t pa_p, pa_q;
+ long sz = size;
+
+ if (pfn_range_is_mapped(start_pfn, end_pfn))
+ return start;
+
+ /* allocate and copy */
+ pa_p = memblock_alloc(size, PAGE_SIZE);
+ if (!pa_p)
+ return start;
+
+ p = phys_to_virt(pa_p);
+
+ pa_q = start;
+ while (sz > 0) {
+ long chunk_size = 64<<10;
+
+ if (chunk_size > sz)
+ chunk_size = sz;
+
+ q = early_memremap(pa_q, chunk_size);
+ if (!q) {
+ memblock_free(pa_p, size);
+ return start;
+ }
+ memcpy(p, q, chunk_size);
+ early_memunmap(q, chunk_size);
+ p += chunk_size;
+ pa_q += chunk_size;
+ sz -= chunk_size;
+ }
+
+ early_memunmap(start, size);
+
+ return pa_p;
+}
+
int __init fill_setup_pci_entries(void)
{
struct setup_data *data;
@@ -723,8 +765,9 @@ int __init fill_setup_pci_entries(void)
entry->vendor = rom->vendor;
entry->devid = rom->devid;
entry->pcilen = rom->pcilen;
- entry->romdata = pa_data +
- offsetof(struct pci_setup_rom, romdata);
+ entry->romdata = check_copy(pa_data +
+ offsetof(struct pci_setup_rom, romdata),
+ rom->pcilen);
list_add(&entry->list, &setup_pci_entries);
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v2 15/15] x86, pci: export SETUP_PCI data via sysfs
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
` (13 preceding siblings ...)
2015-03-04 8:00 ` [PATCH v2 14/15] x86, boot: copy rom to kernel space Yinghai Lu
@ 2015-03-04 8:00 ` Yinghai Lu
14 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 8:00 UTC (permalink / raw)
To: Matt Fleming, H. Peter Anvin, Bjorn Helgaas
Cc: Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, linux-kernel, linux-efi, linux-pci, Yinghai Lu
So we could let kexec-tools to rebuild SETUP_PCI and pass it to
second kernel.
Now kexec-tools already build SETUP_EFI and SETUP_E820EXT.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/pci/common.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 175 insertions(+)
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index a5de331..fb4765b 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -682,6 +682,8 @@ void __init add_pci(u64 pa_data)
struct firmware_setup_pci_entry {
struct list_head list;
+ struct kobject kobj;
+ struct bin_attribute *rom_attr;
uint16_t vendor;
uint16_t devid;
uint64_t pcilen;
@@ -803,6 +805,179 @@ int pcibios_add_device(struct pci_dev *dev)
return 0;
}
+#ifdef CONFIG_SYSFS
+static inline struct firmware_setup_pci_entry *
+to_setup_pci_entry(struct kobject *kobj)
+{
+ return container_of(kobj, struct firmware_setup_pci_entry, kobj);
+}
+
+static ssize_t vendor_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%04llx\n",
+ (unsigned long long)entry->vendor);
+}
+
+static ssize_t devid_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%04llx\n",
+ (unsigned long long)entry->devid);
+}
+
+static ssize_t pcilen_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+ (unsigned long long)entry->pcilen);
+}
+
+static ssize_t segment_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%04llx\n",
+ (unsigned long long)entry->segment);
+}
+
+static ssize_t bus_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%02llx\n",
+ (unsigned long long)entry->bus);
+}
+
+static ssize_t device_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%02llx\n",
+ (unsigned long long)entry->device);
+}
+
+static ssize_t function_show(struct firmware_setup_pci_entry *entry, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "0x%1llx\n",
+ (unsigned long long)entry->function);
+}
+
+struct setup_pci_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct firmware_setup_pci_entry *entry, char *buf);
+};
+
+static inline struct setup_pci_attribute *to_setup_pci_attr(
+ struct attribute *attr)
+{
+ return container_of(attr, struct setup_pci_attribute, attr);
+}
+
+static ssize_t setup_pci_attr_show(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ struct firmware_setup_pci_entry *entry = to_setup_pci_entry(kobj);
+ struct setup_pci_attribute *setup_pci_attr = to_setup_pci_attr(attr);
+
+ return setup_pci_attr->show(entry, buf);
+}
+
+static struct setup_pci_attribute setup_pci_vendor_attr = __ATTR_RO(vendor);
+static struct setup_pci_attribute setup_pci_devid_attr = __ATTR_RO(devid);
+static struct setup_pci_attribute setup_pci_pcilen_attr = __ATTR_RO(pcilen);
+static struct setup_pci_attribute setup_pci_segment_attr = __ATTR_RO(segment);
+static struct setup_pci_attribute setup_pci_bus_attr = __ATTR_RO(bus);
+static struct setup_pci_attribute setup_pci_device_attr = __ATTR_RO(device);
+static struct setup_pci_attribute setup_pci_function_attr = __ATTR_RO(function);
+
+/*
+ * These are default attributes that are added for every memmap entry.
+ */
+static struct attribute *def_attrs[] = {
+ &setup_pci_vendor_attr.attr,
+ &setup_pci_devid_attr.attr,
+ &setup_pci_pcilen_attr.attr,
+ &setup_pci_segment_attr.attr,
+ &setup_pci_bus_attr.attr,
+ &setup_pci_device_attr.attr,
+ &setup_pci_function_attr.attr,
+ NULL
+};
+
+static const struct sysfs_ops setup_pci_attr_ops = {
+ .show = setup_pci_attr_show,
+};
+
+static struct kobj_type __refdata setup_pci_ktype = {
+ .sysfs_ops = &setup_pci_attr_ops,
+ .default_attrs = def_attrs,
+};
+
+static ssize_t setup_pci_rom_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct firmware_setup_pci_entry *entry = to_setup_pci_entry(kobj);
+
+ if (off >= entry->pcilen)
+ count = 0;
+ else {
+ unsigned char *rom = phys_to_virt(entry->romdata);
+
+ if (off + count > entry->pcilen)
+ count = entry->pcilen - off;
+
+ memcpy(buf, rom + off, count);
+ }
+
+ return count;
+}
+
+static int __init add_sysfs_fw_setup_pci_entry(
+ struct firmware_setup_pci_entry *entry)
+{
+ int retval = 0;
+ static int setup_pci_entries_nr;
+ static struct kset *setup_pci_kset;
+ struct bin_attribute *attr;
+
+ kobject_init(&entry->kobj, &setup_pci_ktype);
+
+ if (!setup_pci_kset) {
+ setup_pci_kset = kset_create_and_add("setup_pci", NULL,
+ firmware_kobj);
+ if (!setup_pci_kset)
+ return -ENOMEM;
+ }
+
+ entry->kobj.kset = setup_pci_kset;
+ retval = kobject_add(&entry->kobj, NULL, "%d", setup_pci_entries_nr++);
+ if (retval) {
+ kobject_put(&entry->kobj);
+ return retval;
+ }
+
+ attr = kzalloc(sizeof(*attr), GFP_ATOMIC);
+ if (!attr)
+ return -ENOMEM;
+
+ sysfs_bin_attr_init(attr);
+ attr->size = entry->pcilen;
+ attr->attr.name = "rom";
+ attr->attr.mode = S_IRUSR;
+ attr->read = setup_pci_rom_read;
+ retval = sysfs_create_bin_file(&entry->kobj, attr);
+ if (retval)
+ kfree(attr);
+ entry->rom_attr = attr;
+
+ return retval;
+}
+
+static int __init firmware_setup_pci_init(void)
+{
+ struct firmware_setup_pci_entry *entry;
+
+ list_for_each_entry(entry, &setup_pci_entries, list)
+ add_sysfs_fw_setup_pci_entry(entry);
+
+ return 0;
+}
+late_initcall(firmware_setup_pci_init);
+#endif
+
int pcibios_enable_device(struct pci_dev *dev, int mask)
{
int err;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 8:00 ` Yinghai Lu
(?)
@ 2015-03-04 10:16 ` Borislav Petkov
2015-03-04 15:54 ` Jiri Kosina
2015-03-04 18:06 ` Yinghai Lu
-1 siblings, 2 replies; 55+ messages in thread
From: Borislav Petkov @ 2015-03-04 10:16 UTC (permalink / raw)
To: Yinghai Lu
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Borislav Petkov, Baoquan He,
linux-kernel, linux-efi, linux-pci, Kees Cook
On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> is using address as value for kaslr_enabled.
>
> That will random kaslr_enabled get that set or cleared.
> Will have problem for system really have kaslr enabled.
>
> -v2: update changelog.
This is still not good enough. Please do this:
In commit f47233c2d34f we did A. The problem with that is B. Change the
code to do C.
Now you only have to fill out the A,B and C variables with the
respective text which is understandable even for people who don't know
this code.
>
> Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> Cc: Matt Fleming <matt.fleming@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
> arch/x86/kernel/setup.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 98dc931..05d444f 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -429,7 +429,13 @@ static void __init reserve_initrd(void)
>
> static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
> {
> - kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
> + /* kaslr_setup_data is defined in aslr.c */
> + unsigned char *data;
> + unsigned long offset = sizeof(struct setup_data);
> +
> + data = early_memremap(pa_data, offset + 1);
early_memremap() needs its retval checked before accessing it.
> + kaslr_enabled = *(data + offset);
> + early_memunmap(data, offset + 1);
> }
>
> static void __init parse_setup_data(void)
> --
> 1.8.4.5
>
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 10:16 ` Borislav Petkov
@ 2015-03-04 15:54 ` Jiri Kosina
2015-03-04 18:12 ` Yinghai Lu
2015-03-04 18:06 ` Yinghai Lu
1 sibling, 1 reply; 55+ messages in thread
From: Jiri Kosina @ 2015-03-04 15:54 UTC (permalink / raw)
To: Borislav Petkov
Cc: Yinghai Lu, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Baoquan He,
linux-kernel, linux-efi, linux-pci, Kees Cook
On Wed, 4 Mar 2015, Borislav Petkov wrote:
> On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
> > commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> > is using address as value for kaslr_enabled.
> >
> > That will random kaslr_enabled get that set or cleared.
> > Will have problem for system really have kaslr enabled.
> >
> > -v2: update changelog.
>
> This is still not good enough. Please do this:
>
> In commit f47233c2d34f we did A. The problem with that is B. Change the
> code to do C.
>
> Now you only have to fill out the A,B and C variables with the
> respective text which is understandable even for people who don't know
> this code.
Also this 15-patch series needs to be separated into two patchsets. The
whole series is not appropriate for -rc3, but this particular one at least
is a regression fix that has to go in.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 10:16 ` Borislav Petkov
2015-03-04 15:54 ` Jiri Kosina
@ 2015-03-04 18:06 ` Yinghai Lu
2015-03-04 18:56 ` Yinghai Lu
2015-03-04 20:00 ` Ingo Molnar
1 sibling, 2 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 18:06 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Borislav Petkov, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Wed, Mar 4, 2015 at 2:16 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
>> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
>> is using address as value for kaslr_enabled.
>>
>> That will random kaslr_enabled get that set or cleared.
>> Will have problem for system really have kaslr enabled.
>>
>> -v2: update changelog.
>
> This is still not good enough. Please do this:
>
> In commit f47233c2d34f we did A. The problem with that is B. Change the
> code to do C.
>
> Now you only have to fill out the A,B and C variables with the
> respective text which is understandable even for people who don't know
> this code.
>
I don't know, that is trivial and obvious.
the old code use address as value instead of using reference...
>>
>>
>> static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
>> {
>> - kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
>> + /* kaslr_setup_data is defined in aslr.c */
>> + unsigned char *data;
>> + unsigned long offset = sizeof(struct setup_data);
>> +
>> + data = early_memremap(pa_data, offset + 1);
>
> early_memremap() needs its retval checked before accessing it.
>
will fix that.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 18:12 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 18:12 UTC (permalink / raw)
To: Jiri Kosina
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Wed, Mar 4, 2015 at 7:54 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>
> Also this 15-patch series needs to be separated into two patchsets. The
> whole series is not appropriate for -rc3, but this particular one at least
> is a regression fix that has to go in.
The first 4 should go v4.0.
could leave others to v4.1
Thanks
Yinghai
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 18:12 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 18:12 UTC (permalink / raw)
To: Jiri Kosina
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Baoquan He,
Linux Kernel Mailing List, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
On Wed, Mar 4, 2015 at 7:54 AM, Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> wrote:
>
> Also this 15-patch series needs to be separated into two patchsets. The
> whole series is not appropriate for -rc3, but this particular one at least
> is a regression fix that has to go in.
The first 4 should go v4.0.
could leave others to v4.1
Thanks
Yinghai
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 18:06 ` Yinghai Lu
@ 2015-03-04 18:56 ` Yinghai Lu
2015-03-04 20:00 ` Ingo Molnar
1 sibling, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 18:56 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Borislav Petkov, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Wed, Mar 4, 2015 at 10:06 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, Mar 4, 2015 at 2:16 AM, Borislav Petkov <bp@alien8.de> wrote:
>> On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
>>> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
>>> is using address as value for kaslr_enabled.
>>>
>>> That will random kaslr_enabled get that set or cleared.
>>> Will have problem for system really have kaslr enabled.
>>>
>>> -v2: update changelog.
>>
>> This is still not good enough. Please do this:
>>
>> In commit f47233c2d34f we did A. The problem with that is B. Change the
>> code to do C.
>>
>> Now you only have to fill out the A,B and C variables with the
>> respective text which is understandable even for people who don't know
>> this code.
Please check if it is ok:
Subject: [PATCH v3] x86, kaslr: get kaslr_enabled back correctly
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
is using address as value for kaslr_enabled.
That will get wrong value for kaslr_enabled, so have problem for system really
have kaslr enabled.
This patch change to using early map and accessing the value.
-v3: add checking about early_memmap according to bp.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 19:41 ` Ingo Molnar
0 siblings, 0 replies; 55+ messages in thread
From: Ingo Molnar @ 2015-03-04 19:41 UTC (permalink / raw)
To: Yinghai Lu
Cc: Jiri Kosina, Borislav Petkov, Matt Fleming, H. Peter Anvin,
Bjorn Helgaas, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Kees Cook
* Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, Mar 4, 2015 at 7:54 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>
> >
> > Also this 15-patch series needs to be separated into two patchsets. The
> > whole series is not appropriate for -rc3, but this particular one at least
> > is a regression fix that has to go in.
>
> The first 4 should go v4.0.
>
> could leave others to v4.1
Then please submit the first 4 only for the time being, and submit the
rest once Boris has accepted and applied the fixes.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 19:41 ` Ingo Molnar
0 siblings, 0 replies; 55+ messages in thread
From: Ingo Molnar @ 2015-03-04 19:41 UTC (permalink / raw)
To: Yinghai Lu
Cc: Jiri Kosina, Borislav Petkov, Matt Fleming, H. Peter Anvin,
Bjorn Helgaas, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
* Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Mar 4, 2015 at 7:54 AM, Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> wrote:
>
> >
> > Also this 15-patch series needs to be separated into two patchsets. The
> > whole series is not appropriate for -rc3, but this particular one at least
> > is a regression fix that has to go in.
>
> The first 4 should go v4.0.
>
> could leave others to v4.1
Then please submit the first 4 only for the time being, and submit the
rest once Boris has accepted and applied the fixes.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 20:00 ` Ingo Molnar
0 siblings, 0 replies; 55+ messages in thread
From: Ingo Molnar @ 2015-03-04 20:00 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Kees Cook
* Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, Mar 4, 2015 at 2:16 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
> >> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> >> is using address as value for kaslr_enabled.
> >>
> >> That will random kaslr_enabled get that set or cleared.
> >> Will have problem for system really have kaslr enabled.
> >>
> >> -v2: update changelog.
> >
> > This is still not good enough. Please do this:
> >
> > In commit f47233c2d34f we did A. The problem with that is B. Change the
> > code to do C.
> >
> > Now you only have to fill out the A,B and C variables with the
> > respective text which is understandable even for people who don't know
> > this code.
> >
>
> I don't know, that is trivial and obvious.
The fix might be obvious, the effects of the bug are not obvious at
all, as you yourself show that you don't understand your own change,
which is evident from the changelog you've written:
> Please check if it is ok:
>
> Subject: [PATCH v3] x86, kaslr: get kaslr_enabled back correctly
Missing capitalization.
>
> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> is using address as value for kaslr_enabled.
Missing capitalization. Do you really expect maintainers to fix up
every single sentence of yours??
> That will get wrong value for kaslr_enabled, so have problem for
> system really have kaslr enabled.
This sentence does not parse, nor is it correct: the bug isn't just
triggering on systems that want to have kaslr enabled - but also on
bootloaders that happen to pass in a kaslr boot parameter but have the
switch value disabled...
You also need to point out the important fact that bootloaders that
don't try to use the kaslr extension (i.e. that don't use SETUP_KASLR)
work just fine - this is why the bug was not noticed to begin with,
i.e. the overwhelming majority of systems out there.
> This patch change to using early map and accessing the value.
s/change to using/changes the code to use/
It is totally unacceptable that you don't do proper analysis of the
patches you submit, and that you don't bother writing proper, readable
changelogs.
Your flippant "that is trivial and obvious" attitude towards
changelogs is unacceptable as well. And this is not about English
knowledge: missing capitalization is a very simple concept any
beginning coder should be able to graps the first time it's pointed
out... yet for the past 3 years half of your patches had totally
careless, often unreadable changelogs.
These subpar changelogs and patches show plain laziness, sloppiness
and lack of care to write clean changelogs - and that sloppiness not
only makes it much harder for maintainers to process your patches, but
also tends to creep over into your patches as well, causing repeat
problems again and again...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-04 20:00 ` Ingo Molnar
0 siblings, 0 replies; 55+ messages in thread
From: Ingo Molnar @ 2015-03-04 20:00 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
* Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Mar 4, 2015 at 2:16 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
> > On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
> >> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> >> is using address as value for kaslr_enabled.
> >>
> >> That will random kaslr_enabled get that set or cleared.
> >> Will have problem for system really have kaslr enabled.
> >>
> >> -v2: update changelog.
> >
> > This is still not good enough. Please do this:
> >
> > In commit f47233c2d34f we did A. The problem with that is B. Change the
> > code to do C.
> >
> > Now you only have to fill out the A,B and C variables with the
> > respective text which is understandable even for people who don't know
> > this code.
> >
>
> I don't know, that is trivial and obvious.
The fix might be obvious, the effects of the bug are not obvious at
all, as you yourself show that you don't understand your own change,
which is evident from the changelog you've written:
> Please check if it is ok:
>
> Subject: [PATCH v3] x86, kaslr: get kaslr_enabled back correctly
Missing capitalization.
>
> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> is using address as value for kaslr_enabled.
Missing capitalization. Do you really expect maintainers to fix up
every single sentence of yours??
> That will get wrong value for kaslr_enabled, so have problem for
> system really have kaslr enabled.
This sentence does not parse, nor is it correct: the bug isn't just
triggering on systems that want to have kaslr enabled - but also on
bootloaders that happen to pass in a kaslr boot parameter but have the
switch value disabled...
You also need to point out the important fact that bootloaders that
don't try to use the kaslr extension (i.e. that don't use SETUP_KASLR)
work just fine - this is why the bug was not noticed to begin with,
i.e. the overwhelming majority of systems out there.
> This patch change to using early map and accessing the value.
s/change to using/changes the code to use/
It is totally unacceptable that you don't do proper analysis of the
patches you submit, and that you don't bother writing proper, readable
changelogs.
Your flippant "that is trivial and obvious" attitude towards
changelogs is unacceptable as well. And this is not about English
knowledge: missing capitalization is a very simple concept any
beginning coder should be able to graps the first time it's pointed
out... yet for the past 3 years half of your patches had totally
careless, often unreadable changelogs.
These subpar changelogs and patches show plain laziness, sloppiness
and lack of care to write clean changelogs - and that sloppiness not
only makes it much harder for maintainers to process your patches, but
also tends to creep over into your patches as well, causing repeat
problems again and again...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 20:00 ` Ingo Molnar
(?)
@ 2015-03-04 21:32 ` Yinghai Lu
2015-03-06 13:33 ` Borislav Petkov
-1 siblings, 1 reply; 55+ messages in thread
From: Yinghai Lu @ 2015-03-04 21:32 UTC (permalink / raw)
To: Ingo Molnar
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Kees Cook
On Wed, Mar 4, 2015 at 12:00 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> It is totally unacceptable that you don't do proper analysis of the
> patches you submit, and that you don't bother writing proper, readable
> changelogs.
Sorry, please check it again:
Subject: [PATCH v4] x86, kaslr: Get kaslr_enabled back correctly
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
is using address as value for kaslr_enabled.
That will get wrong value back for kaslr_enabled in kernel stage.
1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled
get set wrongly in setup.c, late in module.c::get_module_load_offset
will return not wanted random module load offset.
That change behavior when HIBERNATION is defined or nokaslr is passed.
2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled
get cleared wrongly in setup.c, late in module.c::get_module_load_offset
will not return wanted random module load offset.
This patch changes the code to use early_memmap and access the value,
and will keep boot and kernel consistent with kaslr.
-v3: add checking return from early_memmap according to bp.
Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Jiri Kosina <jkosina@suse.cz>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/x86/kernel/setup.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -429,7 +429,18 @@ static void __init reserve_initrd(void)
static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
{
- kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ if (!data) {
+ kaslr_enabled = true;
+ return;
+ }
+
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 18:12 ` Yinghai Lu
(?)
(?)
@ 2015-03-05 2:58 ` joeyli
2015-03-05 3:20 ` Yinghai Lu
-1 siblings, 1 reply; 55+ messages in thread
From: joeyli @ 2015-03-05 2:58 UTC (permalink / raw)
To: Yinghai Lu
Cc: Jiri Kosina, Borislav Petkov, Matt Fleming, H. Peter Anvin,
Bjorn Helgaas, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Kees Cook
Hi Yinghai,
On Wed, Mar 04, 2015 at 10:12:58AM -0800, Yinghai Lu wrote:
> On Wed, Mar 4, 2015 at 7:54 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>
> >
> > Also this 15-patch series needs to be separated into two patchsets. The
> > whole series is not appropriate for -rc3, but this particular one at least
> > is a regression fix that has to go in.
>
> The first 4 should go v4.0.
>
> could leave others to v4.1
>
> Thanks
>
> Yinghai
After 84c91b7ae merged to v3.17 kernel, hibernate code checks the e280 regions
should not be changed when doing hibernate resume. Without your patch 8,
the hibernate resume checking will randomly fail on the machines that reserved
setup_data in e820 regions.
Could you please consider to put "[PATCH v2 08/15] x86: Kill E820_RESERVED_KERN"
to v4.0 kernel?
Thanks a lot!
Joey Lee
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-05 2:58 ` joeyli
@ 2015-03-05 3:20 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-05 3:20 UTC (permalink / raw)
To: joeyli
Cc: Jiri Kosina, Borislav Petkov, Matt Fleming, H. Peter Anvin,
Bjorn Helgaas, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Kees Cook
On Wed, Mar 4, 2015 at 6:58 PM, joeyli <jlee@suse.com> wrote:
>
> After 84c91b7ae merged to v3.17 kernel, hibernate code checks the e280 regions
> should not be changed when doing hibernate resume. Without your patch 8,
> the hibernate resume checking will randomly fail on the machines that reserved
> setup_data in e820 regions.
>
> Could you please consider to put "[PATCH v2 08/15] x86: Kill E820_RESERVED_KERN"
> to v4.0 kernel?
That will trigger SETUP_PCI ioremap warning.
That is the reason I want to put it with other setup_data fix.
Thanks
Yinghai
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-04 21:32 ` Yinghai Lu
@ 2015-03-06 13:33 ` Borislav Petkov
2015-03-06 17:49 ` Yinghai Lu
2015-03-06 19:50 ` Yinghai Lu
0 siblings, 2 replies; 55+ messages in thread
From: Borislav Petkov @ 2015-03-06 13:33 UTC (permalink / raw)
To: Yinghai Lu
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Wed, Mar 04, 2015 at 01:32:53PM -0800, Yinghai Lu wrote:
> On Wed, Mar 4, 2015 at 12:00 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > It is totally unacceptable that you don't do proper analysis of the
> > patches you submit, and that you don't bother writing proper, readable
> > changelogs.
>
> Sorry, please check it again:
>
> Subject: [PATCH v4] x86, kaslr: Get kaslr_enabled back correctly
Subject: x86/kaslr: Access the correct kaslr_enabled variable
> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> is using address as value for kaslr_enabled.
"commit ... started passing KASLR status to kernel proper."
> That will get wrong value back for kaslr_enabled in kernel stage.
> 1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled
> get set wrongly in setup.c, late in module.c::get_module_load_offset
> will return not wanted random module load offset.
> That change behavior when HIBERNATION is defined or nokaslr is passed.
>
> 2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled
> get cleared wrongly in setup.c, late in module.c::get_module_load_offset
> will not return wanted random module load offset.
>
> This patch changes the code to use early_memmap and access the value,
> and will keep boot and kernel consistent with kaslr.
Replace all that with:
"However, the setup_data linked list and thus the element which contains
kaslr_enabled is chained together using physical addresses. At the
time when we access it in the kernel proper, we're already running
with paging enabled and therefore must access it through its virtual
address."
That's it, now how hard was to explain it that way?
> -v3: add checking return from early_memmap according to bp.
I guess with "bp" you mean me? You can call me Boris.
> Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> Cc: Matt Fleming <matt.fleming@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> arch/x86/kernel/setup.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/arch/x86/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/setup.c
> +++ linux-2.6/arch/x86/kernel/setup.c
> @@ -429,7 +429,18 @@ static void __init reserve_initrd(void)
>
> static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
> {
> - kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
> + /* kaslr_setup_data is defined in aslr.c */
> + unsigned char *data;
> + unsigned long offset = sizeof(struct setup_data);
> +
> + data = early_memremap(pa_data, offset + 1);
> + if (!data) {
> + kaslr_enabled = true;
> + return;
> + }
> +
> + kaslr_enabled = *(data + offset);
> + early_memunmap(data, offset + 1);
> }
>
> static void __init parse_setup_data(void)
Please use checkpatch before submitting patches:
WARNING: please, no spaces at the start of a line
#71: FILE: arch/x86/kernel/setup.c:433:
+ unsigned char *data;$
WARNING: please, no spaces at the start of a line
#72: FILE: arch/x86/kernel/setup.c:434:
+ unsigned long offset = sizeof(struct setup_data);$
WARNING: please, no spaces at the start of a line
#74: FILE: arch/x86/kernel/setup.c:436:
+ data = early_memremap(pa_data, offset + 1);$
WARNING: please, no spaces at the start of a line
#75: FILE: arch/x86/kernel/setup.c:437:
+ if (!data) {$
ERROR: code indent should use tabs where possible
#76: FILE: arch/x86/kernel/setup.c:438:
+ kaslr_enabled = true;$
WARNING: please, no spaces at the start of a line
#76: FILE: arch/x86/kernel/setup.c:438:
+ kaslr_enabled = true;$
ERROR: code indent should use tabs where possible
#77: FILE: arch/x86/kernel/setup.c:439:
+ return;$
WARNING: please, no spaces at the start of a line
#77: FILE: arch/x86/kernel/setup.c:439:
+ return;$
WARNING: please, no spaces at the start of a line
#78: FILE: arch/x86/kernel/setup.c:440:
+ }$
WARNING: please, no spaces at the start of a line
#80: FILE: arch/x86/kernel/setup.c:442:
+ kaslr_enabled = *(data + offset);$
WARNING: please, no spaces at the start of a line
#81: FILE: arch/x86/kernel/setup.c:443:
+ early_memunmap(data, offset + 1);$
total: 2 errors, 9 warnings, 19 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
Your patch has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
2015-03-04 8:00 ` [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size Yinghai Lu
@ 2015-03-06 13:55 ` Borislav Petkov
2015-03-06 18:44 ` Yinghai Lu
0 siblings, 1 reply; 55+ messages in thread
From: Borislav Petkov @ 2015-03-06 13:55 UTC (permalink / raw)
To: Yinghai Lu
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Borislav Petkov, Baoquan He,
linux-kernel, linux-efi, linux-pci, Josh Triplett, Kees Cook,
Andrew Morton, Ard Biesheuvel, Junjie Mao
On Wed, Mar 04, 2015 at 12:00:34AM -0800, Yinghai Lu wrote:
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>
> introduced one run_size for kaslr.
>
> We do not need to have home grown run_size.
>
> We should use real runtime size (include copy/decompress) aka init_size
Why?
I can see why but you need to explain it here. Also, look at the
e6023367d779 commit message. Now this is how you do a commit message!
Especially if it is early boot code, you explain stuff. Junjie even went
the distance and did a nice graphic.
So please redo your commit message.
And saying it is trivial and obvious will not get you anywhere, as
you've noticed already.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 02/15] x86, boot: move ZO to end of buffer
2015-03-04 8:00 ` [PATCH v2 02/15] x86, boot: move ZO to end of buffer Yinghai Lu
@ 2015-03-06 13:58 ` Borislav Petkov
0 siblings, 0 replies; 55+ messages in thread
From: Borislav Petkov @ 2015-03-06 13:58 UTC (permalink / raw)
To: Yinghai Lu
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Baoquan He, linux-kernel, linux-efi,
linux-pci, Kees Cook
On Wed, Mar 04, 2015 at 12:00:35AM -0800, Yinghai Lu wrote:
> bp found data from boot stage can not be used kernel stage.
>
> Actually those data area is overlapped with VO kernel bss stage, and clear_bss()
"VO kernel bss stage"?
I'm sure you can think of a better explanation. Right now I'm thinking
of "Video Only" or "Virtually Omnipresent" or ... I can go all day.
Let's be more precise here please.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 17:49 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 17:49 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp@suse.de> wrote:
> Please use checkpatch before submitting patches:
>
> WARNING: please, no spaces at the start of a line
> #71: FILE: arch/x86/kernel/setup.c:433:
> + unsigned char *data;$
>
> WARNING: please, no spaces at the start of a line
> #72: FILE: arch/x86/kernel/setup.c:434:
> + unsigned long offset = sizeof(struct setup_data);$
>
> WARNING: please, no spaces at the start of a line
> #74: FILE: arch/x86/kernel/setup.c:436:
> + data = early_memremap(pa_data, offset + 1);$
>
> WARNING: please, no spaces at the start of a line
> #75: FILE: arch/x86/kernel/setup.c:437:
> + if (!data) {$
>
> ERROR: code indent should use tabs where possible
> #76: FILE: arch/x86/kernel/setup.c:438:
> + kaslr_enabled = true;$
>
> WARNING: please, no spaces at the start of a line
> #76: FILE: arch/x86/kernel/setup.c:438:
> + kaslr_enabled = true;$
>
> ERROR: code indent should use tabs where possible
> #77: FILE: arch/x86/kernel/setup.c:439:
> + return;$
>
> WARNING: please, no spaces at the start of a line
> #77: FILE: arch/x86/kernel/setup.c:439:
> + return;$
>
> WARNING: please, no spaces at the start of a line
> #78: FILE: arch/x86/kernel/setup.c:440:
> + }$
>
> WARNING: please, no spaces at the start of a line
> #80: FILE: arch/x86/kernel/setup.c:442:
> + kaslr_enabled = *(data + offset);$
>
> WARNING: please, no spaces at the start of a line
> #81: FILE: arch/x86/kernel/setup.c:443:
> + early_memunmap(data, offset + 1);$
>
> total: 2 errors, 9 warnings, 19 lines checked
>
> NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
> scripts/cleanfile
>
> Your patch has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
That is "copy and paste" instead of attachment for easy review.
but gmail web client convert tab to spaces.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 17:49 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 17:49 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> wrote:
> Please use checkpatch before submitting patches:
>
> WARNING: please, no spaces at the start of a line
> #71: FILE: arch/x86/kernel/setup.c:433:
> + unsigned char *data;$
>
> WARNING: please, no spaces at the start of a line
> #72: FILE: arch/x86/kernel/setup.c:434:
> + unsigned long offset = sizeof(struct setup_data);$
>
> WARNING: please, no spaces at the start of a line
> #74: FILE: arch/x86/kernel/setup.c:436:
> + data = early_memremap(pa_data, offset + 1);$
>
> WARNING: please, no spaces at the start of a line
> #75: FILE: arch/x86/kernel/setup.c:437:
> + if (!data) {$
>
> ERROR: code indent should use tabs where possible
> #76: FILE: arch/x86/kernel/setup.c:438:
> + kaslr_enabled = true;$
>
> WARNING: please, no spaces at the start of a line
> #76: FILE: arch/x86/kernel/setup.c:438:
> + kaslr_enabled = true;$
>
> ERROR: code indent should use tabs where possible
> #77: FILE: arch/x86/kernel/setup.c:439:
> + return;$
>
> WARNING: please, no spaces at the start of a line
> #77: FILE: arch/x86/kernel/setup.c:439:
> + return;$
>
> WARNING: please, no spaces at the start of a line
> #78: FILE: arch/x86/kernel/setup.c:440:
> + }$
>
> WARNING: please, no spaces at the start of a line
> #80: FILE: arch/x86/kernel/setup.c:442:
> + kaslr_enabled = *(data + offset);$
>
> WARNING: please, no spaces at the start of a line
> #81: FILE: arch/x86/kernel/setup.c:443:
> + early_memunmap(data, offset + 1);$
>
> total: 2 errors, 9 warnings, 19 lines checked
>
> NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
> scripts/cleanfile
>
> Your patch has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
That is "copy and paste" instead of attachment for easy review.
but gmail web client convert tab to spaces.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
2015-03-06 13:55 ` Borislav Petkov
@ 2015-03-06 18:44 ` Yinghai Lu
2015-03-06 18:55 ` Kees Cook
0 siblings, 1 reply; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 18:44 UTC (permalink / raw)
To: Borislav Petkov
Cc: Matt Fleming, H. Peter Anvin, Bjorn Helgaas, Thomas Gleixner,
Ingo Molnar, Jiri Kosina, Borislav Petkov, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Josh Triplett,
Kees Cook, Andrew Morton, Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 5:55 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Mar 04, 2015 at 12:00:34AM -0800, Yinghai Lu wrote:
>> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>>
>> introduced one run_size for kaslr.
>>
>> We do not need to have home grown run_size.
>>
>> We should use real runtime size (include copy/decompress) aka init_size
>
> Why?
New change log:
Subject: [PATCH] x86, kaslr: Use init_size instead of run_size
commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
introduced one run_size for kaslr.
We should use real runtime size (include copy/decompress) aka init_size.
run_size is size of VO (vmlinux).
init_size is the size needed for decompress and it is bigger than run_size
when decompress need more buff.
According to arch/x86/boot/header.S:
| #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
| #define VO_INIT_SIZE (VO__end - VO__text)
| #if ZO_INIT_SIZE > VO_INIT_SIZE
| #define INIT_SIZE ZO_INIT_SIZE
| #else
| #define INIT_SIZE VO_INIT_SIZE
| #endif
| init_size: .long INIT_SIZE # kernel initialization size
Bootloader allocate buffer according to init_size in hdr, and load the
ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
During running of ZO, ZO move itself to the middle of buffer at
z_extract_offset to make sure that decompressor would not have output
overwrite input data before input data get consumed.
But z_extract_offset calculating is based on size of VO (vmlinux) and size
of compressed VO only at first.
So need to make [z_extra_offset, init_size) will fit ZO, that means
init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.
During aslr buffer searching, we need to make sure the buffer is bigger
enough for decompress at first. So use init_size instead, and kill not
needed run_size related code.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-06 18:55 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2015-03-06 18:55 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Josh Triplett, Andrew Morton, Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 10:44 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Mar 6, 2015 at 5:55 AM, Borislav Petkov <bp@alien8.de> wrote:
>> On Wed, Mar 04, 2015 at 12:00:34AM -0800, Yinghai Lu wrote:
>>> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>>>
>>> introduced one run_size for kaslr.
>>>
>>> We do not need to have home grown run_size.
>>>
>>> We should use real runtime size (include copy/decompress) aka init_size
>>
>> Why?
>
> New change log:
>
> Subject: [PATCH] x86, kaslr: Use init_size instead of run_size
>
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
>
> run_size is size of VO (vmlinux).
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
>
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE (VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the buffer is bigger
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
I don't see how bss and brk are related to these sizes. Can you
explain how bss, brk, and initrd factor into these sizes? Those were
what run_size was created to represent. I don't want to accidentally
start stomping on bss and brk again. :)
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-06 18:55 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2015-03-06 18:55 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Josh Triplett, Andrew Morton,
Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 10:44 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Mar 6, 2015 at 5:55 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
>> On Wed, Mar 04, 2015 at 12:00:34AM -0800, Yinghai Lu wrote:
>>> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>>>
>>> introduced one run_size for kaslr.
>>>
>>> We do not need to have home grown run_size.
>>>
>>> We should use real runtime size (include copy/decompress) aka init_size
>>
>> Why?
>
> New change log:
>
> Subject: [PATCH] x86, kaslr: Use init_size instead of run_size
>
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
>
> run_size is size of VO (vmlinux).
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
>
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE (VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the buffer is bigger
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
I don't see how bss and brk are related to these sizes. Can you
explain how bss, brk, and initrd factor into these sizes? Those were
what run_size was created to represent. I don't want to accidentally
start stomping on bss and brk again. :)
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
2015-03-06 18:55 ` Kees Cook
(?)
@ 2015-03-06 19:28 ` Yinghai Lu
2015-03-06 19:56 ` Kees Cook
-1 siblings, 1 reply; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 19:28 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Josh Triplett, Andrew Morton, Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 10:55 AM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Mar 6, 2015 at 10:44 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> I don't see how bss and brk are related to these sizes. Can you
> explain how bss, brk, and initrd factor into these sizes? Those were
> what run_size was created to represent. I don't want to accidentally
> start stomping on bss and brk again. :)
VO (vlinux) init size aka VO_INIT_SIZE already inlude that.
Please check update version.
commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
introduced one run_size for kaslr.
We should use real runtime size (include copy/decompress) aka init_size.
run_size is VO (vmlinux) init size include bss and brk.
init_size is the size needed for decompress and it is bigger than run_size
when decompress need more buff.
According to arch/x86/boot/header.S:
| #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
| #define VO_INIT_SIZE (VO__end - VO__text)
| #if ZO_INIT_SIZE > VO_INIT_SIZE
| #define INIT_SIZE ZO_INIT_SIZE
| #else
| #define INIT_SIZE VO_INIT_SIZE
| #endif
| init_size: .long INIT_SIZE # kernel initialization size
Bootloader allocate buffer according to init_size in hdr, and load the
ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
init_size first should come from VO (vmlinux) init size. That VO init size
is from VO _end to VO _end and include VO bss and brk area.
During running of ZO, ZO move itself to the middle of buffer at
z_extract_offset to make sure that decompressor would not have output
overwrite input data before input data get consumed.
But z_extract_offset calculating is based on size of VO (vmlinux) and size
of compressed VO only at first.
So need to make sure [z_extra_offset, init_size) will fit ZO, that means
init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.
During aslr buffer searching, we need to make sure the buffer is bigger
enough for decompress at first. So use init_size instead, and kill not
needed run_size related code.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 19:50 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 19:50 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp@suse.de> wrote:
>
> "However, the setup_data linked list and thus the element which contains
> kaslr_enabled is chained together using physical addresses. At the
> time when we access it in the kernel proper, we're already running
> with paging enabled and therefore must access it through its virtual
> address."
>
> That's it, now how hard was to explain it that way?
No, I don't think your change log is right.
Actually the old code is using address as value.
if the old code would be like:
kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data)));
then your change log would be good, but the old code is
kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 19:50 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 19:50 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> wrote:
>
> "However, the setup_data linked list and thus the element which contains
> kaslr_enabled is chained together using physical addresses. At the
> time when we access it in the kernel proper, we're already running
> with paging enabled and therefore must access it through its virtual
> address."
>
> That's it, now how hard was to explain it that way?
No, I don't think your change log is right.
Actually the old code is using address as value.
if the old code would be like:
kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data)));
then your change log would be good, but the old code is
kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 19:53 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 19:53 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 6, 2015 at 11:50 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp@suse.de> wrote:
>
>>
>> "However, the setup_data linked list and thus the element which contains
>> kaslr_enabled is chained together using physical addresses. At the
>> time when we access it in the kernel proper, we're already running
>> with paging enabled and therefore must access it through its virtual
>> address."
>>
>> That's it, now how hard was to explain it that way?
>
> No, I don't think your change log is right.
>
> Actually the old code is using address as value.
>
> if the old code would be like:
>
> kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data)));
>
> then your change log would be good, but the old code is
>
> kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
Please check if you are ok with this:
Subject: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
started passing KASLR status to kernel proper, but it uses address as
the vaule.
That will get wrong value back for kaslr_enabled in kernel stage.
1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled
get set wrongly in setup.c, late in module.c::get_module_load_offset
will return not wanted random module load offset.
That change behavior when HIBERNATION is defined or nokaslr is passed.
2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled
get cleared wrongly in setup.c, late in module.c::get_module_load_offset
will not return wanted random module load offset.
The setup_data linked list and thus the element which contains
kaslr_enabled is chained together using physical addresses. At the
time when we access it in the kernel proper, we're already running
with paging enabled and therefore must access it through its virtual
address.
This patch changes the code to use early_memmap and access the value,
and will keep boot and kernel consistent with kaslr.
-v3: add checking return from early_memmap according to Boris.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
@ 2015-03-06 19:53 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-06 19:53 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Kees Cook
On Fri, Mar 6, 2015 at 11:50 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org> wrote:
>
>>
>> "However, the setup_data linked list and thus the element which contains
>> kaslr_enabled is chained together using physical addresses. At the
>> time when we access it in the kernel proper, we're already running
>> with paging enabled and therefore must access it through its virtual
>> address."
>>
>> That's it, now how hard was to explain it that way?
>
> No, I don't think your change log is right.
>
> Actually the old code is using address as value.
>
> if the old code would be like:
>
> kaslr_enabled = (bool)(*(unsigned char *)(pa_data + sizeof(struct setup_data)));
>
> then your change log would be good, but the old code is
>
> kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
Please check if you are ok with this:
Subject: [PATCH v4] x86, kaslr: Access the correct kaslr_enabled variable
commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
started passing KASLR status to kernel proper, but it uses address as
the vaule.
That will get wrong value back for kaslr_enabled in kernel stage.
1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled
get set wrongly in setup.c, late in module.c::get_module_load_offset
will return not wanted random module load offset.
That change behavior when HIBERNATION is defined or nokaslr is passed.
2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled
get cleared wrongly in setup.c, late in module.c::get_module_load_offset
will not return wanted random module load offset.
The setup_data linked list and thus the element which contains
kaslr_enabled is chained together using physical addresses. At the
time when we access it in the kernel proper, we're already running
with paging enabled and therefore must access it through its virtual
address.
This patch changes the code to use early_memmap and access the value,
and will keep boot and kernel consistent with kaslr.
-v3: add checking return from early_memmap according to Boris.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-06 19:56 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2015-03-06 19:56 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Josh Triplett, Andrew Morton, Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 11:28 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Mar 6, 2015 at 10:55 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Mar 6, 2015 at 10:44 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>
>>
>> I don't see how bss and brk are related to these sizes. Can you
>> explain how bss, brk, and initrd factor into these sizes? Those were
>> what run_size was created to represent. I don't want to accidentally
>> start stomping on bss and brk again. :)
>
> VO (vlinux) init size aka VO_INIT_SIZE already inlude that.
>
> Please check update version.
>
>
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
>
> run_size is VO (vmlinux) init size include bss and brk.
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
>
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE (VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
Okay, I've proven this to myself now. :) I think it would be valuable
to call out that brk and bss are included in the _end calculation. For
others:
$ objdump -h vmlinux | egrep '\.(bss|brk)'
25 .bss 00da7000 ffffffff82436000 0000000002436000 01836000 2**12
26 .brk 00026000 ffffffff831dd000 00000000031dd000 01836000 2**0
$ nm vmlinux | egrep ' _(text|end)'
ffffffff83203000 B _end
ffffffff81000000 T _text
$ objdump -h vmlinux | bash arch/x86/tools/calc_run_size.sh
39858176
$ bc
obase=16
39858176
2603000
ibase=16
81000000 + 2603000
83603000
So, _end - _text does equal _text + bss offset + bss size + brk size
Thanks! It'll be nice to lose the run_size hack. Adding some
documentation to the code here would help others in the future trying
to find this value, I think. :)
-Kees
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> init_size first should come from VO (vmlinux) init size. That VO init size
> is from VO _end to VO _end and include VO bss and brk area.
>
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make sure [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the buffer is bigger
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-06 19:56 ` Kees Cook
0 siblings, 0 replies; 55+ messages in thread
From: Kees Cook @ 2015-03-06 19:56 UTC (permalink / raw)
To: Yinghai Lu
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Josh Triplett, Andrew Morton,
Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 11:28 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Mar 6, 2015 at 10:55 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> On Fri, Mar 6, 2015 at 10:44 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>>
>> I don't see how bss and brk are related to these sizes. Can you
>> explain how bss, brk, and initrd factor into these sizes? Those were
>> what run_size was created to represent. I don't want to accidentally
>> start stomping on bss and brk again. :)
>
> VO (vlinux) init size aka VO_INIT_SIZE already inlude that.
>
> Please check update version.
>
>
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
>
> run_size is VO (vmlinux) init size include bss and brk.
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
>
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE (VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
Okay, I've proven this to myself now. :) I think it would be valuable
to call out that brk and bss are included in the _end calculation. For
others:
$ objdump -h vmlinux | egrep '\.(bss|brk)'
25 .bss 00da7000 ffffffff82436000 0000000002436000 01836000 2**12
26 .brk 00026000 ffffffff831dd000 00000000031dd000 01836000 2**0
$ nm vmlinux | egrep ' _(text|end)'
ffffffff83203000 B _end
ffffffff81000000 T _text
$ objdump -h vmlinux | bash arch/x86/tools/calc_run_size.sh
39858176
$ bc
obase=16
39858176
2603000
ibase=16
81000000 + 2603000
83603000
So, _end - _text does equal _text + bss offset + bss size + brk size
Thanks! It'll be nice to lose the run_size hack. Adding some
documentation to the code here would help others in the future trying
to find this value, I think. :)
-Kees
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> init_size first should come from VO (vmlinux) init size. That VO init size
> is from VO _end to VO _end and include VO bss and brk area.
>
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make sure [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the buffer is bigger
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-07 0:52 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-07 0:52 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List, linux-efi, linux-pci,
Josh Triplett, Andrew Morton, Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 11:56 AM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Mar 6, 2015 at 11:28 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> Okay, I've proven this to myself now. :) I think it would be valuable
> to call out that brk and bss are included in the _end calculation. For
> others:
...
> So, _end - _text does equal _text + bss offset + bss size + brk size
>
> Thanks! It'll be nice to lose the run_size hack. Adding some
> documentation to the code here would help others in the future trying
> to find this value, I think. :)
in arch/x86/kernel/vmlinux.lds.S, we have
/* BSS */
. = ALIGN(PAGE_SIZE);
.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
__bss_start = .;
*(.bss..page_aligned)
*(.bss)
. = ALIGN(PAGE_SIZE);
__bss_stop = .;
}
. = ALIGN(PAGE_SIZE);
.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
__brk_base = .;
. += 64 * 1024; /* 64k alignment slop space */
*(.brk_reservation) /* areas brk users have reserved */
__brk_limit = .;
}
_end = .;
so _end already cover bss and brk.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size
@ 2015-03-07 0:52 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-07 0:52 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Borislav Petkov,
Baoquan He, Linux Kernel Mailing List,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Josh Triplett, Andrew Morton,
Ard Biesheuvel, Junjie Mao
On Fri, Mar 6, 2015 at 11:56 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Fri, Mar 6, 2015 at 11:28 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Okay, I've proven this to myself now. :) I think it would be valuable
> to call out that brk and bss are included in the _end calculation. For
> others:
...
> So, _end - _text does equal _text + bss offset + bss size + brk size
>
> Thanks! It'll be nice to lose the run_size hack. Adding some
> documentation to the code here would help others in the future trying
> to find this value, I think. :)
in arch/x86/kernel/vmlinux.lds.S, we have
/* BSS */
. = ALIGN(PAGE_SIZE);
.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
__bss_start = .;
*(.bss..page_aligned)
*(.bss)
. = ALIGN(PAGE_SIZE);
__bss_stop = .;
}
. = ALIGN(PAGE_SIZE);
.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
__brk_base = .;
. += 64 * 1024; /* 64k alignment slop space */
*(.brk_reservation) /* areas brk users have reserved */
__brk_limit = .;
}
_end = .;
so _end already cover bss and brk.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-06 17:49 ` Yinghai Lu
(?)
@ 2015-03-07 20:50 ` Borislav Petkov
-1 siblings, 0 replies; 55+ messages in thread
From: Borislav Petkov @ 2015-03-07 20:50 UTC (permalink / raw)
To: Yinghai Lu
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 06, 2015 at 09:49:25AM -0800, Yinghai Lu wrote:
> That is "copy and paste" instead of attachment for easy review.
> but gmail web client convert tab to spaces.
Next time you send a patch *only* for review *and* *not* for
application, do state that at the top like everyone else. Better yet,
don't use gmail for sending patches at all.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-06 19:50 ` Yinghai Lu
(?)
(?)
@ 2015-03-07 20:56 ` Borislav Petkov
-1 siblings, 0 replies; 55+ messages in thread
From: Borislav Petkov @ 2015-03-07 20:56 UTC (permalink / raw)
To: Yinghai Lu
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 06, 2015 at 11:50:54AM -0800, Yinghai Lu wrote:
> On Fri, Mar 6, 2015 at 5:33 AM, Borislav Petkov <bp@suse.de> wrote:
>
> >
> > "However, the setup_data linked list and thus the element which contains
> > kaslr_enabled is chained together using physical addresses. At the
> > time when we access it in the kernel proper, we're already running
> > with paging enabled and therefore must access it through its virtual
> > address."
> >
> > That's it, now how hard was to explain it that way?
>
> No, I don't think your change log is right.
>
> Actually the old code is using address as value.
Am I saying something about using a physical address as value above? Or
you can't read now either?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-06 19:53 ` Yinghai Lu
(?)
@ 2015-03-07 21:05 ` Borislav Petkov
2015-03-07 21:11 ` Yinghai Lu
-1 siblings, 1 reply; 55+ messages in thread
From: Borislav Petkov @ 2015-03-07 21:05 UTC (permalink / raw)
To: Yinghai Lu
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Fri, Mar 06, 2015 at 11:53:22AM -0800, Yinghai Lu wrote:
> That will get wrong value back for kaslr_enabled in kernel stage.
> 1. When kaslr is not enabled at boot/choose_kernel_location, if kaslr_enabled
> get set wrongly in setup.c, late in module.c::get_module_load_offset
> will return not wanted random module load offset.
> That change behavior when HIBERNATION is defined or nokaslr is passed.
>
> 2. When kaslr is enabled at boot/choose_kernel_location, if kaslr_enabled
> get cleared wrongly in setup.c, late in module.c::get_module_load_offset
> will not return wanted random module load offset.
Now you went from the one extreme to the other. Initially it was
"trivial and obvious" now it is too much unreadable detail which no one
needs.
How about this:
---
Commit
f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
started passing KASLR status to kernel proper, but it uses a physical
address as the vaule, leading to parsing bogus KASLR status in kernel
proper.
The setup_data linked list and thus the element which contains
kaslr_enabled is chained together using physical addresses. At the time
when we access it in the kernel proper, we're already running with
paging enabled and therefore must access it through its virtual address.
This patch changes the code to use early_memmap() and access the value.
---
Complaints?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly
2015-03-07 21:05 ` Borislav Petkov
@ 2015-03-07 21:11 ` Yinghai Lu
0 siblings, 0 replies; 55+ messages in thread
From: Yinghai Lu @ 2015-03-07 21:11 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, Matt Fleming, H. Peter Anvin, Bjorn Helgaas,
Thomas Gleixner, Ingo Molnar, Jiri Kosina, Baoquan He,
Linux Kernel Mailing List, linux-efi, linux-pci, Kees Cook
On Sat, Mar 7, 2015 at 1:05 PM, Borislav Petkov <bp@suse.de> wrote:
> On Fri, Mar 06, 2015 at 11:53:22AM -0800, Yinghai Lu wrote:
> ---
> Commit
>
> f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
>
> started passing KASLR status to kernel proper, but it uses a physical
> address as the vaule, leading to parsing bogus KASLR status in kernel
> proper.
>
> The setup_data linked list and thus the element which contains
> kaslr_enabled is chained together using physical addresses. At the time
> when we access it in the kernel proper, we're already running with
> paging enabled and therefore must access it through its virtual address.
>
> This patch changes the code to use early_memmap() and access the value.
> ---
Good to me.
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2015-03-07 21:11 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-04 8:00 [PATCH v2 00/15] x86, boot: clean up kasl and setup_data handling Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 01/15] x86, kaslr: Use init_size instead of run_size Yinghai Lu
2015-03-06 13:55 ` Borislav Petkov
2015-03-06 18:44 ` Yinghai Lu
2015-03-06 18:55 ` Kees Cook
2015-03-06 18:55 ` Kees Cook
2015-03-06 19:28 ` Yinghai Lu
2015-03-06 19:56 ` Kees Cook
2015-03-06 19:56 ` Kees Cook
2015-03-07 0:52 ` Yinghai Lu
2015-03-07 0:52 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 02/15] x86, boot: move ZO to end of buffer Yinghai Lu
2015-03-06 13:58 ` Borislav Petkov
2015-03-04 8:00 ` [PATCH v2 03/15] x86, boot: keep data from ZO boot stage to VO kernel stage Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 04/15] x86, kaslr: get kaslr_enabled back correctly Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
2015-03-04 10:16 ` Borislav Petkov
2015-03-04 15:54 ` Jiri Kosina
2015-03-04 18:12 ` Yinghai Lu
2015-03-04 18:12 ` Yinghai Lu
2015-03-04 19:41 ` Ingo Molnar
2015-03-04 19:41 ` Ingo Molnar
2015-03-05 2:58 ` joeyli
2015-03-05 3:20 ` Yinghai Lu
2015-03-04 18:06 ` Yinghai Lu
2015-03-04 18:56 ` Yinghai Lu
2015-03-04 20:00 ` Ingo Molnar
2015-03-04 20:00 ` Ingo Molnar
2015-03-04 21:32 ` Yinghai Lu
2015-03-06 13:33 ` Borislav Petkov
2015-03-06 17:49 ` Yinghai Lu
2015-03-06 17:49 ` Yinghai Lu
2015-03-07 20:50 ` Borislav Petkov
2015-03-06 19:50 ` Yinghai Lu
2015-03-06 19:50 ` Yinghai Lu
2015-03-06 19:53 ` Yinghai Lu
2015-03-06 19:53 ` Yinghai Lu
2015-03-07 21:05 ` Borislav Petkov
2015-03-07 21:11 ` Yinghai Lu
2015-03-07 20:56 ` Borislav Petkov
2015-03-04 8:00 ` [PATCH v2 05/15] x86, kaslr: consolidate the mem_avoid filling Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 06/15] x86, boot: split kernel_ident_mapping_init into another file Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 07/15] x86, kaslr, 64bit: set new or extra ident_mapping Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 08/15] x86: Kill E820_RESERVED_KERN Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 09/15] x86, efi: copy SETUP_EFI data and access directly Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 10/15] x86, of: let add_dtb reserve by itself Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 11/15] x86, boot: Add add_pci handler for SETUP_PCI Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 12/15] x86: kill not used setup_data handling code Yinghai Lu
2015-03-04 8:00 ` Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 13/15] x86, pci: convert SETUP_PCI data to list Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 14/15] x86, boot: copy rom to kernel space Yinghai Lu
2015-03-04 8:00 ` [PATCH v2 15/15] x86, pci: export SETUP_PCI data via sysfs Yinghai Lu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.