[v2] KEYS: fix memory leaks when reading certificate

Commit Message

Denis Glazkov March 3, 2022, 12:56 p.m. UTC

The `exit()` function usage produce possible memory leaks. This
patch removes the use of the `exit()` function and adds memory
free in case of a negative scenarios.

Fixes: c4c361059585 ("KEYS: Reserve an extra certificate symbol for inserting without recompiling")
Signed-off-by: Denis Glazkov <[e-mail]>
---
 scripts/insert-sys-cert.c | 51 +++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 18 deletions(-)

Comments

David Howells March 30, 2022, 3:37 p.m. UTC | #1

Denis Glazkov <[e-mail]> wrote:

> The `exit()` function usage produce possible memory leaks. This
> patch removes the use of the `exit()` function and adds memory
> free in case of a negative scenarios.

?

Barring a kernel bug, there should be no memory leaks from exit().  _exit() is
the ultimate process cleanup tool.  Calling free() won't necessarily return
the memory allocated by malloc() to the kernel.

Unless you have a good reason to actually tear down everything, just print a
message and call exit on error in little helpers like this.

David

Patch

diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
index 8902836c2342..8046682188f3 100644
--- a/scripts/insert-sys-cert.c
+++ b/scripts/insert-sys-cert.c
@@ -101,7 +101,7 @@  static void get_symbol_from_map(Elf_Ehdr *hdr, FILE *f, char *name,
 	s->offset = 0;
 	if (fseek(f, 0, SEEK_SET) != 0) {
 		perror("File seek failed");
-		exit(EXIT_FAILURE);
+		return;
 	}
 	while (fgets(l, LINE_SIZE, f)) {
 		p = strchr(l, '\n');
@@ -251,6 +251,7 @@  static char *read_file(char *file_name, int *size)
 	if (read(fd, buf, *size) != *size) {
 		perror("File read failed");
 		close(fd);
+		free(buf);
 		return NULL;
 	}
 	close(fd);
@@ -277,14 +278,15 @@  int main(int argc, char **argv)
 	char *cert_file = NULL;
 	int vmlinux_size;
 	int cert_size;
-	Elf_Ehdr *hdr;
-	char *cert;
-	FILE *system_map;
+	Elf_Ehdr *hdr = NULL;
+	char *cert = NULL;
+	FILE *system_map = NULL;
 	unsigned long *lsize;
 	int *used;
 	int opt;
 	Elf_Shdr *symtab = NULL;
 	struct sym cert_sym, lsize_sym, used_sym;
+	int ret = EXIT_FAILURE;
 
 	while ((opt = getopt(argc, argv, "b:c:s:")) != -1) {
 		switch (opt) {
@@ -304,20 +306,20 @@  int main(int argc, char **argv)
 
 	if (!vmlinux_file || !cert_file) {
 		print_usage(argv[0]);
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	cert = read_file(cert_file, &cert_size);
 	if (!cert)
-		exit(EXIT_FAILURE);
+		goto finish;
 
 	hdr = map_file(vmlinux_file, &vmlinux_size);
 	if (!hdr)
-		exit(EXIT_FAILURE);
+		goto finish;
 
 	if (vmlinux_size < sizeof(*hdr)) {
 		err("Invalid ELF file.\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	if ((hdr->e_ident[EI_MAG0] != ELFMAG0) ||
@@ -325,22 +327,22 @@  int main(int argc, char **argv)
 	    (hdr->e_ident[EI_MAG2] != ELFMAG2) ||
 	    (hdr->e_ident[EI_MAG3] != ELFMAG3)) {
 		err("Invalid ELF magic.\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	if (hdr->e_ident[EI_CLASS] != CURRENT_ELFCLASS) {
 		err("ELF class mismatch.\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	if (hdr->e_ident[EI_DATA] != endianness()) {
 		err("ELF endian mismatch.\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	if (hdr->e_shoff > vmlinux_size) {
 		err("Could not find section header.\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
 	symtab = get_symbol_table(hdr);
@@ -349,13 +351,13 @@  int main(int argc, char **argv)
 		if (!system_map_file) {
 			err("Please provide a System.map file.\n");
 			print_usage(argv[0]);
-			exit(EXIT_FAILURE);
+			goto finish;
 		}
 
 		system_map = fopen(system_map_file, "r");
 		if (!system_map) {
 			perror(system_map_file);
-			exit(EXIT_FAILURE);
+			goto finish;
 		}
 		get_symbol_from_map(hdr, system_map, CERT_SYM, &cert_sym);
 		get_symbol_from_map(hdr, system_map, USED_SYM, &used_sym);
@@ -371,7 +373,7 @@  int main(int argc, char **argv)
 	}
 
 	if (!cert_sym.offset || !lsize_sym.offset || !used_sym.offset)
-		exit(EXIT_FAILURE);
+		goto finish;
 
 	print_sym(hdr, &cert_sym);
 	print_sym(hdr, &used_sym);
@@ -382,14 +384,16 @@  int main(int argc, char **argv)
 
 	if (cert_sym.size < cert_size) {
 		err("Certificate is larger than the reserved area!\n");
-		exit(EXIT_FAILURE);
+		goto finish;
 	}
 
+	ret = EXIT_SUCCESS;
+
 	/* If the existing cert is the same, don't overwrite */
 	if (cert_size == *used &&
 	    strncmp(cert_sym.content, cert, cert_size) == 0) {
 		warn("Certificate was already inserted.\n");
-		exit(EXIT_SUCCESS);
+		goto finish;
 	}
 
 	if (*used > 0)
@@ -406,5 +410,16 @@  int main(int argc, char **argv)
 						cert_sym.address);
 	info("Used %d bytes out of %d bytes reserved.\n", *used,
 						 cert_sym.size);
-	exit(EXIT_SUCCESS);
+
+finish:
+	if (cert != NULL)
+		free(cert);
+
+	if (hdr != NULL)
+		munmap(hdr, vmlinux_size);
+
+	if (system_map != NULL)
+		fclose(system_map);
+
+	return ret;
 }