Re: [RFC] Have insn decoder functions return success/failure

From: Borislav Petkov
Date: Fri Oct 30 2020 - 09:07:34 EST


On Fri, Oct 30, 2020 at 10:24:53AM +0900, Masami Hiramatsu wrote:
> What's the objdump say here?

The expected "bad":

0: c5 ec 95 (bad)
3: b2 02 mov $0x2,%dl
5: bd 4b c8 a8 36 mov $0x36a8c84b,%ebp
a: b2 c5 mov $0xc5,%dl
c: c0 df 13 rcr $0x13,%bh

> Yes, in this case, we would better to handle it as an undecodable input
> instead of access violation in insn_sanity.

Ok, good. I've got the hunk below now and it does the right thing. The
whole patch has become huuge now, lemme split it finally. :)

Thx.

---
diff --git a/arch/x86/tools/insn_sanity.c b/arch/x86/tools/insn_sanity.c
index 185ceba9d289..f20765beec9c 100644
--- a/arch/x86/tools/insn_sanity.c
+++ b/arch/x86/tools/insn_sanity.c
@@ -222,8 +224,8 @@ static void parse_args(int argc, char **argv)

int main(int argc, char **argv)
{
+ int insns = 0, ret;
struct insn insn;
- int insns = 0;
int errors = 0;
unsigned long i;
unsigned char insn_buff[MAX_INSN_SIZE * 2];
@@ -241,15 +243,15 @@ int main(int argc, char **argv)
continue;

/* Decode an instruction */
- insn_init(&insn, insn_buff, sizeof(insn_buff), x86_64);
- insn_get_length(&insn);
+ ret = insn_decode(&insn, insn_buff, sizeof(insn_buff),
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);

if (insn.next_byte <= insn.kaddr ||
insn.kaddr + MAX_INSN_SIZE < insn.next_byte) {
/* Access out-of-range memory */
dump_stream(stderr, "Error: Found an access violation", i, insn_buff, &insn);
errors++;
- } else if (verbose && !insn_complete(&insn))
+ } else if (verbose && ret < 0)
dump_stream(stdout, "Info: Found an undecodable input", i, insn_buff, &insn);
else if (verbose >= 2)
dump_insn(stdout, &insn);

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette