2020-03-12 13:53:18

by Peter Zijlstra

[permalink] [raw]
Subject: [RFC][PATCH 04/16] objtool: Annotate identity_mapped()

Normally identity_mapped is not visible to objtool, due to:

arch/x86/kernel/Makefile:OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y

However, when we want to run objtool on vmlinux.o there is no hiding
it. Without the annotation we'll get complaints about the:

call 1f
1: popq %rB

construct from the add_call_destinations() pass. Because
identity_mapped() is a SYM_CODE_START_LOCAL_NOALIGN() it is STT_NOTYPE
and we need sym_for_each_insn() to iterate the actual instructions.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/kernel/relocate_kernel_64.S | 2 ++
include/linux/frame.h | 16 ++++++++++++++++
tools/objtool/check.c | 4 ++--
3 files changed, 20 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -5,6 +5,7 @@
*/

#include <linux/linkage.h>
+#include <linux/frame.h>
#include <asm/page_types.h>
#include <asm/kexec.h>
#include <asm/processor-flags.h>
@@ -210,6 +211,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_ma
pushq %rax
ret
SYM_CODE_END(identity_mapped)
+STACK_FRAME_NON_STANDARD(identity_mapped)

SYM_CODE_START_LOCAL_NOALIGN(virtual_mapped)
movq RSP(%r8), %rsp
--- a/include/linux/frame.h
+++ b/include/linux/frame.h
@@ -3,6 +3,9 @@
#define _LINUX_FRAME_H

#ifdef CONFIG_STACK_VALIDATION
+
+#ifndef __ASSEMBLY__
+
/*
* This macro marks the given function's stack frame as "non-standard", which
* tells objtool to ignore the function when doing stack metadata validation.
@@ -15,6 +18,19 @@
static void __used __section(.discard.func_stack_frame_non_standard) \
*__func_stack_frame_non_standard_##func = func

+#else /* __ASSEMBLY__ */
+
+#define STACK_FRAME_NON_STANDARD(func) \
+ .pushsection .discard.func_stack_frame_non_standard, "aw"; \
+ .align 8; \
+ .type __func_stack_frame_non_standard_##func, @object; \
+ .size __func_stack_frame_non_standard_##func, 8; \
+__func_stack_frame_non_standard_##func: \
+ .quad func; \
+ .popsection
+
+#endif /* __ASSEMBLY */
+
#else /* !CONFIG_STACK_VALIDATION */

#define STACK_FRAME_NON_STANDARD(func)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -416,7 +416,7 @@ static void add_ignores(struct objtool_f

case STT_SECTION:
func = find_symbol_by_offset(rela->sym->sec, rela->addend);
- if (!func || func->type != STT_FUNC)
+ if (!func || (func->type != STT_FUNC && func->type != STT_NOTYPE))
continue;
break;

@@ -425,7 +425,7 @@ static void add_ignores(struct objtool_f
continue;
}

- func_for_each_insn(file, func, insn)
+ sym_for_each_insn(file, func, insn)
insn->ignore = true;
}
}



2020-03-13 14:37:05

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH 04/16] objtool: Annotate identity_mapped()

On Thu, Mar 12, 2020 at 02:41:11PM +0100, Peter Zijlstra wrote:

> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -416,7 +416,7 @@ static void add_ignores(struct objtool_f
>
> case STT_SECTION:
> func = find_symbol_by_offset(rela->sym->sec, rela->addend);
> - if (!func || func->type != STT_FUNC)
> + if (!func || (func->type != STT_FUNC && func->type != STT_NOTYPE))
> continue;
> break;
>
> @@ -425,7 +425,7 @@ static void add_ignores(struct objtool_f
> continue;
> }
>
> - func_for_each_insn(file, func, insn)
> + sym_for_each_insn(file, func, insn)
> insn->ignore = true;
> }
> }


This conflicts with:

7acfe5315312 ("objtool: Improve call destination function detection")

which wasn't in the tree we were working against :/

I've resolved it something like so.

---
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -406,7 +406,7 @@ static void add_ignores(struct objtool_f
{
struct instruction *insn;
struct section *sec;
- struct symbol *func;
+ struct symbol *sym;
struct rela *rela;

sec = find_section_by_name(file->elf, ".rela.discard.func_stack_frame_non_standard");
@@ -416,12 +416,12 @@ static void add_ignores(struct objtool_f
list_for_each_entry(rela, &sec->rela_list, list) {
switch (rela->sym->type) {
case STT_FUNC:
- func = rela->sym;
+ sym = rela->sym;
break;

case STT_SECTION:
- func = find_func_by_offset(rela->sym->sec, rela->addend);
- if (!func)
+ sym = find_symbol_by_offset(rela->sym->sec, rela->addend);
+ if (!sym || (sym->type != STT_FUNC || sym->type != STT_NOTYPE))
continue;
break;

@@ -430,7 +430,7 @@ static void add_ignores(struct objtool_f
continue;
}

- sym_for_each_insn(file, func, insn)
+ sym_for_each_insn(file, sym, insn)
insn->ignore = true;
}
}

2020-03-13 16:47:20

by Brian Gerst

[permalink] [raw]
Subject: Re: [RFC][PATCH 04/16] objtool: Annotate identity_mapped()

On Thu, Mar 12, 2020 at 9:53 AM Peter Zijlstra <[email protected]> wrote:
>
> Normally identity_mapped is not visible to objtool, due to:
>
> arch/x86/kernel/Makefile:OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
>
> However, when we want to run objtool on vmlinux.o there is no hiding
> it. Without the annotation we'll get complaints about the:
>
call 1f
1: popq %r8
subq $(1b - relocate_kernel), %r8

It looks to me that this code is simply trying to get the virtual
address of relocate_kernel using the old 32-bit method of PIC address
calculation. On 64-bit can be done with leaq relocate_kernel(%rip),
%r8.

--
Brian Gerst

2020-03-13 17:24:03

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH 04/16] objtool: Annotate identity_mapped()

On Fri, Mar 13, 2020 at 12:46:05PM -0400, Brian Gerst wrote:
> On Thu, Mar 12, 2020 at 9:53 AM Peter Zijlstra <[email protected]> wrote:
> >
> > Normally identity_mapped is not visible to objtool, due to:
> >
> > arch/x86/kernel/Makefile:OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
> >
> > However, when we want to run objtool on vmlinux.o there is no hiding
> > it. Without the annotation we'll get complaints about the:
> >
> call 1f
> 1: popq %r8
> subq $(1b - relocate_kernel), %r8
>
> It looks to me that this code is simply trying to get the virtual
> address of relocate_kernel using the old 32-bit method of PIC address
> calculation. On 64-bit can be done with leaq relocate_kernel(%rip),
> %r8.

Indeed. Objtool would be happy with that. And it seems I can still kexec
a kernel too.

Thanks!

2020-03-15 15:49:16

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [RFC][PATCH 04/16] objtool: Annotate identity_mapped()

On Fri, Mar 13, 2020 at 03:34:29PM +0100, Peter Zijlstra wrote:
> This conflicts with:
>
> 7acfe5315312 ("objtool: Improve call destination function detection")
>
> which wasn't in the tree we were working against :/
>
> I've resolved it something like so.
>
> ---
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -406,7 +406,7 @@ static void add_ignores(struct objtool_f
> {
> struct instruction *insn;
> struct section *sec;
> - struct symbol *func;
> + struct symbol *sym;
> struct rela *rela;
>
> sec = find_section_by_name(file->elf, ".rela.discard.func_stack_frame_non_standard");
> @@ -416,12 +416,12 @@ static void add_ignores(struct objtool_f
> list_for_each_entry(rela, &sec->rela_list, list) {
> switch (rela->sym->type) {
> case STT_FUNC:
> - func = rela->sym;
> + sym = rela->sym;
> break;
>
> case STT_SECTION:
> - func = find_func_by_offset(rela->sym->sec, rela->addend);
> - if (!func)
> + sym = find_symbol_by_offset(rela->sym->sec, rela->addend);
> + if (!sym || (sym->type != STT_FUNC || sym->type != STT_NOTYPE))
^^
&&

--
Josh