Introduce _cleanup_(strbuf_cleanupp) and use it to fix null deref on error

catalog_update() would call strbuf_cleanup(NULL) on allocation error.
CID #1390928.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-05-10 01:16:03 +02:00
parent db983479af
commit f201daec89
4 changed files with 24 additions and 39 deletions

View file

@ -11,6 +11,8 @@
#include <stdint.h>
#include <sys/types.h>
#include "macro.h"
struct strbuf {
char *buf;
size_t len;
@ -40,3 +42,4 @@ struct strbuf *strbuf_new(void);
ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len);
void strbuf_complete(struct strbuf *str);
void strbuf_cleanup(struct strbuf *str);
DEFINE_TRIVIAL_CLEANUP_FUNC(struct strbuf*, strbuf_cleanup);

View file

@ -448,7 +448,7 @@ error:
int catalog_update(const char* database, const char* root, const char* const* dirs) {
_cleanup_strv_free_ char **files = NULL;
char **f;
struct strbuf *sb = NULL;
_cleanup_(strbuf_cleanupp) struct strbuf *sb = NULL;
_cleanup_hashmap_free_free_free_ Hashmap *h = NULL;
_cleanup_free_ CatalogItem *items = NULL;
ssize_t offset;
@ -461,38 +461,29 @@ int catalog_update(const char* database, const char* root, const char* const* di
h = hashmap_new(&catalog_hash_ops);
sb = strbuf_new();
if (!h || !sb) {
r = log_oom();
goto finish;
}
if (!h || !sb)
return log_oom();
r = conf_files_list_strv(&files, ".catalog", root, 0, dirs);
if (r < 0) {
log_error_errno(r, "Failed to get catalog files: %m");
goto finish;
}
if (r < 0)
return log_error_errno(r, "Failed to get catalog files: %m");
STRV_FOREACH(f, files) {
log_debug("Reading file '%s'", *f);
r = catalog_import_file(h, *f);
if (r < 0) {
log_error_errno(r, "Failed to import file '%s': %m", *f);
goto finish;
}
if (r < 0)
return log_error_errno(r, "Failed to import file '%s': %m", *f);
}
if (hashmap_size(h) <= 0) {
log_info("No items in catalog.");
goto finish;
return 0;
} else
log_debug("Found %u items in catalog.", hashmap_size(h));
items = new(CatalogItem, hashmap_size(h));
if (!items) {
r = log_oom();
goto finish;
}
if (!items)
return log_oom();
n = 0;
HASHMAP_FOREACH_KEY(payload, i, h, j) {
@ -501,10 +492,9 @@ int catalog_update(const char* database, const char* root, const char* const* di
isempty(i->language) ? "C" : i->language);
offset = strbuf_add_string(sb, payload, strlen(payload));
if (offset < 0) {
r = log_oom();
goto finish;
}
if (offset < 0)
return log_oom();
i->offset = htole64((uint64_t) offset);
items[n++] = *i;
}
@ -516,17 +506,11 @@ int catalog_update(const char* database, const char* root, const char* const* di
sz = write_catalog(database, sb, items, n);
if (sz < 0)
r = log_error_errno(sz, "Failed to write %s: %m", database);
else {
r = 0;
log_debug("%s: wrote %u items, with %zu bytes of strings, %"PRIi64" total size.",
database, n, sb->len, sz);
}
return log_error_errno(sz, "Failed to write %s: %m", database);
finish:
strbuf_cleanup(sb);
return r;
log_debug("%s: wrote %u items, with %zu bytes of strings, %"PRIi64" total size.",
database, n, sb->len, sz);
return 0;
}
static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p) {

View file

@ -177,16 +177,16 @@ static void test_catalog_update(void) {
/* Test what happens if there are no files. */
r = catalog_update(database, NULL, NULL);
assert_se(r >= 0);
assert_se(r == 0);
/* Test what happens if there are no files in the directory. */
r = catalog_update(database, NULL, no_catalog_dirs);
assert_se(r >= 0);
assert_se(r == 0);
/* Make sure that we at least have some files loaded or the
catalog_list below will fail. */
r = catalog_update(database, NULL, catalog_dirs);
assert_se(r >= 0);
assert_se(r == 0);
}
static void test_catalog_file_lang(void) {

View file

@ -18,7 +18,7 @@ static ssize_t add_string(struct strbuf *sb, const char *s) {
}
static void test_strbuf(void) {
struct strbuf *sb;
_cleanup_(strbuf_cleanupp) struct strbuf *sb;
_cleanup_strv_free_ char **l;
ssize_t a, b, c, d, e, f, g, h;
@ -72,8 +72,6 @@ static void test_strbuf(void) {
strbuf_complete(sb);
assert_se(sb->root == NULL);
strbuf_cleanup(sb);
}
int main(int argc, const char *argv[]) {