1
0
mirror of https://github.com/systemd/systemd.git synced 2024-11-02 19:21:53 +03:00

journal/catalog: fix memory leaks

Various buffers were lost because finish_item() either consumed
the buffer or allocated a new one (if an entry with the same key existed).
The caller would simply forget the buffer in either case.

Also add a check for the case when a valid identifier is followed by
an empty body. We should not allow this.

Also be more consistent in error handling and always print an error
message.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2016-02-18 17:37:17 -05:00
parent 82501b3fc4
commit 06466a7f03
2 changed files with 43 additions and 40 deletions

View File

@ -164,14 +164,14 @@ static int finish_item(
Hashmap *h, Hashmap *h,
sd_id128_t id, sd_id128_t id,
const char *language, const char *language,
char *payload) { char *payload, size_t payload_size) {
_cleanup_free_ CatalogItem *i = NULL; _cleanup_free_ CatalogItem *i = NULL;
_cleanup_free_ char *combined = NULL, *prev = NULL; _cleanup_free_ char *prev = NULL, *combined = NULL;
int r;
assert(h); assert(h);
assert(payload); assert(payload);
assert(payload_size > 0);
i = new0(CatalogItem, 1); i = new0(CatalogItem, 1);
if (!i) if (!i)
@ -184,23 +184,25 @@ static int finish_item(
} }
prev = hashmap_get(h, i); prev = hashmap_get(h, i);
/* Already have such an item, combine them */
if (prev) { if (prev) {
/* Already have such an item, combine them */
combined = combine_entries(payload, prev); combined = combine_entries(payload, prev);
if (!combined) if (!combined)
return log_oom(); return log_oom();
r = hashmap_update(h, i, combined);
if (r < 0)
return r;
combined = NULL;
/* A new item */ if (hashmap_update(h, i, combined) < 0)
return log_oom();
combined = NULL;
} else { } else {
r = hashmap_put(h, i, payload); /* A new item */
if (r < 0) combined = memdup(payload, payload_size + 1);
return r; if (!combined)
return log_oom();
if (hashmap_put(h, i, combined) < 0)
return log_oom();
i = NULL; i = NULL;
combined = NULL;
} }
return 0; return 0;
@ -262,6 +264,7 @@ static int catalog_entry_lang(const char* filename, int line,
int catalog_import_file(Hashmap *h, const char *path) { int catalog_import_file(Hashmap *h, const char *path) {
_cleanup_fclose_ FILE *f = NULL; _cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *payload = NULL; _cleanup_free_ char *payload = NULL;
size_t payload_size = 0, payload_allocated = 0;
unsigned n = 0; unsigned n = 0;
sd_id128_t id; sd_id128_t id;
_cleanup_free_ char *deflang = NULL, *lang = NULL; _cleanup_free_ char *deflang = NULL, *lang = NULL;
@ -283,8 +286,7 @@ int catalog_import_file(Hashmap *h, const char *path) {
for (;;) { for (;;) {
char line[LINE_MAX]; char line[LINE_MAX];
size_t a, b, c; size_t line_len;
char *t;
if (!fgets(line, sizeof(line), f)) { if (!fgets(line, sizeof(line), f)) {
if (feof(f)) if (feof(f))
@ -323,17 +325,23 @@ int catalog_import_file(Hashmap *h, const char *path) {
if (sd_id128_from_string(line + 2 + 1, &jd) >= 0) { if (sd_id128_from_string(line + 2 + 1, &jd) >= 0) {
if (got_id) { if (got_id) {
r = finish_item(h, id, lang ?: deflang, payload); if (payload_size == 0) {
log_error("[%s:%u] No payload text.", path, n);
return -EINVAL;
}
r = finish_item(h, id, lang ?: deflang, payload, payload_size);
if (r < 0) if (r < 0)
return r; return r;
payload = NULL;
lang = mfree(lang); lang = mfree(lang);
payload_size = 0;
} }
if (with_language) { if (with_language) {
t = strstrip(line + 2 + 1 + 32 + 1); char *t;
t = strstrip(line + 2 + 1 + 32 + 1);
r = catalog_entry_lang(path, n, t, deflang, &lang); r = catalog_entry_lang(path, n, t, deflang, &lang);
if (r < 0) if (r < 0)
return r; return r;
@ -343,9 +351,6 @@ int catalog_import_file(Hashmap *h, const char *path) {
empty_line = false; empty_line = false;
id = jd; id = jd;
if (payload)
payload[0] = '\0';
continue; continue;
} }
} }
@ -356,34 +361,30 @@ int catalog_import_file(Hashmap *h, const char *path) {
return -EINVAL; return -EINVAL;
} }
a = payload ? strlen(payload) : 0; line_len = strlen(line);
b = strlen(line); if (!GREEDY_REALLOC(payload, payload_allocated,
payload_size + (empty_line ? 1 : 0) + line_len + 1 + 1))
c = a + (empty_line ? 1 : 0) + b + 1 + 1;
t = realloc(payload, c);
if (!t)
return log_oom(); return log_oom();
if (empty_line) { if (empty_line)
t[a] = '\n'; payload[payload_size++] = '\n';
memcpy(t + a + 1, line, b); memcpy(payload + payload_size, line, line_len);
t[a+b+1] = '\n'; payload_size += line_len;
t[a+b+2] = 0; payload[payload_size++] = '\n';
} else { payload[payload_size] = '\0';
memcpy(t + a, line, b);
t[a+b] = '\n';
t[a+b+1] = 0;
}
payload = t;
empty_line = false; empty_line = false;
} }
if (got_id) { if (got_id) {
r = finish_item(h, id, lang ?: deflang, payload); if (payload_size == 0) {
log_error("[%s:%u] No payload text.", path, n);
return -EINVAL;
}
r = finish_item(h, id, lang ?: deflang, payload, payload_size);
if (r < 0) if (r < 0)
return r; return r;
payload = NULL;
} }
return 0; return 0;

View File

@ -103,6 +103,8 @@ static void test_catalog_import_one(void) {
assert_se(hashmap_size(h) == 1); assert_se(hashmap_size(h) == 1);
HASHMAP_FOREACH(payload, h, j) { HASHMAP_FOREACH(payload, h, j) {
printf("expect: %s\n", expect);
printf("actual: %s\n", payload);
assert_se(streq(expect, payload)); assert_se(streq(expect, payload));
} }
} }