322 lines
11 KiB
Diff
322 lines
11 KiB
Diff
From 9b9103a07cc32fa76be4c71fcd9a93b5d946edd4 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
|
Date: Sat, 28 Dec 2013 19:33:23 -0500
|
|
Subject: [PATCH] journal: fix access to munmapped memory in
|
|
sd_journal_enumerate_unique
|
|
|
|
sd_j_e_u needs to keep a reference to an object while comparing it
|
|
with possibly duplicate objects in other files. Because the size of
|
|
mmap cache is limited, with enough files and object to compare to,
|
|
at some point the object being compared would be munmapped, resulting
|
|
in a segmentation fault.
|
|
|
|
Fix this issue by turning keep_always into a reference count that can
|
|
be increased and decreased. Other callers which set keep_always=true
|
|
are unmodified: their references are never released but are ignored
|
|
when the whole file is closed, which happens at some point. keep_always
|
|
is increased in sd_j_e_u and later on released.
|
|
---
|
|
src/journal/journal-file.c | 5 +---
|
|
src/journal/journal-file.h | 24 +++++++++++++++++++
|
|
src/journal/journal-verify.c | 4 ----
|
|
src/journal/mmap-cache.c | 57 +++++++++++++++++++++++++++++++++++---------
|
|
src/journal/mmap-cache.h | 18 +++++++++++++-
|
|
src/journal/sd-journal.c | 18 +++++++++++---
|
|
6 files changed, 103 insertions(+), 23 deletions(-)
|
|
|
|
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
|
|
index ba65464..4d9787a 100644
|
|
--- a/src/journal/journal-file.c
|
|
+++ b/src/journal/journal-file.c
|
|
@@ -419,7 +419,6 @@ int journal_file_move_to_object(JournalFile *f, int type, uint64_t offset, Objec
|
|
void *t;
|
|
Object *o;
|
|
uint64_t s;
|
|
- unsigned context;
|
|
|
|
assert(f);
|
|
assert(ret);
|
|
@@ -428,10 +427,8 @@ int journal_file_move_to_object(JournalFile *f, int type, uint64_t offset, Objec
|
|
if (!VALID64(offset))
|
|
return -EFAULT;
|
|
|
|
- /* One context for each type, plus one catch-all for the rest */
|
|
- context = type > 0 && type < _OBJECT_TYPE_MAX ? type : 0;
|
|
|
|
- r = journal_file_move_to(f, context, false, offset, sizeof(ObjectHeader), &t);
|
|
+ r = journal_file_move_to(f, type_to_context(type), false, offset, sizeof(ObjectHeader), &t);
|
|
if (r < 0)
|
|
return r;
|
|
|
|
diff --git a/src/journal/journal-file.h b/src/journal/journal-file.h
|
|
index 50bdb67..0bd23f7 100644
|
|
--- a/src/journal/journal-file.h
|
|
+++ b/src/journal/journal-file.h
|
|
@@ -128,6 +128,10 @@ int journal_file_open_reliably(
|
|
#define ALIGN64(x) (((x) + 7ULL) & ~7ULL)
|
|
#define VALID64(x) (((x) & 7ULL) == 0ULL)
|
|
|
|
+/* Use six characters to cover the offsets common in smallish journal
|
|
+ * files without adding too many zeros. */
|
|
+#define OFSfmt "%06"PRIx64
|
|
+
|
|
static inline bool VALID_REALTIME(uint64_t u) {
|
|
/* This considers timestamps until the year 3112 valid. That should be plenty room... */
|
|
return u > 0 && u < (1ULL << 55);
|
|
@@ -197,3 +201,23 @@ int journal_file_get_cutoff_realtime_usec(JournalFile *f, usec_t *from, usec_t *
|
|
int journal_file_get_cutoff_monotonic_usec(JournalFile *f, sd_id128_t boot, usec_t *from, usec_t *to);
|
|
|
|
bool journal_file_rotate_suggested(JournalFile *f, usec_t max_file_usec);
|
|
+
|
|
+
|
|
+static unsigned type_to_context(int type) {
|
|
+ /* One context for each type, plus one catch-all for the rest */
|
|
+ return type > 0 && type < _OBJECT_TYPE_MAX ? type : 0;
|
|
+}
|
|
+
|
|
+static inline int journal_file_object_keep(JournalFile *f, Object *o, uint64_t offset) {
|
|
+ unsigned context = type_to_context(o->object.type);
|
|
+
|
|
+ return mmap_cache_get(f->mmap, f->fd, f->prot, context, true,
|
|
+ offset, o->object.size, &f->last_stat, NULL);
|
|
+}
|
|
+
|
|
+static inline int journal_file_object_release(JournalFile *f, Object *o, uint64_t offset) {
|
|
+ unsigned context = type_to_context(o->object.type);
|
|
+
|
|
+ return mmap_cache_release(f->mmap, f->fd, f->prot, context,
|
|
+ offset, o->object.size);
|
|
+}
|
|
diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c
|
|
index 82b0f0a..f2422ff 100644
|
|
--- a/src/journal/journal-verify.c
|
|
+++ b/src/journal/journal-verify.c
|
|
@@ -34,10 +34,6 @@
|
|
#include "compress.h"
|
|
#include "fsprg.h"
|
|
|
|
-/* Use six characters to cover the offsets common in smallish journal
|
|
- * files without adding to many zeros. */
|
|
-#define OFSfmt "%06"PRIx64
|
|
-
|
|
static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o) {
|
|
uint64_t i;
|
|
|
|
diff --git a/src/journal/mmap-cache.c b/src/journal/mmap-cache.c
|
|
index 42a8a7d..24b2bb8 100644
|
|
--- a/src/journal/mmap-cache.c
|
|
+++ b/src/journal/mmap-cache.c
|
|
@@ -38,7 +38,7 @@ typedef struct FileDescriptor FileDescriptor;
|
|
struct Window {
|
|
MMapCache *cache;
|
|
|
|
- bool keep_always;
|
|
+ unsigned keep_always;
|
|
bool in_unused;
|
|
|
|
int prot;
|
|
@@ -185,7 +185,7 @@ static void context_detach_window(Context *c) {
|
|
c->window = NULL;
|
|
LIST_REMOVE(Context, by_window, w->contexts, c);
|
|
|
|
- if (!w->contexts && !w->keep_always) {
|
|
+ if (!w->contexts && w->keep_always == 0) {
|
|
/* Not used anymore? */
|
|
LIST_PREPEND(Window, unused, c->cache->unused, w);
|
|
if (!c->cache->last_unused)
|
|
@@ -360,7 +360,6 @@ static int try_context(
|
|
assert(m->n_ref > 0);
|
|
assert(fd >= 0);
|
|
assert(size > 0);
|
|
- assert(ret);
|
|
|
|
c = hashmap_get(m->contexts, UINT_TO_PTR(context+1));
|
|
if (!c)
|
|
@@ -378,9 +377,10 @@ static int try_context(
|
|
return 0;
|
|
}
|
|
|
|
- c->window->keep_always = c->window->keep_always || keep_always;
|
|
+ c->window->keep_always += keep_always;
|
|
|
|
- *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset);
|
|
+ if (ret)
|
|
+ *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset);
|
|
return 1;
|
|
}
|
|
|
|
@@ -402,7 +402,6 @@ static int find_mmap(
|
|
assert(m->n_ref > 0);
|
|
assert(fd >= 0);
|
|
assert(size > 0);
|
|
- assert(ret);
|
|
|
|
f = hashmap_get(m->fds, INT_TO_PTR(fd + 1));
|
|
if (!f)
|
|
@@ -422,9 +421,10 @@ static int find_mmap(
|
|
return -ENOMEM;
|
|
|
|
context_attach_window(c, w);
|
|
- w->keep_always = w->keep_always || keep_always;
|
|
+ w->keep_always += keep_always;
|
|
|
|
- *ret = (uint8_t*) w->ptr + (offset - w->offset);
|
|
+ if (ret)
|
|
+ *ret = (uint8_t*) w->ptr + (offset - w->offset);
|
|
return 1;
|
|
}
|
|
|
|
@@ -450,7 +450,6 @@ static int add_mmap(
|
|
assert(m->n_ref > 0);
|
|
assert(fd >= 0);
|
|
assert(size > 0);
|
|
- assert(ret);
|
|
|
|
woffset = offset & ~((uint64_t) page_size() - 1ULL);
|
|
wsize = size + (offset - woffset);
|
|
@@ -520,7 +519,8 @@ static int add_mmap(
|
|
c->window = w;
|
|
LIST_PREPEND(Context, by_window, w->contexts, c);
|
|
|
|
- *ret = (uint8_t*) w->ptr + (offset - w->offset);
|
|
+ if (ret)
|
|
+ *ret = (uint8_t*) w->ptr + (offset - w->offset);
|
|
return 1;
|
|
}
|
|
|
|
@@ -541,7 +541,6 @@ int mmap_cache_get(
|
|
assert(m->n_ref > 0);
|
|
assert(fd >= 0);
|
|
assert(size > 0);
|
|
- assert(ret);
|
|
|
|
/* Check whether the current context is the right one already */
|
|
r = try_context(m, fd, prot, context, keep_always, offset, size, ret);
|
|
@@ -563,6 +562,42 @@ int mmap_cache_get(
|
|
return add_mmap(m, fd, prot, context, keep_always, offset, size, st, ret);
|
|
}
|
|
|
|
+int mmap_cache_release(
|
|
+ MMapCache *m,
|
|
+ int fd,
|
|
+ int prot,
|
|
+ unsigned context,
|
|
+ uint64_t offset,
|
|
+ size_t size) {
|
|
+
|
|
+ FileDescriptor *f;
|
|
+ Window *w;
|
|
+
|
|
+ assert(m);
|
|
+ assert(m->n_ref > 0);
|
|
+ assert(fd >= 0);
|
|
+ assert(size > 0);
|
|
+
|
|
+ f = hashmap_get(m->fds, INT_TO_PTR(fd + 1));
|
|
+ if (!f)
|
|
+ return -EBADF;
|
|
+
|
|
+ assert(f->fd == fd);
|
|
+
|
|
+ LIST_FOREACH(by_fd, w, f->windows)
|
|
+ if (window_matches(w, fd, prot, offset, size))
|
|
+ break;
|
|
+
|
|
+ if (!w)
|
|
+ return -ENOENT;
|
|
+
|
|
+ if (w->keep_always == 0)
|
|
+ return -ENOLCK;
|
|
+
|
|
+ w->keep_always -= 1;
|
|
+ return 0;
|
|
+}
|
|
+
|
|
void mmap_cache_close_fd(MMapCache *m, int fd) {
|
|
FileDescriptor *f;
|
|
|
|
diff --git a/src/journal/mmap-cache.h b/src/journal/mmap-cache.h
|
|
index 912336d..647555a 100644
|
|
--- a/src/journal/mmap-cache.h
|
|
+++ b/src/journal/mmap-cache.h
|
|
@@ -31,7 +31,23 @@ MMapCache* mmap_cache_new(void);
|
|
MMapCache* mmap_cache_ref(MMapCache *m);
|
|
MMapCache* mmap_cache_unref(MMapCache *m);
|
|
|
|
-int mmap_cache_get(MMapCache *m, int fd, int prot, unsigned context, bool keep_always, uint64_t offset, size_t size, struct stat *st, void **ret);
|
|
+int mmap_cache_get(
|
|
+ MMapCache *m,
|
|
+ int fd,
|
|
+ int prot,
|
|
+ unsigned context,
|
|
+ bool keep_always,
|
|
+ uint64_t offset,
|
|
+ size_t size,
|
|
+ struct stat *st,
|
|
+ void **ret);
|
|
+int mmap_cache_release(
|
|
+ MMapCache *m,
|
|
+ int fd,
|
|
+ int prot,
|
|
+ unsigned context,
|
|
+ uint64_t offset,
|
|
+ size_t size);
|
|
void mmap_cache_close_fd(MMapCache *m, int fd);
|
|
void mmap_cache_close_context(MMapCache *m, unsigned context);
|
|
|
|
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
|
|
index 52abbe9..46c3feb 100644
|
|
--- a/src/journal/sd-journal.c
|
|
+++ b/src/journal/sd-journal.c
|
|
@@ -2508,9 +2508,7 @@ _public_ int sd_journal_query_unique(sd_journal *j, const char *field) {
|
|
}
|
|
|
|
_public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_t *l) {
|
|
- Object *o;
|
|
size_t k;
|
|
- int r;
|
|
|
|
if (!j)
|
|
return -EINVAL;
|
|
@@ -2535,9 +2533,11 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
|
|
for (;;) {
|
|
JournalFile *of;
|
|
Iterator i;
|
|
+ Object *o;
|
|
const void *odata;
|
|
size_t ol;
|
|
bool found;
|
|
+ int r;
|
|
|
|
/* Proceed to next data object in the field's linked list */
|
|
if (j->unique_offset == 0) {
|
|
@@ -2574,8 +2574,16 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
|
|
return r;
|
|
|
|
/* Let's do the type check by hand, since we used 0 context above. */
|
|
- if (o->object.type != OBJECT_DATA)
|
|
+ if (o->object.type != OBJECT_DATA) {
|
|
+ log_error("%s:offset " OFSfmt ": object has type %d, expected %d",
|
|
+ j->unique_file->path, j->unique_offset,
|
|
+ o->object.type, OBJECT_DATA);
|
|
return -EBADMSG;
|
|
+ }
|
|
+
|
|
+ r = journal_file_object_keep(j->unique_file, o, j->unique_offset);
|
|
+ if (r < 0)
|
|
+ return r;
|
|
|
|
r = return_data(j, j->unique_file, o, &odata, &ol);
|
|
if (r < 0)
|
|
@@ -2609,6 +2617,10 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
|
|
if (found)
|
|
continue;
|
|
|
|
+ r = journal_file_object_release(j->unique_file, o, j->unique_offset);
|
|
+ if (r < 0)
|
|
+ return r;
|
|
+
|
|
r = return_data(j, j->unique_file, o, data, l);
|
|
if (r < 0)
|
|
return r;
|