2014-10-31 20:58:42

by Kees Cook

[permalink] [raw]
Subject: [PATCH v2] x86, boot: add hex output for debugging

This is useful for reporting various addresses or other values while
debugging early boot. For example, when CONFIG_X86_VERBOSE_BOOTUP is set,
this is now visible at boot time:

early console in setup code
early console in decompress_kernel
input_data: 0x0000000001e1526e
input_len: 0x0000000000732236
output: 0x0000000001000000
output_len: 0x0000000001535640
run_size: 0x00000000021fb000
KASLR using RDTSC...

Signed-off-by: Kees Cook <[email protected]>
---
Since this displays run_size, this patch depends on Junjie Mao's patch
"x86, kaslr: Prevent .bss from overlaping initrd"

---
arch/x86/boot/compressed/misc.c | 24 ++++++++++++++++++++++++
arch/x86/boot/compressed/misc.h | 11 +++++++++++
2 files changed, 35 insertions(+)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 30dd59a9f0b4..2aefc8a63655 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -220,6 +220,23 @@ void __putstr(const char *s)
outb(0xff & (pos >> 1), vidport+1);
}

+void __puthex(unsigned long value)
+{
+ char alpha[2] = "0";
+ int bits;
+
+ for (bits = sizeof(value) * 8 - 4; bits >= 0; bits -= 4) {
+ unsigned long digit = (value >> bits) & 0xf;
+
+ if (digit < 0xA)
+ alpha[0] = '0' + digit;
+ else
+ alpha[0] = 'a' + (digit - 0xA);
+
+ __putstr(alpha);
+ }
+}
+
static void error(char *x)
{
error_putstr("\n\n");
@@ -382,6 +399,13 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
free_mem_ptr = heap; /* Heap */
free_mem_end_ptr = heap + BOOT_HEAP_SIZE;

+ /* Report initial kernel position details. */
+ debug_putaddr(input_data);
+ debug_putaddr(input_len);
+ debug_putaddr(output);
+ debug_putaddr(output_len);
+ debug_putaddr(run_size);
+
/*
* The memory hole needed for the kernel is the larger of either
* the entire decompressed kernel plus relocation table, or the
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 24e3e569a13c..70472ae98a73 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -34,16 +34,27 @@ extern memptr free_mem_ptr;
extern memptr free_mem_end_ptr;
extern struct boot_params *real_mode; /* Pointer to real-mode data */
void __putstr(const char *s);
+void __puthex(unsigned long value);
#define error_putstr(__x) __putstr(__x)
+#define error_puthex(__x) __puthex(__x)

#ifdef CONFIG_X86_VERBOSE_BOOTUP

#define debug_putstr(__x) __putstr(__x)
+#define debug_puthex(__x) __puthex(__x)
+#define debug_putaddr(__x) { \
+ debug_putstr(#__x ": 0x"); \
+ debug_puthex((unsigned long)(__x)); \
+ debug_putstr("\n"); \
+ }

#else

static inline void debug_putstr(const char *s)
{ }
+static inline void debug_puthex(const char *s)
+{ }
+#define debug_putaddr(x) /* */

#endif

--
1.9.1


--
Kees Cook
Chrome OS Security


2014-10-31 21:34:46

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v2] x86, boot: add hex output for debugging

On 10/31/2014 01:58 PM, Kees Cook wrote:
> This is useful for reporting various addresses or other values while
> debugging early boot. For example, when CONFIG_X86_VERBOSE_BOOTUP is set,
> this is now visible at boot time:
>
> early console in setup code
> early console in decompress_kernel
> input_data: 0x0000000001e1526e
> input_len: 0x0000000000732236
> output: 0x0000000001000000
> output_len: 0x0000000001535640
> run_size: 0x00000000021fb000
> KASLR using RDTSC...
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> Since this displays run_size, this patch depends on Junjie Mao's patch
> "x86, kaslr: Prevent .bss from overlaping initrd"
>

This reminds me: I have a patch to add an early-boot IDT that can report
#GP and #PF in this code if CONFIG_X86_VERBOSE_BOOTUP is set. I can
dust it off if anyone thinks it'll be useful, even though I won't end up
needed it for the thing I wrote it for.

--Andy

> ---
> arch/x86/boot/compressed/misc.c | 24 ++++++++++++++++++++++++
> arch/x86/boot/compressed/misc.h | 11 +++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 30dd59a9f0b4..2aefc8a63655 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -220,6 +220,23 @@ void __putstr(const char *s)
> outb(0xff & (pos >> 1), vidport+1);
> }
>
> +void __puthex(unsigned long value)
> +{
> + char alpha[2] = "0";
> + int bits;
> +
> + for (bits = sizeof(value) * 8 - 4; bits >= 0; bits -= 4) {
> + unsigned long digit = (value >> bits) & 0xf;
> +
> + if (digit < 0xA)
> + alpha[0] = '0' + digit;
> + else
> + alpha[0] = 'a' + (digit - 0xA);
> +
> + __putstr(alpha);
> + }
> +}
> +
> static void error(char *x)
> {
> error_putstr("\n\n");
> @@ -382,6 +399,13 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> free_mem_ptr = heap; /* Heap */
> free_mem_end_ptr = heap + BOOT_HEAP_SIZE;
>
> + /* Report initial kernel position details. */
> + debug_putaddr(input_data);
> + debug_putaddr(input_len);
> + debug_putaddr(output);
> + debug_putaddr(output_len);
> + debug_putaddr(run_size);
> +
> /*
> * The memory hole needed for the kernel is the larger of either
> * the entire decompressed kernel plus relocation table, or the
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index 24e3e569a13c..70472ae98a73 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -34,16 +34,27 @@ extern memptr free_mem_ptr;
> extern memptr free_mem_end_ptr;
> extern struct boot_params *real_mode; /* Pointer to real-mode data */
> void __putstr(const char *s);
> +void __puthex(unsigned long value);
> #define error_putstr(__x) __putstr(__x)
> +#define error_puthex(__x) __puthex(__x)
>
> #ifdef CONFIG_X86_VERBOSE_BOOTUP
>
> #define debug_putstr(__x) __putstr(__x)
> +#define debug_puthex(__x) __puthex(__x)
> +#define debug_putaddr(__x) { \
> + debug_putstr(#__x ": 0x"); \
> + debug_puthex((unsigned long)(__x)); \
> + debug_putstr("\n"); \
> + }
>
> #else
>
> static inline void debug_putstr(const char *s)
> { }
> +static inline void debug_puthex(const char *s)
> +{ }
> +#define debug_putaddr(x) /* */
>
> #endif
>
>

2014-10-31 23:31:35

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH v2] x86, boot: add hex output for debugging

On Fri, Oct 31, 2014 at 1:58 PM, Kees Cook <[email protected]> wrote:
> This is useful for reporting various addresses or other values while
> debugging early boot. For example, when CONFIG_X86_VERBOSE_BOOTUP is set,
> this is now visible at boot time:
>
> early console in setup code
> early console in decompress_kernel
> input_data: 0x0000000001e1526e
> input_len: 0x0000000000732236
> output: 0x0000000001000000
> output_len: 0x0000000001535640
> run_size: 0x00000000021fb000
> KASLR using RDTSC...
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> Since this displays run_size, this patch depends on Junjie Mao's patch
> "x86, kaslr: Prevent .bss from overlaping initrd"
>
> ---
> arch/x86/boot/compressed/misc.c | 24 ++++++++++++++++++++++++
> arch/x86/boot/compressed/misc.h | 11 +++++++++++
> 2 files changed, 35 insertions(+)
...

We can reuse printf.c in arch/x86/boot.

I had attached one in local tree for a while. or even sent it before
several years ago.

Thanks

Yinghai


Subject: [PATCH] x86, boot: Add printf support for early console in
compressed/misc.c

Reuse printf.c in x86 setup code.
And print out decompress_kernel input and output info.

Later decompresser code could print out more info for debug info.

Signed-off-by: Yinghai Lu <[email protected]>

---
arch/x86/boot/compressed/Makefile | 2 +-
arch/x86/boot/compressed/misc.c | 8 ++++++++
arch/x86/boot/compressed/misc.h | 7 +++++++
arch/x86/boot/compressed/printf.c | 5 +++++
4 files changed, 21 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/boot/compressed/Makefile
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/Makefile
+++ linux-2.6/arch/x86/boot/compressed/Makefile
@@ -28,7 +28,7 @@ HOST_EXTRACFLAGS += -I$(srctree)/tools/i

vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
$(obj)/string.o $(obj)/cmdline.o \
- $(obj)/piggy.o $(obj)/cpuflags.o
+ $(obj)/printf.o $(obj)/piggy.o $(obj)/cpuflags.o

vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/aslr.o
Index: linux-2.6/arch/x86/boot/compressed/printf.c
===================================================================
--- /dev/null
+++ linux-2.6/arch/x86/boot/compressed/printf.c
@@ -0,0 +1,5 @@
+#include "misc.h"
+
+#define puts(__x) __putstr(__x)
+
+#include "../printf.c"
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/misc.c
+++ linux-2.6/arch/x86/boot/compressed/misc.c
@@ -390,6 +390,14 @@ asmlinkage __visible void *decompress_ke
output = choose_kernel_location(input_data, input_len,
output, output_len);

+ debug_putstr("decompress_kernel:\n");
+ debug_printf(" input: [0x%lx-0x%lx], output: 0x%lx, heap:
[0x%lx-0x%lx]\n",
+ (unsigned long)input_data,
+ (unsigned long)input_data + input_len - 1,
+ (unsigned long)output,
+ (unsigned long)heap,
+ (unsigned long)heap + BOOT_HEAP_SIZE - 1);
+
/* Validate memory location choices. */
if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
error("Destination address inappropriately aligned");
Index: linux-2.6/arch/x86/boot/compressed/misc.h
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/misc.h
+++ linux-2.6/arch/x86/boot/compressed/misc.h
@@ -36,14 +36,21 @@ extern struct boot_params *real_mode; /
void __putstr(const char *s);
#define error_putstr(__x) __putstr(__x)

+/* printf.c */
+int sprintf(char *buf, const char *fmt, ...);
+int printf(const char *fmt, ...);
+
#ifdef CONFIG_X86_VERBOSE_BOOTUP

#define debug_putstr(__x) __putstr(__x)
+#define debug_printf printf

#else

static inline void debug_putstr(const char *s)
{ }
+static inline int debug_printf(const char *fmt, ...)
+{ }

#endif


Attachments:
early_console_more_2_2x.patch (2.86 kB)

2014-10-31 23:45:43

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v2] x86, boot: add hex output for debugging

On Fri, Oct 31, 2014 at 04:31:32PM -0700, Yinghai Lu wrote:
> On Fri, Oct 31, 2014 at 1:58 PM, Kees Cook <[email protected]> wrote:
> > This is useful for reporting various addresses or other values while
> > debugging early boot. For example, when CONFIG_X86_VERBOSE_BOOTUP is set,
> > this is now visible at boot time:
> >
> > early console in setup code
> > early console in decompress_kernel
> > input_data: 0x0000000001e1526e
> > input_len: 0x0000000000732236
> > output: 0x0000000001000000
> > output_len: 0x0000000001535640
> > run_size: 0x00000000021fb000
> > KASLR using RDTSC...
> >
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > Since this displays run_size, this patch depends on Junjie Mao's patch
> > "x86, kaslr: Prevent .bss from overlaping initrd"
> >
> > ---
> > arch/x86/boot/compressed/misc.c | 24 ++++++++++++++++++++++++
> > arch/x86/boot/compressed/misc.h | 11 +++++++++++
> > 2 files changed, 35 insertions(+)
> ...
>
> We can reuse printf.c in arch/x86/boot.
>
> I had attached one in local tree for a while. or even sent it before
> several years ago.

I don't think we need the full generality of printf in the decompression
stub. I prefer Kees' patch, though I'd still like to see __puthex made
conditional.

- Josh Triplett

2014-11-01 00:03:27

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] x86, boot: add hex output for debugging

On Fri, 2014-10-31 at 16:45 -0700, Josh Triplett wrote:
> I don't think we need the full generality of printf in the decompression
> stub. I prefer Kees' patch, though I'd still like to see __puthex made
> conditional.

Maybe use a statement expression macro instead?
Something like this could emit the right number of bits for any type

#define __puthex(val) \
({ \
typeof(val) value = val; \
char alpha[2] = {}; \
int bits; \
\
__putstr("0x"); \
for (bits = sizeof(value) * 8 - 4; bits >= 0; bits -= 4) { \
int digit = (value >> bits) & 0xf; \
\
if (digit < 10) \
alpha[0] = '0' + digit; \
else \
alpha[0] = 'a' + (digit - 10); \
\
__putstr(alpha); \
} \
})