Merge pull request #849 from sm00th/cdo-symtab

create-diff-object symtab

.travis.yml

@@ -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

kpatch-build/create-diff-object.c

@@ -2993,11 +2993,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" },
@@ -3016,13 +3016,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;
@@ -3042,8 +3042,8 @@ int main(int argc, char *argv[])
 	struct lookup_table *lookup;
 	struct section *sec, *symtab;
 	struct symbol *sym;
-	char *hint = NULL, *objname, *pos;
+	char *hint = NULL, *orig_obj, *patched_obj, *parent_name;
-	char *mod_symvers_path, *pmod_name;
+	char *parent_symtab, *mod_symvers, *patch_name, *output_obj;
 	struct sym_compare_type *base_locals;
 
 	arguments.debug = 0;
@@ -3053,13 +3053,18 @@ int main(int argc, char *argv[])
 
 	elf_version(EV_CURRENT);
 
-	childobj = basename(arguments.args[0]);
+	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];
 
-	mod_symvers_path = arguments.args[4];
+	childobj = basename(orig_obj);
-	pmod_name = arguments.args[5];
 
-	kelf_base = kpatch_elf_open(arguments.args[0]);
+	kelf_base = kpatch_elf_open(orig_obj);
-	kelf_patched = kpatch_elf_open(arguments.args[1]);
+	kelf_patched = kpatch_elf_open(patched_obj);
 
 	kpatch_bundle_symbols(kelf_base);
 	kpatch_bundle_symbols(kelf_patched);
@@ -3079,7 +3084,7 @@ int main(int argc, char *argv[])
 
 	/* create symbol lookup table */
 	base_locals = kpatch_elf_locals(kelf_base);
-	lookup = lookup_open(arguments.args[2], mod_symvers_path, hint, base_locals);
+	lookup = lookup_open(parent_symtab, mod_symvers, hint, base_locals);
 	free(base_locals);
 
 	kpatch_mark_grouped_sections(kelf_patched);
@@ -3137,27 +3142,12 @@ int main(int argc, char *argv[])
 	 */
 	kpatch_elf_teardown(kelf_patched);
 
-	/* extract module name (destructive to arguments.modulefile) */
-	objname = basename(arguments.args[2]);
-	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_patches_sections(kelf_out, lookup, parent_name);
-	kpatch_create_intermediate_sections(kelf_out, lookup, objname, pmod_name);
+	kpatch_create_intermediate_sections(kelf_out, lookup, parent_name, patch_name);
-	kpatch_create_kpatch_arch_section(kelf_out, objname);
+	kpatch_create_kpatch_arch_section(kelf_out, parent_name);
-	kpatch_create_callbacks_objname_rela(kelf_out, objname);
+	kpatch_create_callbacks_objname_rela(kelf_out, parent_name);
 	kpatch_build_strings_section_data(kelf_out);
 
 	kpatch_create_mcount_sections(kelf_out);
@@ -3191,7 +3181,7 @@ int main(int argc, char *argv[])
 	kpatch_create_strtab(kelf_out);
 	kpatch_create_symtab(kelf_out);
 	kpatch_dump_kelf(kelf_out);
-	kpatch_write_output_elf(kelf_out, kelf_patched->elf, arguments.args[3]);
+	kpatch_write_output_elf(kelf_out, kelf_patched->elf, output_obj);
 
 	kpatch_elf_free(kelf_patched);
 	kpatch_elf_teardown(kelf_out);

kpatch-build/kpatch-build

@@ -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
+		if [[ "$KOBJFILE" = vmlinux ]]; then
-		"$TOOLSDIR"/create-diff-object "orig/$i" "patched/$i" "$KOBJFILE" \
+			KOBJFILE_NAME=vmlinux
-			"output/$i" "$SRCDIR/Module.symvers" "${MODNAME//-/_}" 2>&1 | logger 1
+			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
 

kpatch-build/lookup.c

@@ -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;

kpatch-build/lookup.h

@@ -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,

test/unit/Makefile.include

@@ -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 $@