kpatch-build: use symbol table instead of kobject

create-diff-object doesn't really need the full kernel object file as
input. All it requires is a symbol table. Switch to using "eu-readelf -s"'s
output instead of object files. This will enable us to cover more cases
in unit tests.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Artem Savkov <asavkov@redhat.com>
This commit is contained in:
Artem Savkov 2018-05-11 11:51:00 +02:00
parent e790d59bec
commit 3aa5abb807
6 changed files with 124 additions and 135 deletions

View File

@ -1,7 +1,7 @@
language: c
before_install:
- sudo apt-get -qq update
- sudo apt-get install -y libelf-dev linux-headers-$(uname -r) shellcheck
- sudo apt-get install -y libelf-dev linux-headers-$(uname -r) shellcheck elfutils
script:
- make

View File

@ -2984,11 +2984,11 @@ static void kpatch_build_strings_section_data(struct kpatch_elf *kelf)
}
struct arguments {
char *args[6];
char *args[7];
int debug;
};
static char args_doc[] = "original.o patched.o kernel-object output.o Module.symvers patch-module-name";
static char args_doc[] = "original.o patched.o parent-name parent-symtab Module.symvers patch-module-name output.o";
static struct argp_option options[] = {
{"debug", 'd', NULL, 0, "Show debug output" },
@ -3007,13 +3007,13 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
arguments->debug = 1;
break;
case ARGP_KEY_ARG:
if (state->arg_num >= 6)
if (state->arg_num >= 7)
/* Too many arguments. */
argp_usage (state);
arguments->args[state->arg_num] = arg;
break;
case ARGP_KEY_END:
if (state->arg_num < 6)
if (state->arg_num < 7)
/* Not enough arguments. */
argp_usage (state);
break;
@ -3033,8 +3033,8 @@ int main(int argc, char *argv[])
struct lookup_table *lookup;
struct section *sec, *symtab;
struct symbol *sym;
char *hint = NULL, *objname, *pos, *orig_obj, *patched_obj;
char *parent_kobj, *mod_symvers, *patch_name, *output_obj;
char *hint = NULL, *orig_obj, *patched_obj, *parent_name;
char *parent_symtab, *mod_symvers, *patch_name, *output_obj;
struct sym_compare_type *base_locals;
arguments.debug = 0;
@ -3044,12 +3044,13 @@ int main(int argc, char *argv[])
elf_version(EV_CURRENT);
orig_obj = arguments.args[0];
patched_obj = arguments.args[1];
parent_kobj = arguments.args[2];
output_obj = arguments.args[3];
mod_symvers = arguments.args[4];
patch_name = arguments.args[5];
orig_obj = arguments.args[0];
patched_obj = arguments.args[1];
parent_name = arguments.args[2];
parent_symtab = arguments.args[3];
mod_symvers = arguments.args[4];
patch_name = arguments.args[5];
output_obj = arguments.args[6];
childobj = basename(orig_obj);
@ -3074,7 +3075,7 @@ int main(int argc, char *argv[])
/* create symbol lookup table */
base_locals = kpatch_elf_locals(kelf_base);
lookup = lookup_open(parent_kobj, mod_symvers, hint, base_locals);
lookup = lookup_open(parent_symtab, mod_symvers, hint, base_locals);
free(base_locals);
kpatch_mark_grouped_sections(kelf_patched);
@ -3132,27 +3133,12 @@ int main(int argc, char *argv[])
*/
kpatch_elf_teardown(kelf_patched);
/* extract module name (destructive to arguments.modulefile) */
objname = basename(parent_kobj);
if (!strncmp(objname, "vmlinux-", 8))
objname = "vmlinux";
else {
pos = strchr(objname,'.');
if (pos) {
/* kernel module */
*pos = '\0';
pos = objname;
while ((pos = strchr(pos, '-')))
*pos++ = '_';
}
}
/* create strings, patches, and dynrelas sections */
kpatch_create_strings_elements(kelf_out);
kpatch_create_patches_sections(kelf_out, lookup, objname);
kpatch_create_intermediate_sections(kelf_out, lookup, objname, patch_name);
kpatch_create_kpatch_arch_section(kelf_out, objname);
kpatch_create_callbacks_objname_rela(kelf_out, objname);
kpatch_create_patches_sections(kelf_out, lookup, parent_name);
kpatch_create_intermediate_sections(kelf_out, lookup, parent_name, patch_name);
kpatch_create_kpatch_arch_section(kelf_out, parent_name);
kpatch_create_callbacks_objname_rela(kelf_out, parent_name);
kpatch_build_strings_section_data(kelf_out);
kpatch_create_mcount_sections(kelf_out);

View File

@ -773,16 +773,26 @@ for i in $FILES; do
mkdir -p "output/$(dirname "$i")"
cd "$SRCDIR" || die
find_kobj "$i"
if [[ "$KOBJFILE" = vmlinux ]]; then
KOBJFILE="$VMLINUX"
else
KOBJFILE="$TEMPDIR/module/$KOBJFILE"
fi
cd "$TEMPDIR" || die
if [[ -e "orig/$i" ]]; then
# create-diff-object orig.o patched.o kernel-object output.o Module.symvers patch-mod-name
"$TOOLSDIR"/create-diff-object "orig/$i" "patched/$i" "$KOBJFILE" \
"output/$i" "$SRCDIR/Module.symvers" "${MODNAME//-/_}" 2>&1 | logger 1
if [[ "$KOBJFILE" = vmlinux ]]; then
KOBJFILE_NAME=vmlinux
KOBJFILE_PATH="$VMLINUX"
SYMTAB="${TEMPDIR}/${KOBJFILE_NAME}.symtab"
else
KOBJFILE_NAME=$(basename "${KOBJFILE%.ko}")
KOBJFILE_NAME="${KOBJFILE_NAME/-/_}"
KOBJFILE_PATH="${TEMPDIR}/module/$KOBJFILE"
SYMTAB="${KOBJFILE_PATH}.symtab"
fi
eu-readelf -s "$KOBJFILE_PATH" > "$SYMTAB"
# create-diff-object orig.o patched.o parent-name parent-symtab
# Module.symvers patch-mod-name output.o
"$TOOLSDIR"/create-diff-object "orig/$i" "patched/$i" "$KOBJFILE_NAME" \
"$SYMTAB" "$SRCDIR/Module.symvers" "${MODNAME//-/_}" \
"output/$i" 2>&1 | logger 1
check_pipe_status create-diff-object
# create-diff-object returns 3 if no functional change is found
[[ "$rc" -eq 0 ]] || [[ "$rc" -eq 3 ]] || ERROR="$((ERROR + 1))"
@ -808,7 +818,7 @@ fi
echo -n "Patched objects:"
for i in $(echo "${objnames[@]}" | tr ' ' '\n' | sort -u | tr '\n' ' ')
do
echo -n " $(basename "$i")"
echo -n " $i"
done
echo

View File

@ -43,7 +43,7 @@ struct object_symbol {
unsigned long value;
unsigned long size;
char *name;
int type, bind, skip;
int type, bind;
};
struct export_symbol {
@ -174,86 +174,6 @@ static void find_local_syms(struct lookup_table *table, char *hint,
ERROR("find_local_syms for %s: found_none", hint);
}
static void obj_read(struct lookup_table *table, char *path)
{
Elf *elf;
int fd, i, len;
Elf_Scn *scn;
GElf_Shdr sh;
GElf_Sym sym;
Elf_Data *data;
char *name;
struct object_symbol *mysym;
size_t shstrndx;
if ((fd = open(path, O_RDONLY, 0)) < 0)
ERROR("open");
elf_version(EV_CURRENT);
elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
if (!elf) {
printf("%s\n", elf_errmsg(-1));
ERROR("elf_begin");
}
if (elf_getshdrstrndx(elf, &shstrndx))
ERROR("elf_getshdrstrndx");
scn = NULL;
while ((scn = elf_nextscn(elf, scn))) {
if (!gelf_getshdr(scn, &sh))
ERROR("gelf_getshdr");
name = elf_strptr(elf, shstrndx, sh.sh_name);
if (!name)
ERROR("elf_strptr scn");
if (!strcmp(name, ".symtab"))
break;
}
if (!scn)
ERROR(".symtab section not found");
data = elf_getdata(scn, NULL);
if (!data)
ERROR("elf_getdata");
len = sh.sh_size / sh.sh_entsize;
table->obj_syms = malloc(len * sizeof(*table->obj_syms));
if (!table->obj_syms)
ERROR("malloc table.obj_syms");
memset(table->obj_syms, 0, len * sizeof(*table->obj_syms));
table->obj_nr = len;
for_each_obj_symbol(i, mysym, table) {
if (!gelf_getsym(data, i, &sym))
ERROR("gelf_getsym");
if (sym.st_shndx == SHN_UNDEF) {
mysym->skip = 1;
continue;
}
name = elf_strptr(elf, sh.sh_link, sym.st_name);
if(!name)
ERROR("elf_strptr sym");
mysym->value = sym.st_value;
mysym->size = sym.st_size;
mysym->type = GELF_ST_TYPE(sym.st_info);
mysym->bind = GELF_ST_BIND(sym.st_info);
mysym->name = strdup(name);
if (!mysym->name)
ERROR("strdup");
}
close(fd);
elf_end(elf);
}
/* Strip the path and replace '-' with '_' */
static char *make_modname(char *modname)
{
@ -272,6 +192,72 @@ static char *make_modname(char *modname)
return basename(modname);
}
static void symtab_read(struct lookup_table *table, char *path)
{
FILE *file;
long unsigned int value, size;
unsigned int i = 0;
char line[256], name[256], type[16], bind[16];
if ((file = fopen(path, "r")) == NULL)
ERROR("fopen");
while (fgets(line, 256, file)) {
if (sscanf(line, "%*s %lx %lu %s %s %*s %*s %s\n",
&value, &size, type, bind, name) == 5 &&
strcmp(bind, "SECTION")) {
table->obj_nr++;
}
}
table->obj_syms = malloc(table->obj_nr * sizeof(*table->obj_syms));
if (!table->obj_syms)
ERROR("malloc table.obj_syms");
memset(table->obj_syms, 0, table->obj_nr * sizeof(*table->obj_syms));
rewind(file);
while (fgets(line, 256, file)) {
if (sscanf(line, "%*s %lx %lu %s %s %*s %*s %s\n",
&value, &size, type, bind, name) != 5 ||
!strcmp(bind, "SECTION")) {
continue;
}
table->obj_syms[i].value = value;
table->obj_syms[i].size = size;
table->obj_syms[i].name = strdup(name);
if (!strcmp(bind, "LOCAL")) {
table->obj_syms[i].bind = STB_LOCAL;
} else if (!strcmp(bind, "GLOBAL")) {
table->obj_syms[i].bind = STB_GLOBAL;
} else if (!strcmp(bind, "WEAK")) {
table->obj_syms[i].bind = STB_WEAK;
} else {
ERROR("unknown symbol bind %s", bind);
}
if (!strcmp(type, "NOTYPE")) {
table->obj_syms[i].type = STT_NOTYPE;
} else if (!strcmp(type, "OBJECT")) {
table->obj_syms[i].type = STT_OBJECT;
} else if (!strcmp(type, "FUNC")) {
table->obj_syms[i].type = STT_FUNC;
} else if (!strcmp(type, "FILE")) {
table->obj_syms[i].type = STT_FILE;
} else {
ERROR("unknown symbol type %s", type);
}
table->obj_syms[i].name = strdup(name);
if (!table->obj_syms[i].name)
ERROR("strdup");
i++;
}
fclose(file);
}
static void symvers_read(struct lookup_table *table, char *path)
{
FILE *file;
@ -279,7 +265,7 @@ static void symvers_read(struct lookup_table *table, char *path)
char name[256], mod[256], export[256];
char *objname, *symname;
if ((file = fopen(path, "r")) < 0)
if ((file = fopen(path, "r")) == NULL)
ERROR("fopen");
while (fscanf(file, "%x %s %s %s\n",
@ -314,7 +300,7 @@ static void symvers_read(struct lookup_table *table, char *path)
fclose(file);
}
struct lookup_table *lookup_open(char *obj_path, char *symvers_path,
struct lookup_table *lookup_open(char *symtab_path, char *symvers_path,
char *hint, struct sym_compare_type *locals)
{
struct lookup_table *table;
@ -324,7 +310,7 @@ struct lookup_table *lookup_open(char *obj_path, char *symvers_path,
ERROR("malloc table");
memset(table, 0, sizeof(*table));
obj_read(table, obj_path);
symtab_read(table, symtab_path);
symvers_read(table, symvers_path);
find_local_syms(table, hint, locals);
@ -350,9 +336,6 @@ int lookup_local_symbol(struct lookup_table *table, char *name,
memset(result, 0, sizeof(*result));
for_each_obj_symbol(i, sym, table) {
if (sym->skip)
continue;
if (sym->bind == STB_LOCAL && !strcmp(sym->name, name))
pos++;
@ -390,7 +373,7 @@ int lookup_global_symbol(struct lookup_table *table, char *name,
memset(result, 0, sizeof(*result));
for_each_obj_symbol(i, sym, table) {
if (!sym->skip && (sym->bind == STB_GLOBAL || sym->bind == STB_WEAK) &&
if ((sym->bind == STB_GLOBAL || sym->bind == STB_WEAK) &&
!strcmp(sym->name, name)) {
result->value = sym->value;
result->size = sym->size;

View File

@ -14,7 +14,7 @@ struct sym_compare_type {
int type;
};
struct lookup_table *lookup_open(char *obj_path, char *symvers_path,
struct lookup_table *lookup_open(char *symtab_path, char *symvers_path,
char *hint, struct sym_compare_type *locals);
void lookup_close(struct lookup_table *table);
int lookup_local_symbol(struct lookup_table *table, char *name,

View File

@ -4,6 +4,8 @@ EXT_FAIL ?= PATCHED.FAIL.o
EXT_TEST ?= test
EXT_OUTPUT ?= OUTPUT.o
EXT_TEST_OUTPUT ?= test.out
EXT_SYMTAB ?= symtab
EXT_SYMVERS ?= symvers
TNAME = $(@:.$(EXT_OUTPUT)=)
ifndef VERBOSE
@ -21,6 +23,8 @@ TEST_ENV = KPATCH_TEST_LIBRARY=$(TEST_LIBRARY)
TARGETS = $(patsubst %.$(EXT_ORIG),%.$(EXT_OUTPUT),$(wildcard *.$(EXT_ORIG)))
TEST_TARGETS = $(patsubst %.$(EXT_TEST),%.$(EXT_TEST_OUTPUT),$(wildcard *.$(EXT_TEST)))
SYMVERS_FILE = $(if $(wildcard $(TNAME).$(EXT_SYMVERS)),$(TNAME).$(EXT_SYMVERS),/dev/null)
define check_stripped =
$(if $(shell readelf --debug-dump $(1)),
$(error $(1) is not properly stripped, use 'strip --strip-debug --keep-file-symbols $(1)' to fix this),
@ -31,30 +35,36 @@ define check_all =
$(call check_stripped,$(1))
endef
all: $(TARGETS) $(TEST_TARGETS)
clean:
rm -f *.$(EXT_TEST_OUTPUT) *.$(EXT_OUTPUT)
%.$(EXT_SYMTAB):
eu-readelf -s $(patsubst %.$(EXT_SYMTAB),%.$(EXT_ORIG),$(@)) >$@
%.$(EXT_TEST_OUTPUT): %.$(EXT_OUTPUT) %.$(EXT_TEST) $(TEST_LIBRARY)
@echo "TEST $(@:.$(EXT_TEST_OUTPUT)=)"
$(TEST_ENV) bash $(@:.$(EXT_TEST_OUTPUT)=.$(EXT_TEST)) $<
# Don't rely on script creating this
@touch $@
%.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_PATCHED) $(CDO)
%.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_PATCHED) %.$(EXT_SYMTAB) $(CDO)
@echo "BUILD $(TNAME)"
$(call check_all,$(TNAME).$(EXT_ORIG))
$(call check_all,$(TNAME).$(EXT_PATCHED))
$(CDO_ENV) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_PATCHED) \
$(TNAME).$(EXT_ORIG) $@ /dev/null test_$(TNAME) $(MUTE_PASS)
vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \
test_$(TNAME) $@ $(MUTE_PASS)
%.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_FAIL) $(CDO)
%.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_FAIL) %.$(EXT_SYMTAB) $(CDO)
@echo "BUILD $(TNAME)-FAIL"
$(call check_all,$(TNAME).$(EXT_ORIG))
$(call check_all,$(TNAME).$(EXT_FAIL))
! $(CDO_ENV) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_FAIL) \
$(TNAME).$(EXT_ORIG) $@ /dev/null test_$(TNAME) $(MUTE_FAIL)
vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \
test_$(TNAME) $@ $(MUTE_FAIL)
# Expecting to fail, thus create output file manually so we won't rerun the
# test without clean
@touch $@