Find all ENDBR instructions which are never referenced and stick them
in a section such that the kernel can poison them, sealing the
functions from ever being an indirect call target.
This removes about 1-in-4 ENDBR instructions.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 9 ++++
tools/objtool/check.c | 69 +++++++++++++++++++++++++++++++-
tools/objtool/include/objtool/objtool.h | 1
tools/objtool/objtool.c | 1
4 files changed, 78 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -285,6 +285,15 @@ SECTIONS
}
#endif
+#ifdef CONFIG_X86_KERNEL_IBT
+ . = ALIGN(8);
+ .ibt_endbr_seal : AT(ADDR(.ibt_endbr_seal) - LOAD_OFFSET) {
+ __ibt_endbr_seal = .;
+ *(.ibt_endbr_seal)
+ __ibt_endbr_seal_end = .;
+ }
+#endif
+
/*
* struct alt_inst entries. From the header (alternative.h):
* "Alternative instructions for different CPU types or capabilities"
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -380,6 +380,7 @@ static int decode_instructions(struct ob
memset(insn, 0, sizeof(*insn));
INIT_LIST_HEAD(&insn->alts);
INIT_LIST_HEAD(&insn->stack_ops);
+ INIT_LIST_HEAD(&insn->call_node);
insn->sec = sec;
insn->offset = offset;
@@ -409,8 +410,9 @@ static int decode_instructions(struct ob
sym_for_each_insn(file, func, insn) {
insn->func = func;
- if (insn->type == INSN_ENDBR) {
+ if (insn->type == INSN_ENDBR && list_empty(&insn->call_node)) {
if (insn->offset == insn->func->offset) {
+ list_add_tail(&insn->call_node, &file->endbr_list);
file->nr_endbr++;
} else {
file->nr_endbr_int++;
@@ -739,6 +741,58 @@ static int create_retpoline_sites_sectio
return 0;
}
+static int create_ibt_endbr_seal_sections(struct objtool_file *file)
+{
+ struct instruction *insn;
+ struct section *sec;
+ int idx;
+
+ sec = find_section_by_name(file->elf, ".ibt_endbr_seal");
+ if (sec) {
+ WARN("file already has .ibt_endbr_seal, skipping");
+ return 0;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node)
+ idx++;
+
+ if (stats) {
+ printf("ibt: ENDBR at function start: %d\n", file->nr_endbr);
+ printf("ibt: ENDBR inside functions: %d\n", file->nr_endbr_int);
+ printf("ibt: superfluous ENDBR: %d\n", idx);
+ }
+
+ if (!idx)
+ return 0;
+
+ sec = elf_create_section(file->elf, ".ibt_endbr_seal", 0,
+ sizeof(int), idx);
+ if (!sec) {
+ WARN("elf_create_section: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node) {
+
+ int *site = (int *)sec->data->d_buf + idx;
+ *site = 0;
+
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(int),
+ R_X86_64_PC32,
+ insn->sec, insn->offset)) {
+ WARN("elf_add_reloc_to_insn: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx++;
+ }
+
+ return 0;
+}
+
static int create_mcount_loc_sections(struct objtool_file *file)
{
struct section *sec;
@@ -3097,8 +3151,12 @@ validate_ibt_reloc(struct objtool_file *
if (!dest)
return NULL;
- if (dest->type == INSN_ENDBR)
+ if (dest->type == INSN_ENDBR) {
+ if (!list_empty(&dest->call_node))
+ list_del_init(&dest->call_node);
+
return NULL;
+ }
if (reloc->sym->static_call_tramp)
return NULL;
@@ -3805,6 +3863,13 @@ int check(struct objtool_file *file)
if (ret < 0)
goto out;
warnings += ret;
+ }
+
+ if (ibt) {
+ ret = create_ibt_endbr_seal_sections(file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
}
if (stats) {
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -26,6 +26,7 @@ struct objtool_file {
struct list_head retpoline_call_list;
struct list_head static_call_list;
struct list_head mcount_loc_list;
+ struct list_head endbr_list;
bool ignore_unreachables, c_file, hints, rodata;
unsigned int nr_endbr;
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -128,6 +128,7 @@ struct objtool_file *objtool_open_read(c
INIT_LIST_HEAD(&file.retpoline_call_list);
INIT_LIST_HEAD(&file.static_call_list);
INIT_LIST_HEAD(&file.mcount_loc_list);
+ INIT_LIST_HEAD(&file.endbr_list);
file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
The following commit has been merged into the x86/core branch of tip:
Commit-ID: 49f8cb48085ddcacc942d9270c305b31887f0aca
Gitweb: https://git.kernel.org/tip/49f8cb48085ddcacc942d9270c305b31887f0aca
Author: Peter Zijlstra <[email protected]>
AuthorDate: Tue, 08 Mar 2022 16:30:55 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 08 Mar 2022 23:53:39 +01:00
objtool: Find unused ENDBR instructions
Find all ENDBR instructions which are never referenced and stick them
in a section such that the kernel can poison them, sealing the
functions from ever being an indirect call target.
This removes about 1-in-4 ENDBR instructions.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/vmlinux.lds.S | 9 +++-
tools/objtool/check.c | 69 +++++++++++++++++++++++-
tools/objtool/include/objtool/objtool.h | 1 +-
tools/objtool/objtool.c | 1 +-
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 27f8303..7fda7f2 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -285,6 +285,15 @@ SECTIONS
}
#endif
+#ifdef CONFIG_X86_KERNEL_IBT
+ . = ALIGN(8);
+ .ibt_endbr_seal : AT(ADDR(.ibt_endbr_seal) - LOAD_OFFSET) {
+ __ibt_endbr_seal = .;
+ *(.ibt_endbr_seal)
+ __ibt_endbr_seal_end = .;
+ }
+#endif
+
/*
* struct alt_inst entries. From the header (alternative.h):
* "Alternative instructions for different CPU types or capabilities"
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 28163ea..d921a47 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -382,6 +382,7 @@ static int decode_instructions(struct objtool_file *file)
memset(insn, 0, sizeof(*insn));
INIT_LIST_HEAD(&insn->alts);
INIT_LIST_HEAD(&insn->stack_ops);
+ INIT_LIST_HEAD(&insn->call_node);
insn->sec = sec;
insn->offset = offset;
@@ -419,8 +420,9 @@ static int decode_instructions(struct objtool_file *file)
sym_for_each_insn(file, func, insn) {
insn->func = func;
- if (insn->type == INSN_ENDBR) {
+ if (insn->type == INSN_ENDBR && list_empty(&insn->call_node)) {
if (insn->offset == insn->func->offset) {
+ list_add_tail(&insn->call_node, &file->endbr_list);
file->nr_endbr++;
} else {
file->nr_endbr_int++;
@@ -741,6 +743,58 @@ static int create_retpoline_sites_sections(struct objtool_file *file)
return 0;
}
+static int create_ibt_endbr_seal_sections(struct objtool_file *file)
+{
+ struct instruction *insn;
+ struct section *sec;
+ int idx;
+
+ sec = find_section_by_name(file->elf, ".ibt_endbr_seal");
+ if (sec) {
+ WARN("file already has .ibt_endbr_seal, skipping");
+ return 0;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node)
+ idx++;
+
+ if (stats) {
+ printf("ibt: ENDBR at function start: %d\n", file->nr_endbr);
+ printf("ibt: ENDBR inside functions: %d\n", file->nr_endbr_int);
+ printf("ibt: superfluous ENDBR: %d\n", idx);
+ }
+
+ if (!idx)
+ return 0;
+
+ sec = elf_create_section(file->elf, ".ibt_endbr_seal", 0,
+ sizeof(int), idx);
+ if (!sec) {
+ WARN("elf_create_section: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node) {
+
+ int *site = (int *)sec->data->d_buf + idx;
+ *site = 0;
+
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(int),
+ R_X86_64_PC32,
+ insn->sec, insn->offset)) {
+ WARN("elf_add_reloc_to_insn: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx++;
+ }
+
+ return 0;
+}
+
static int create_mcount_loc_sections(struct objtool_file *file)
{
struct section *sec;
@@ -3119,8 +3173,12 @@ validate_ibt_reloc(struct objtool_file *file, struct reloc *reloc)
if (!dest)
return NULL;
- if (dest->type == INSN_ENDBR)
+ if (dest->type == INSN_ENDBR) {
+ if (!list_empty(&dest->call_node))
+ list_del_init(&dest->call_node);
+
return NULL;
+ }
if (reloc->sym->static_call_tramp)
return NULL;
@@ -3859,6 +3917,13 @@ int check(struct objtool_file *file)
warnings += ret;
}
+ if (ibt) {
+ ret = create_ibt_endbr_seal_sections(file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
+ }
+
if (stats) {
printf("nr_insns_visited: %ld\n", nr_insns_visited);
printf("nr_cfi: %ld\n", nr_cfi);
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index fa3c7fa..7a5c13a 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -26,6 +26,7 @@ struct objtool_file {
struct list_head retpoline_call_list;
struct list_head static_call_list;
struct list_head mcount_loc_list;
+ struct list_head endbr_list;
bool ignore_unreachables, c_file, hints, rodata;
unsigned int nr_endbr;
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index bdf699f..b09946f 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -128,6 +128,7 @@ struct objtool_file *objtool_open_read(const char *_objname)
INIT_LIST_HEAD(&file.retpoline_call_list);
INIT_LIST_HEAD(&file.static_call_list);
INIT_LIST_HEAD(&file.mcount_loc_list);
+ INIT_LIST_HEAD(&file.endbr_list);
file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
From: Peter Zijlstra
>
> objtool: Find unused ENDBR instructions
>
> Find all ENDBR instructions which are never referenced and stick them
> in a section such that the kernel can poison them, sealing the
> functions from ever being an indirect call target.
Thought, what happens if the only indirect call is from
code in a module?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
The following commit has been merged into the x86/core branch of tip:
Commit-ID: 89bc853eae4ad125030ef99f207ba76c2f00a26e
Gitweb: https://git.kernel.org/tip/89bc853eae4ad125030ef99f207ba76c2f00a26e
Author: Peter Zijlstra <[email protected]>
AuthorDate: Tue, 08 Mar 2022 16:30:55 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 15 Mar 2022 10:32:47 +01:00
objtool: Find unused ENDBR instructions
Find all ENDBR instructions which are never referenced and stick them
in a section such that the kernel can poison them, sealing the
functions from ever being an indirect call target.
This removes about 1-in-4 ENDBR instructions.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/vmlinux.lds.S | 9 +++-
tools/objtool/check.c | 69 +++++++++++++++++++++++-
tools/objtool/include/objtool/objtool.h | 1 +-
tools/objtool/objtool.c | 1 +-
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 27f8303..7fda7f2 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -285,6 +285,15 @@ SECTIONS
}
#endif
+#ifdef CONFIG_X86_KERNEL_IBT
+ . = ALIGN(8);
+ .ibt_endbr_seal : AT(ADDR(.ibt_endbr_seal) - LOAD_OFFSET) {
+ __ibt_endbr_seal = .;
+ *(.ibt_endbr_seal)
+ __ibt_endbr_seal_end = .;
+ }
+#endif
+
/*
* struct alt_inst entries. From the header (alternative.h):
* "Alternative instructions for different CPU types or capabilities"
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d4cf831..6de5085 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -383,6 +383,7 @@ static int decode_instructions(struct objtool_file *file)
memset(insn, 0, sizeof(*insn));
INIT_LIST_HEAD(&insn->alts);
INIT_LIST_HEAD(&insn->stack_ops);
+ INIT_LIST_HEAD(&insn->call_node);
insn->sec = sec;
insn->offset = offset;
@@ -420,8 +421,9 @@ static int decode_instructions(struct objtool_file *file)
sym_for_each_insn(file, func, insn) {
insn->func = func;
- if (insn->type == INSN_ENDBR) {
+ if (insn->type == INSN_ENDBR && list_empty(&insn->call_node)) {
if (insn->offset == insn->func->offset) {
+ list_add_tail(&insn->call_node, &file->endbr_list);
file->nr_endbr++;
} else {
file->nr_endbr_int++;
@@ -742,6 +744,58 @@ static int create_retpoline_sites_sections(struct objtool_file *file)
return 0;
}
+static int create_ibt_endbr_seal_sections(struct objtool_file *file)
+{
+ struct instruction *insn;
+ struct section *sec;
+ int idx;
+
+ sec = find_section_by_name(file->elf, ".ibt_endbr_seal");
+ if (sec) {
+ WARN("file already has .ibt_endbr_seal, skipping");
+ return 0;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node)
+ idx++;
+
+ if (stats) {
+ printf("ibt: ENDBR at function start: %d\n", file->nr_endbr);
+ printf("ibt: ENDBR inside functions: %d\n", file->nr_endbr_int);
+ printf("ibt: superfluous ENDBR: %d\n", idx);
+ }
+
+ if (!idx)
+ return 0;
+
+ sec = elf_create_section(file->elf, ".ibt_endbr_seal", 0,
+ sizeof(int), idx);
+ if (!sec) {
+ WARN("elf_create_section: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx = 0;
+ list_for_each_entry(insn, &file->endbr_list, call_node) {
+
+ int *site = (int *)sec->data->d_buf + idx;
+ *site = 0;
+
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(int),
+ R_X86_64_PC32,
+ insn->sec, insn->offset)) {
+ WARN("elf_add_reloc_to_insn: .ibt_endbr_seal");
+ return -1;
+ }
+
+ idx++;
+ }
+
+ return 0;
+}
+
static int create_mcount_loc_sections(struct objtool_file *file)
{
struct section *sec;
@@ -3120,8 +3174,12 @@ validate_ibt_reloc(struct objtool_file *file, struct reloc *reloc)
if (!dest)
return NULL;
- if (dest->type == INSN_ENDBR)
+ if (dest->type == INSN_ENDBR) {
+ if (!list_empty(&dest->call_node))
+ list_del_init(&dest->call_node);
+
return NULL;
+ }
if (reloc->sym->static_call_tramp)
return NULL;
@@ -3860,6 +3918,13 @@ int check(struct objtool_file *file)
warnings += ret;
}
+ if (ibt) {
+ ret = create_ibt_endbr_seal_sections(file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
+ }
+
if (stats) {
printf("nr_insns_visited: %ld\n", nr_insns_visited);
printf("nr_cfi: %ld\n", nr_cfi);
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index fa3c7fa..7a5c13a 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -26,6 +26,7 @@ struct objtool_file {
struct list_head retpoline_call_list;
struct list_head static_call_list;
struct list_head mcount_loc_list;
+ struct list_head endbr_list;
bool ignore_unreachables, c_file, hints, rodata;
unsigned int nr_endbr;
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index bdf699f..b09946f 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -128,6 +128,7 @@ struct objtool_file *objtool_open_read(const char *_objname)
INIT_LIST_HEAD(&file.retpoline_call_list);
INIT_LIST_HEAD(&file.static_call_list);
INIT_LIST_HEAD(&file.mcount_loc_list);
+ INIT_LIST_HEAD(&file.endbr_list);
file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
On Tue, Mar 15, 2022 at 03:39:52PM +0000, David Laight wrote:
> From: Peter Zijlstra
> >
> > objtool: Find unused ENDBR instructions
> >
> > Find all ENDBR instructions which are never referenced and stick them
> > in a section such that the kernel can poison them, sealing the
> > functions from ever being an indirect call target.
>
> Thought, what happens if the only indirect call is from
> code in a module?
Then <boom>, I guess. Is it safe to assume in-tree modules don't need
to do indirect calls to exported functions? I guess we'll find out :-)
--
Josh
On Thu, Mar 17, 2022 at 03:22:54PM -0700, Josh Poimboeuf wrote:
> On Tue, Mar 15, 2022 at 03:39:52PM +0000, David Laight wrote:
> > From: Peter Zijlstra
> > >
> > > objtool: Find unused ENDBR instructions
> > >
> > > Find all ENDBR instructions which are never referenced and stick them
> > > in a section such that the kernel can poison them, sealing the
> > > functions from ever being an indirect call target.
> >
> > Thought, what happens if the only indirect call is from
> > code in a module?
>
> Then <boom>, I guess. Is it safe to assume in-tree modules don't need
> to do indirect calls to exported functions? I guess we'll find out :-)
So exported functions will keep their ENDBR. Specifically, their address
is taken by the EXPORT_SYMBOL thing.
Sealing them might work, but let's not do that just now ;-)
Any unexported function discovered through kallsyms OTOH... those
deservedly will go *boom*.