From ea819a18b0681531a2b925bd6d592c0672a6c780 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 7 Oct 2014 22:01:14 -0500 Subject: [PATCH 1/2] warn detection fix The current WARN detection logic catches the majority of cases, but there are still a lot of outliers which it doesn't catch (thanks, gcc). I looked at a much larger sample of WARN calls and came up with a more generic algorithm. --- kpatch-build/create-diff-object.c | 46 +++++++++++++++++-------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index eaa27ee..9b68605 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -658,15 +658,20 @@ out: * * base object: * - * 28e: be de 10 00 00 mov $0x10de,%esi - * 293: 48 c7 c7 00 00 00 00 mov $0x0,%rdi - * 29a: e8 00 00 00 00 callq 29f + * be 5e 04 00 00 mov $0x45e,%esi + * 48 c7 c7 ff 5a a1 81 mov $0xffffffff81a15aff,%rdi + * e8 26 13 08 00 callq ffffffff8108d0d0 * * patched object: * - * 28e: be df 10 00 00 mov $0x10df,%esi - * 293: 48 c7 c7 00 00 00 00 mov $0x0,%rdi - * 29a: e8 00 00 00 00 callq 29f + * be 5e 04 00 00 mov $0x45f,%esi + * 48 c7 c7 ff 5a a1 81 mov $0xffffffff81a15aff,%rdi + * e8 26 13 08 00 callq ffffffff8108d0d0 + * + * The above is the most common case. The pattern which applies to all cases + * is an immediate move of the line number to %esi followed by zero or more + * relas to a string section followed by a rela to warn_slowpath_*. + * */ static int kpatch_warn_only_change(struct section *sec) { @@ -710,21 +715,21 @@ static int kpatch_warn_only_change(struct section *sec) if (insn1.opcode.value != 0xbe || insn2.opcode.value != 0xbe) return 0; - /* verify the next instruction is mov $0x0,%rdi */ - if (*(unsigned int *)(start1 + offset + length) != 0xc7c748) - return 0; - - /* verify the instruction after that is callq */ - if (*(unsigned int *)(start1 + offset + length + 7) != 0xe8) - return 0; - - /* verify the callq's rela is for warn_slowpath_null */ + /* + * Verify zero or more string relas followed by a + * warn_slowpath_* rela. + */ found = 0; list_for_each_entry(rela, &sec->rela->relas, list) { - if (rela->offset == offset + length + 8 && - !strcmp(rela->sym->name, "warn_slowpath_null")) { - found = 1; - break; + if (rela->offset >= offset + length) { + if (rela->string) + continue; + if (!strncmp(rela->sym->name, + "warn_slowpath_", 14)) { + found = 1; + break; + } + return 0; } } if (!found) @@ -734,7 +739,8 @@ static int kpatch_warn_only_change(struct section *sec) } if (!warnonly) - ERROR("no instruction changes detected for changed section %s", sec->name); + ERROR("no instruction changes detected for changed section %s", + sec->name); return 1; } From fca189152a476de331fbb5997f540d8bc4f34599 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 8 Oct 2014 11:16:09 -0500 Subject: [PATCH 2/2] fix review comment --- kpatch-build/create-diff-object.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 9b68605..53191fb 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -721,16 +721,15 @@ static int kpatch_warn_only_change(struct section *sec) */ found = 0; list_for_each_entry(rela, &sec->rela->relas, list) { - if (rela->offset >= offset + length) { - if (rela->string) - continue; - if (!strncmp(rela->sym->name, - "warn_slowpath_", 14)) { - found = 1; - break; - } - return 0; + if (rela->offset < offset + length) + continue; + if (rela->string) + continue; + if (!strncmp(rela->sym->name, "warn_slowpath_", 14)) { + found = 1; + break; } + return 0; } if (!found) return 0;