In the `read_file` function of `insert-sys-cert.c` script, if
the data is read incorrectly, the memory allocated for the `buf`
array is not freed.
Fixes: c4c361059585 ("KEYS: Reserve an extra certificate symbol for inserting without recompiling")
Signed-off-by: Denis Glazkov <[email protected]>
---
scripts/insert-sys-cert.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
index 8902836c2342..b98a0b12f16f 100644
--- a/scripts/insert-sys-cert.c
+++ b/scripts/insert-sys-cert.c
@@ -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);
--
2.25.1
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 <[email protected]>
---
scripts/insert-sys-cert.c | 51 +++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 18 deletions(-)
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;
}
--
2.25.1
On Thu, Mar 3, 2022 at 7:49 PM Denis Glazkov <[email protected]> wrote:
>
> In the `read_file` function of `insert-sys-cert.c` script, if
> the data is read incorrectly, the memory allocated for the `buf`
> array is not freed.
>
> Fixes: c4c361059585 ("KEYS: Reserve an extra certificate symbol for inserting without recompiling")
> Signed-off-by: Denis Glazkov <[email protected]>
> ---
> scripts/insert-sys-cert.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
> index 8902836c2342..b98a0b12f16f 100644
> --- a/scripts/insert-sys-cert.c
> +++ b/scripts/insert-sys-cert.c
> @@ -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);
Hi Denis,
There is another issue related to variable buf. On the success path,
buf will be assigned to variable cert in the main function. And cert
is not free when the main function exits.
> --
> 2.25.1
Denis Glazkov <[email protected]> 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