dilim/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2026-06-30 19:57:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

journal-remote: fix hostname double-free on request_meta() error paths

Browse Source 
request_handler() owns the hostname var and passes it by value to
request_meta(), which hands it to source_new(), which stores it in
source->importer.name without copying. If build_accept_encoding()
then fails, the hostname var is freed, and then the caller's
_cleanup_free_ frees it a second time.

Follow-up for 9ff48d0982
This commit is contained in:
Luca Boccassi
2026-06-24 13:41:06 +01:00
parent f8891704d3
commit 94d49182a0
3 changed files with 31 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/journal-remote/journal-remote-main.c
View File

@@ -221,21 +221,23 @@ static int build_accept_encoding(char **ret) {
return 0;
}
static int request_meta(void **connection_cls, int fd, char *hostname) {
static int request_meta(void **connection_cls, int fd, char *_hostname) {
int r;
assert(connection_cls);
/* This takes ownership of the hostname in all cases, including on failure. */
_cleanup_free_ char *hostname = TAKE_PTR(_hostname);
if (*connection_cls)
return 0; /* already assigned. */
Writer *writer;
r = journal_remote_get_writer(journal_remote_server_global, hostname, &writer);
if (r < 0)
return log_warning_errno(r, "Failed to get writer for source %s: %m",
hostname);
return log_warning_errno(r, "Failed to get writer for source %s: %m", hostname);
_cleanup_(source_freep) RemoteSource *source = source_new(fd, true, hostname, writer);
_cleanup_(source_freep) RemoteSource *source = source_new(fd, true, TAKE_PTR(hostname), writer);
if (!source)
return log_oom();
@@ -445,13 +447,12 @@ static mhd_result request_handler(
assert(hostname);
r = request_meta(connection_cls, fd, hostname);
r = request_meta(connection_cls, fd, TAKE_PTR(hostname));
if (r == -ENOMEM)
return respond_oom(connection);
else if (r < 0)
return mhd_respondf(connection, r, MHD_HTTP_INTERNAL_SERVER_ERROR, "%m");
hostname = NULL;
return MHD_YES;
}

7
src/journal-remote/journal-remote-parse.c
View File

@@ -24,7 +24,8 @@ RemoteSource* source_free(RemoteSource *source) {
/**
* Initialize zero-filled source with given values. On success, takes
* ownership of fd, name, and writer, otherwise does not touch them.
* ownership of fd and writer, otherwise does not touch them. Always takes
* ownership of name, even on failure.
*/
RemoteSource* source_new(int fd, bool passive_fd, char *name, Writer *writer) {
RemoteSource *source;
@@ -35,8 +36,10 @@ RemoteSource* source_new(int fd, bool passive_fd, char *name, Writer *writer) {
assert(fd >= 0);
source = new0(RemoteSource, 1);
if (!source)
if (!source) {
free(name);
return NULL;
}
source->importer = JOURNAL_IMPORTER_MAKE(fd);
source->importer.passive_fd = passive_fd;

38
src/journal-remote/journal-remote.c
View File

@@ -162,11 +162,12 @@ static int dispatch_raw_connection_event(sd_event_source *event,
void *userdata);
static int get_source_for_fd(RemoteServer *s,
int fd, char *name, RemoteSource **source) {
int fd, char *_name, RemoteSource **source) {
_cleanup_free_ char *name = TAKE_PTR(_name);
Writer *writer;
int r;
/* This takes ownership of name, but only on success. */
/* This takes ownership of the name in all cases, including on failure. */
assert(s);
assert(fd >= 0);
@@ -177,11 +178,10 @@ static int get_source_for_fd(RemoteServer *s,
r = journal_remote_get_writer(s, name, &writer);
if (r < 0)
return log_warning_errno(r, "Failed to get writer for source %s: %m",
name);
return log_warning_errno(r, "Failed to get writer for source %s: %m", name);
if (!s->sources[fd]) {
s->sources[fd] = source_new(fd, false, name, writer);
s->sources[fd] = source_new(fd, false, TAKE_PTR(name), writer);
if (!s->sources[fd]) {
writer_unref(writer);
return log_oom();
@@ -211,29 +211,29 @@ static int remove_source(RemoteServer *s, int fd) {
return 0;
}
int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name) {
int journal_remote_add_source(RemoteServer *s, int fd, char *_name, bool own_name) {
_cleanup_free_ char *name = NULL;
RemoteSource *source = NULL;
int r;
/* This takes ownership of name, even on failure, if own_name is true. */
/* This takes ownership of _name, even on failure, if own_name is true. */
assert(s);
assert(fd >= 0);
assert(name);
assert(_name);
if (!own_name) {
name = strdup(name);
if (own_name)
name = TAKE_PTR(_name);
else {
name = strdup(_name);
if (!name)
return log_oom();
}
r = get_source_for_fd(s, fd, name, &source);
if (r < 0) {
log_error_errno(r, "Failed to create source for fd:%d (%s): %m",
fd, name);
free(name);
return r;
}
/* get_source_for_fd() takes ownership of the name in all cases, so it must not be touched below. */
r = get_source_for_fd(s, fd, TAKE_PTR(name), &source);
if (r < 0)
return log_error_errno(r, "Failed to create source for fd:%d: %m", fd);
r = sd_event_add_io(s->event, &source->event,
fd, EPOLLIN|EPOLLRDHUP|EPOLLPRI,
@@ -246,7 +246,7 @@ int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name
if (r == 0)
r = sd_event_source_set_enabled(source->buffer_event, SD_EVENT_OFF);
} else if (r == -EPERM) {
log_debug("Falling back to sd_event_add_defer for fd:%d (%s)", fd, name);
log_debug("Falling back to sd_event_add_defer for fd:%d (%s)", fd, source->importer.name);
r = sd_event_add_defer(s->event, &source->event,
dispatch_blocking_source_event, source);
if (r == 0)
@@ -258,7 +258,7 @@ int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name
goto error;
}
r = sd_event_source_set_description(source->event, name);
r = sd_event_source_set_description(source->event, source->importer.name);
if (r < 0) {
log_error_errno(r, "Failed to set source name for fd:%d: %m", fd);
goto error;