| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220422105037.GV2731@worktop.programming.kicks-ass.net>
Date: Fri, 22 Apr 2022 12:50:37 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Miroslav Benes <mbenes@...e.cz>
Cc: Josh Poimboeuf <jpoimboe@...hat.com>, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 15/25] objtool: Rework ibt and extricate from stack
validation
On Wed, Apr 20, 2022 at 07:25:16PM +0200, Miroslav Benes wrote:
> A nit and it was there even before this patch...
>
> > -static struct instruction *
> > -validate_ibt_reloc(struct objtool_file *file, struct reloc *reloc)
> > -{
> > - struct instruction *dest;
> > - struct section *sec;
> > - unsigned long off;
> > -
> > - sec = reloc->sym->sec;
> > - off = reloc->sym->offset;
> > -
> > - if ((reloc->sec->base->sh.sh_flags & SHF_EXECINSTR) &&
> > - (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32))
> > - off += arch_dest_reloc_offset(reloc->addend);
>
> here...
>
> > +static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn)
> > +{
>
> ...
> > + off = reloc->sym->offset;
> > + if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32)
> > + off += arch_dest_reloc_offset(reloc->addend);
> > + else
> > + off += reloc->addend;
>
> it looks kind of strange to have arch_dest_reloc_offset() and still
> reference arch-specific relocation types here. On the other hand it seems
> difficult to achieve complete arch-agnostic code, so take it just as a
> note and maybe someone porting objtool to a different architecture will
> split the code, make it all arch-independent and all will be nice and
> shiny.
Something like so perhaps? Seems to build and boot x86_64-defconfig.
---
tools/objtool/arch/x86/decode.c | 9 +++++++--
tools/objtool/check.c | 18 +++++++-----------
tools/objtool/include/objtool/arch.h | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 8b990a52aada..775e1963ecfc 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -63,9 +63,14 @@ bool arch_callee_saved_reg(unsigned char reg)
}
}
-unsigned long arch_dest_reloc_offset(int addend)
+unsigned long arch_dest_reloc_offset(struct reloc *reloc)
{
- return addend + 4;
+ unsigned long offset = reloc->sym->offset + reloc->addend;
+
+ if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32)
+ offset += 4;
+
+ return offset;
}
unsigned long arch_jump_destination(struct instruction *insn)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2063f9fea1a2..5752013dd6e8 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1295,7 +1295,7 @@ static int add_jump_destinations(struct objtool_file *file)
dest_off = arch_jump_destination(insn);
} else if (reloc->sym->type == STT_SECTION) {
dest_sec = reloc->sym->sec;
- dest_off = arch_dest_reloc_offset(reloc->addend);
+ dest_off = arch_dest_reloc_offset(reloc);
} else if (reloc->sym->retpoline_thunk) {
add_retpoline_call(file, insn);
continue;
@@ -1308,8 +1308,7 @@ static int add_jump_destinations(struct objtool_file *file)
continue;
} else if (reloc->sym->sec->idx) {
dest_sec = reloc->sym->sec;
- dest_off = reloc->sym->sym.st_value +
- arch_dest_reloc_offset(reloc->addend);
+ dest_off = arch_dest_reloc_offset(reloc);
} else {
/* non-func asm code jumping to another file */
continue;
@@ -1413,7 +1412,7 @@ static int add_call_destinations(struct objtool_file *file)
}
} else if (reloc->sym->type == STT_SECTION) {
- dest_off = arch_dest_reloc_offset(reloc->addend);
+ dest_off = arch_dest_reloc_offset(reloc);
dest = find_call_destination(reloc->sym->sec, dest_off);
if (!dest) {
WARN_FUNC("can't find call dest symbol at %s+0x%lx",
@@ -3031,6 +3030,7 @@ static inline const char *call_dest_name(struct instruction *insn)
static bool pv_call_dest(struct objtool_file *file, struct instruction *insn)
{
struct symbol *target;
+ unsigned long offset;
struct reloc *rel;
int idx;
@@ -3038,7 +3038,8 @@ static bool pv_call_dest(struct objtool_file *file, struct instruction *insn)
if (!rel || strcmp(rel->sym->name, "pv_ops"))
return false;
- idx = (arch_dest_reloc_offset(rel->addend) / sizeof(void *));
+ offset = arch_dest_reloc_offset(rel) - rel->sym->offset;
+ idx = offset / sizeof(void *);
if (file->pv_ops[idx].clean)
return true;
@@ -3709,12 +3710,7 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn
if (reloc->sym->static_call_tramp)
continue;
- off = reloc->sym->offset;
- if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32)
- off += arch_dest_reloc_offset(reloc->addend);
- else
- off += reloc->addend;
-
+ off = arch_dest_reloc_offset(reloc);
dest = find_insn(file, reloc->sym->sec, off);
if (!dest)
continue;
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 9b19cc304195..57562eaa0967 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -81,7 +81,7 @@ bool arch_callee_saved_reg(unsigned char reg);
unsigned long arch_jump_destination(struct instruction *insn);
-unsigned long arch_dest_reloc_offset(int addend);
+unsigned long arch_dest_reloc_offset(struct reloc *reloc);
const char *arch_nop_insn(int len);
const char *arch_ret_insn(int len);
Powered by blists - more mailing lists