From e6299e970cc2d1ed97364f0294a9862e05751257 Mon Sep 17 00:00:00 2001 From: J. Ali Harlow Date: Fri, 17 Oct 2014 10:10:57 +0100 Subject: [PATCH] Fix multiple memory allocation problems (found with valgrind) --- librazor/atomic-actions.c | 2 + librazor/atomic-emulate.c | 9 ++++++ librazor/importer.c | 14 ++++++--- librazor/iterator.c | 6 ++-- librazor/merger.c | 18 +++++++++-- librazor/razor-internal.h | 2 +- librazor/razor.c | 1 + librazor/rpm.c | 6 ++++ librazor/transaction.c | 2 + librazor/types/graph.c | 7 ++++- librazor/util.c | 69 ++++++++++++++++++++++++++++++++++++++++++-- src/main.c | 4 ++ 12 files changed, 122 insertions(+), 18 deletions(-) diff --git a/librazor/atomic-actions.c b/librazor/atomic-actions.c index 8215cd4..b90592c 100644 --- a/librazor/atomic-actions.c +++ b/librazor/atomic-actions.c @@ -190,6 +190,8 @@ atomic_action_make_dirs(struct razor_atomic *atomic, } primitives = atomic_action_list_reverse(primitives); + atomic_action_free(action); + return atomic_action_do(atomic, primitives); } diff --git a/librazor/atomic-emulate.c b/librazor/atomic-emulate.c index da65008..40f2653 100644 --- a/librazor/atomic-emulate.c +++ b/librazor/atomic-emulate.c @@ -69,6 +69,8 @@ static void recursive_remove(const char *directory) } } + closedir(dp); + rmdir(directory); } @@ -108,6 +110,11 @@ RAZOR_EXPORT int razor_atomic_commit(struct razor_atomic *atomic) RAZOR_EXPORT void razor_atomic_destroy(struct razor_atomic *atomic) { + if (atomic->actions) { + atomic_action_free(atomic->actions); + atomic->actions = NULL; + } + if (atomic->toplevel) { recursive_remove(atomic->toplevel); free(atomic->toplevel); @@ -117,6 +124,8 @@ RAZOR_EXPORT void razor_atomic_destroy(struct razor_atomic *atomic) if (atomic->error) razor_error_free(atomic->error); + free(atomic->description); + free(atomic); } diff --git a/librazor/importer.c b/librazor/importer.c index c4dc56c..8bf8795 100644 --- a/librazor/importer.c +++ b/librazor/importer.c @@ -415,6 +415,8 @@ serialize_files(struct razor_set *set, serialize_files(set, p, array); p++; } + + array_release(&d->files); } static void @@ -434,6 +436,7 @@ build_file_tree(struct razor_importer *importer) struct import_entry *filenames; char *f, *end; uint32_t name, *r, s; + uint32_t *map; char dirname[256]; struct import_directory *d, *last_root; struct array roots; @@ -441,11 +444,12 @@ build_file_tree(struct razor_importer *importer) count = importer->files.size / sizeof (struct import_entry); filenames = importer->files.data; - razor_qsort_with_data(filenames, - count, - sizeof (struct import_entry), - compare_filenames, - NULL); + map = razor_qsort_with_data(filenames, + count, + sizeof (struct import_entry), + compare_filenames, + NULL); + free(map); array_init(&roots); last_root = NULL; diff --git a/librazor/iterator.c b/librazor/iterator.c index c77cbbf..060467f 100644 --- a/librazor/iterator.c +++ b/librazor/iterator.c @@ -161,8 +161,8 @@ razor_package_iterator_destroy(struct razor_package_iterator *pi) { assert (pi != NULL); - if (pi->free_index) - free(pi->index); + if (pi->alloced_index) + free(pi->alloced_index); free(pi); } @@ -399,7 +399,7 @@ razor_package_query_finish(struct razor_package_query *pq) free(pq); pi = razor_package_iterator_create_with_index(set, index); - pi->free_index = 1; + pi->alloced_index = index; return pi; } diff --git a/librazor/merger.c b/librazor/merger.c index 4038ac8..d86bde2 100644 --- a/librazor/merger.c +++ b/librazor/merger.c @@ -318,8 +318,14 @@ merge_one_directory(struct razor_merger *merger, struct merge_directory *md) set2 = merger->source2.set; map1 = merger->source1.file_map; map2 = merger->source2.file_map; - pool1 = set1->file_string_pool.data; - pool2 = set2->file_string_pool.data; + if (set1->file_string_pool.size) + pool1 = set1->file_string_pool.data; + else + pool1 = NULL; + if (set2->file_string_pool.size) + pool2 = set2->file_string_pool.data; + else + pool2 = NULL; root1 = (struct razor_entry *) set1->files.data; root2 = (struct razor_entry *) set2->files.data; @@ -349,9 +355,9 @@ merge_one_directory(struct razor_merger *merger, struct merge_directory *md) continue; } - if (!e1) + if (!e1 || !pool1) cmp = 1; - else if (!e2) + else if (!e2 || !pool2) cmp = -1; else { cmp = strcmp (&pool1[e1->name], @@ -631,5 +637,9 @@ void razor_merger_destroy(struct razor_merger *merger) hashtable_release(&merger->table); hashtable_release(&merger->file_table); hashtable_release(&merger->details_table); + free(merger->source1.property_map); + free(merger->source1.file_map); + free(merger->source2.property_map); + free(merger->source2.file_map); free(merger); } diff --git a/librazor/razor-internal.h b/librazor/razor-internal.h index 029c26a..e9fff68 100644 --- a/librazor/razor-internal.h +++ b/librazor/razor-internal.h @@ -154,7 +154,7 @@ struct razor_package_iterator { struct razor_set *set; struct razor_package *package, *end; struct list *index; - int free_index; + void *alloced_index; }; void diff --git a/librazor/razor.c b/librazor/razor.c index ab3ab41..60cd2cf 100644 --- a/librazor/razor.c +++ b/librazor/razor.c @@ -1284,6 +1284,7 @@ razor_install_iterator_commit_set(struct razor_install_iterator *ii) set = razor_merger_commit(merger); razor_merger_destroy(merger); + deque_free(done); return set; } diff --git a/librazor/rpm.c b/librazor/rpm.c index c5385c7..643133c 100644 --- a/librazor/rpm.c +++ b/librazor/rpm.c @@ -381,6 +381,12 @@ razor_relocations_apply(struct razor_relocations *rr, const char *path) RAZOR_EXPORT void razor_relocations_destroy(struct razor_relocations *rr) { + int i; + + for (i = 0; i < rr->n_relocations; i++) { + free(rr->relocations[i].oldpath); + free(rr->relocations[i].newpath); + } free(rr->path); free(rr->relocations); free(rr); diff --git a/librazor/transaction.c b/librazor/transaction.c index 60a4d94..fd881e2 100644 --- a/librazor/transaction.c +++ b/librazor/transaction.c @@ -753,6 +753,8 @@ flush_scheduled_upstream_updates(struct razor_transaction *trans) fprintf(stderr, "installing %s-%s\n", name, version); #endif } + + razor_package_iterator_destroy(pi); } RAZOR_EXPORT int diff --git a/librazor/types/graph.c b/librazor/types/graph.c index 6a042c1..126f565 100644 --- a/librazor/types/graph.c +++ b/librazor/types/graph.c @@ -383,10 +383,15 @@ struct deque *graph_sort(struct graph *graph) { struct bitset active_edges; int n_edges; + struct deque *deque; n_edges = graph->edges.size / sizeof(struct graph_edge); bitset_init(active_edges); bitset_set_n_bits(active_edges, 0, n_edges); - return graph_edges_sort(graph, active_edges); + deque = graph_edges_sort(graph, active_edges); + + bitset_release(active_edges); + + return deque; } diff --git a/librazor/util.c b/librazor/util.c index b640939..8d66ad7 100644 --- a/librazor/util.c +++ b/librazor/util.c @@ -62,6 +62,19 @@ zalloc(size_t size) return p; } +#if HAVE_SYS_MMAN_H +#define OPEN_FILE_USED (1U<<0) +#define OPEN_FILE_MMAPPED (1U<<1) + +struct open_file { + void *addr; + size_t length; + uint32_t flags; +}; + +struct array open_files = { 0, }; +#endif /* HAVE_SYS_MMAN_H */ + void * razor_file_get_contents(const char *filename, size_t *length, int private, struct razor_error **error) @@ -71,6 +84,9 @@ razor_file_get_contents(const char *filename, size_t *length, int private, void *addr = NULL; size_t nb; ssize_t res; +#if HAVE_SYS_MMAN_H + struct open_file *of, *ofend; +#endif fd = open(filename, O_RDONLY | O_BINARY); if (fd < 0) { @@ -85,16 +101,31 @@ razor_file_get_contents(const char *filename, size_t *length, int private, } *length = st.st_size; + #if HAVE_SYS_MMAN_H + ofend = open_files.data + open_files.size; + for (of = open_files.data; of < ofend; of++) + if (!(of->flags & OPEN_FILE_USED)) + break; + if (of == ofend) { + of = array_add(&open_files, sizeof *of); + of->flags = 0; + } + if (!private) { addr = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); if (addr == MAP_FAILED) addr = NULL; + else + of->flags = OPEN_FILE_USED | OPEN_FILE_MMAPPED; } -#endif +#endif /* HAVE_SYS_MMAN_H */ if (!addr) { addr = malloc(st.st_size); if (addr) { +#if HAVE_SYS_MMAN_H + of->flags = OPEN_FILE_USED; +#endif nb = 0; while(nb < st.st_size) { res = read(fd, addr + nb, st.st_size - nb); @@ -112,17 +143,47 @@ razor_file_get_contents(const char *filename, size_t *length, int private, } close(fd); +#if HAVE_SYS_MMAN_H + of->addr = addr; + of->length = st.st_size; +#endif + return addr; } int razor_file_free_contents(void *addr, size_t length) { #if HAVE_SYS_MMAN_H - return munmap(addr, length); -#else + int retval, mmapped; + struct open_file *of, *ofend; + + ofend = open_files.data + open_files.size; + for (of = open_files.data; of < ofend; of++) + if ((of->flags & OPEN_FILE_USED) && of->addr == addr) + break; + + if (of == ofend) + return 1; + + length = of->length; + mmapped = of->flags & OPEN_FILE_MMAPPED; + of->flags &= ~OPEN_FILE_USED; + + for (of = open_files.data; of < ofend; of++) + if (of->flags & OPEN_FILE_USED) + break; + + if (of == ofend) { + array_release(&open_files); + array_init(&open_files); + } + + if (mmapped) + return munmap(addr, length); +#endif + free(addr); return 0; -#endif } struct qsort_context { diff --git a/src/main.c b/src/main.c index d9d66b3..5598cfc 100644 --- a/src/main.c +++ b/src/main.c @@ -1375,6 +1375,8 @@ update_system(struct razor_root *root, struct razor_relocations *relocations, razor_atomic_destroy(atomic); } while(!retval && r == 1); + razor_install_iterator_destroy(ii); + razor_set_unref(system); free(description); @@ -1443,6 +1445,8 @@ command_install_or_update(int argc, char * const argv[], int do_update) if (upstream == NULL) { fprintf(stderr, "%s\n", razor_error_get_msg(error)); razor_error_free(error); + if (relocations) + razor_relocations_destroy(relocations); return 1; } -- 1.7.1