Skip to content

/ systemd

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Journal/journal-remote/coredump fixes #11374

Merged
merged 11 commits into from Jan 10, 2019
+170 −173
Merged

Journal/journal-remote/coredump fixes #11374

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4f62556
coredump: remove duplicate MESSAGE= prefix from message
keszybz Dec 5, 2018
bc2762a
journald: remove unnecessary {}
keszybz Dec 5, 2018
084eeb8
journald: do not store the iovec entry for process commandline on stack
keszybz Dec 5, 2018
2d5d2e0
basic/process-util: limit command line lengths to _SC_ARG_MAX
keszybz Dec 5, 2018
f0136e0
coredump: fix message when we fail to save a journald coredump
keszybz Dec 5, 2018
052c57f
journald: set a limit on the number of fields (1k)
keszybz Dec 5, 2018
964ef92
journald: when processing a native message, bail more quickly on over…
keszybz Dec 5, 2018
6670c9d
journald: lower the maximum entry size limit to ½ for non-sealed fds
keszybz Dec 5, 2018
d101fb2
µhttpd: use a cleanup function to call MHD_destroy_response
keszybz Dec 7, 2018
7fdb237
journal-remote: verify entry length from header
keszybz Dec 7, 2018
ef4d6ab
journal-remote: set a limit on the number of fields in a message
keszybz Dec 7, 2018
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

10 src/basic/io-util.c
View file
Open in desktop
@@ -8,6 +8,7 @@
#include <unistd.h>

#include "io-util.h"
#include "string-util.h"
#include "time-util.h"

int flush_fd(int fd) {
@@ -252,3 +253,12 @@ ssize_t sparse_write(int fd, const void *p, size_t sz, size_t run_length) {

return q - (const uint8_t*) p;
}

char* set_iovec_string_field(struct iovec *iovec, size_t *n_iovec, const char *field, const char *value) {
char *x;

x = strappend(field, value);
if (x)
iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(x);
return x;
}
2 src/basic/io-util.h
View file
Open in desktop
@@ -71,3 +71,5 @@ static inline bool FILE_SIZE_VALID_OR_INFINITY(uint64_t l) {
#define IOVEC_MAKE(base, len) (struct iovec) IOVEC_INIT(base, len)
#define IOVEC_INIT_STRING(string) IOVEC_INIT((char*) string, strlen(string))
#define IOVEC_MAKE_STRING(string) (struct iovec) IOVEC_INIT_STRING(string)

char* set_iovec_string_field(struct iovec *iovec, size_t *n_iovec, const char *field, const char *value);
73 src/basic/process-util.c
View file
Open in desktop
@@ -129,6 +129,13 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char *

(void) __fsetlocking(f, FSETLOCKING_BYCALLER);

if (max_length == 0) {
/* This is supposed to be a safety guard against runaway command lines. */
long l = sysconf(_SC_ARG_MAX);
assert(l > 0);

This comment has been minimized.

Sign in to view
@disconnect3d

disconnect3d Jan 10, 2019 •
edited

Note that the assert will probably be removed in production builds so if sysconf(_SC_ARG_MAX) would return -1 (in case the symbolic constant doesn't exist) the max_length would probably end up having a maximum possible size_t value.

It is probably not good to assume that the particular sysconf never fails on any machine, so maybe this should be changed to an if?

This comment has been minimized.

Sign in to view
@keszybz

keszybz Jan 10, 2019
Author Member

sysconf should not return -1. _SC_ARG_MAX is obviously a known constant. This could only happen if somebody tries to run this with a different libc than the common ones. In that case, I hope they compile at least once with asserts enabled and run the tests ;)

This comment has been minimized.

Sign in to view
@disconnect3d

disconnect3d Jan 10, 2019 •
edited

Yeah, I totally get it. Still it would be a good practice to move this kind of "expected invariants" e.g. to be fetched just once during program startup and checked against their expected values.
max_length = l;
}

if (max_length == 1) {

/* If there's only room for one byte, return the empty string */
@@ -139,32 +146,6 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char *
*line = ans;
return 0;

} else if (max_length == 0) {
size_t len = 0, allocated = 0;

while ((c = getc(f)) != EOF) {

if (!GREEDY_REALLOC(ans, allocated, len+3)) {
free(ans);
return -ENOMEM;
}

if (isprint(c)) {
if (space) {
ans[len++] = ' ';
space = false;
}

ans[len++] = c;
} else if (len > 0)
space = true;
}

if (len > 0)
ans[len] = '\0';
else
ans = mfree(ans);

} else {
bool dotdotdot = false;
size_t left;
@@ -236,34 +217,30 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char *
if (h < 0)
return h;

if (max_length == 0)
ans = strjoin("[", t, "]");
else {
size_t l;
size_t l = strlen(t);

l = strlen(t);

if (l + 3 <= max_length)
ans = strjoin("[", t, "]");
else if (max_length <= 6) {
if (l + 3 <= max_length) {
ans = strjoin("[", t, "]");
if (!ans)
return -ENOMEM;

ans = new(char, max_length);
if (!ans)
return -ENOMEM;
} else if (max_length <= 6) {
ans = new(char, max_length);
if (!ans)
return -ENOMEM;

memcpy(ans, "[...]", max_length-1);
ans[max_length-1] = 0;
} else {
t[max_length - 6] = 0;
memcpy(ans, "[...]", max_length-1);
ans[max_length-1] = 0;
} else {
t[max_length - 6] = 0;

/* Chop off final spaces */
delete_trailing_chars(t, WHITESPACE);
/* Chop off final spaces */
delete_trailing_chars(t, WHITESPACE);

ans = strjoin("[", t, "...]");
}
ans = strjoin("[", t, "...]");
if (!ans)
return -ENOMEM;
}
if (!ans)
return -ENOMEM;
}

*line = ans;
40 src/coredump/coredump.c
View file
Open in desktop
@@ -794,15 +794,16 @@ static int submit_coredump(
core_message = strjoin("MESSAGE=Process ", context[CONTEXT_PID],
" (", context[CONTEXT_COMM], ") of user ",
context[CONTEXT_UID], " dumped core.",
journald_crash ? "\nCoredump diverted to " : NULL,
journald_crash ? filename : NULL);
journald_crash && filename ? "\nCoredump diverted to " : NULL,
journald_crash && filename ? filename : NULL);
if (!core_message)
return log_oom();

if (journald_crash) {
/* We cannot log to the journal, so just print the MESSAGE.
/* We cannot log to the journal, so just print the message.
* The target was set previously to something safe. */
log_dispatch(LOG_ERR, 0, core_message);
assert(startswith(core_message, "MESSAGE="));
log_dispatch(LOG_ERR, 0, core_message + strlen("MESSAGE="));
return 0;
}

@@ -1062,19 +1063,10 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd)
return 0;
}

static char* set_iovec_field(struct iovec *iovec, size_t *n_iovec, const char *field, const char *value) {
char *x;

x = strappend(field, value);
if (x)
iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(x);
return x;
}

static char* set_iovec_field_free(struct iovec *iovec, size_t *n_iovec, const char *field, char *value) {
char *x;

x = set_iovec_field(iovec, n_iovec, field, value);
x = set_iovec_string_field(iovec, n_iovec, field, value);
free(value);
return x;
}
@@ -1124,36 +1116,36 @@ static int gather_pid_metadata(
disable_coredumps();
}

set_iovec_field(iovec, n_iovec, "COREDUMP_UNIT=", context[CONTEXT_UNIT]);
set_iovec_string_field(iovec, n_iovec, "COREDUMP_UNIT=", context[CONTEXT_UNIT]);
}

if (cg_pid_get_user_unit(pid, &t) >= 0)
set_iovec_field_free(iovec, n_iovec, "COREDUMP_USER_UNIT=", t);

/* The next few are mandatory */
if (!set_iovec_field(iovec, n_iovec, "COREDUMP_PID=", context[CONTEXT_PID]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_PID=", context[CONTEXT_PID]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_UID=", context[CONTEXT_UID]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_UID=", context[CONTEXT_UID]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_GID=", context[CONTEXT_GID]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_GID=", context[CONTEXT_GID]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_SIGNAL=", context[CONTEXT_SIGNAL]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL=", context[CONTEXT_SIGNAL]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_HOSTNAME=", context[CONTEXT_HOSTNAME]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_HOSTNAME=", context[CONTEXT_HOSTNAME]))
return log_oom();

if (!set_iovec_field(iovec, n_iovec, "COREDUMP_COMM=", context[CONTEXT_COMM]))
if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_COMM=", context[CONTEXT_COMM]))
return log_oom();

if (context[CONTEXT_EXE] &&
!set_iovec_field(iovec, n_iovec, "COREDUMP_EXE=", context[CONTEXT_EXE]))
!set_iovec_string_field(iovec, n_iovec, "COREDUMP_EXE=", context[CONTEXT_EXE]))
return log_oom();

if (sd_pid_get_session(pid, &t) >= 0)
@@ -1221,7 +1213,7 @@ static int gather_pid_metadata(
iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(t);

if (safe_atoi(context[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo))
set_iovec_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo));
set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo));

return 0; /* we successfully acquired all metadata */
}
53 src/journal-remote/journal-gatewayd.c
View file
Open in desktop
@@ -461,7 +461,7 @@ static int request_handler_entries(
struct MHD_Connection *connection,
void *connection_cls) {

struct MHD_Response *response;
_cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;
RequestMeta *m = connection_cls;
int r;

@@ -503,11 +503,7 @@ static int request_handler_entries(
return respond_oom(connection);

MHD_add_response_header(response, "Content-Type", mime_types[m->mode]);

r = MHD_queue_response(connection, MHD_HTTP_OK, response);
MHD_destroy_response(response);

return r;
return MHD_queue_response(connection, MHD_HTTP_OK, response);
}

static int output_field(FILE *f, OutputMode m, const char *d, size_t l) {
@@ -619,7 +615,7 @@ static int request_handler_fields(
const char *field,
void *connection_cls) {

struct MHD_Response *response;
_cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;
RequestMeta *m = connection_cls;
int r;

@@ -642,20 +638,15 @@ static int request_handler_fields(
return respond_oom(connection);

MHD_add_response_header(response, "Content-Type", mime_types[m->mode == OUTPUT_JSON ? OUTPUT_JSON : OUTPUT_SHORT]);

r = MHD_queue_response(connection, MHD_HTTP_OK, response);
MHD_destroy_response(response);

return r;
return MHD_queue_response(connection, MHD_HTTP_OK, response);
}

static int request_handler_redirect(
struct MHD_Connection *connection,
const char *target) {

char *page;
struct MHD_Response *response;
int ret;
_cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;

assert(connection);
assert(target);
@@ -671,20 +662,15 @@ static int request_handler_redirect(

MHD_add_response_header(response, "Content-Type", "text/html");
MHD_add_response_header(response, "Location", target);

ret = MHD_queue_response(connection, MHD_HTTP_MOVED_PERMANENTLY, response);
MHD_destroy_response(response);

return ret;
return MHD_queue_response(connection, MHD_HTTP_MOVED_PERMANENTLY, response);
}

static int request_handler_file(
struct MHD_Connection *connection,
const char *path,
const char *mime_type) {

struct MHD_Response *response;
int ret;
_cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;
_cleanup_close_ int fd = -1;
struct stat st;

@@ -702,15 +688,10 @@ static int request_handler_file(
response = MHD_create_response_from_fd_at_offset64(st.st_size, fd, 0);
if (!response)
return respond_oom(connection);

fd = -1;
TAKE_FD(fd);

MHD_add_response_header(response, "Content-Type", mime_type);

ret = MHD_queue_response(connection, MHD_HTTP_OK, response);
MHD_destroy_response(response);

return ret;
return MHD_queue_response(connection, MHD_HTTP_OK, response);
}

static int get_virtualization(char **v) {
@@ -747,14 +728,13 @@ static int request_handler_machine(
struct MHD_Connection *connection,
void *connection_cls) {

struct MHD_Response *response;
_cleanup_(MHD_destroy_responsep) struct MHD_Response *response = NULL;
RequestMeta *m = connection_cls;
int r;
_cleanup_free_ char* hostname = NULL, *os_name = NULL;
uint64_t cutoff_from = 0, cutoff_to = 0, usage = 0;
char *json;
sd_id128_t mid, bid;
_cleanup_free_ char *v = NULL;
_cleanup_free_ char *v = NULL, *json = NULL;

assert(connection);
assert(m);
@@ -803,21 +783,16 @@ static int request_handler_machine(
usage,
cutoff_from,
cutoff_to);

if (r < 0)
return respond_oom(connection);

response = MHD_create_response_from_buffer(strlen(json), json, MHD_RESPMEM_MUST_FREE);
if (!response) {
free(json);
if (!response)
return respond_oom(connection);
}
TAKE_PTR(json);

MHD_add_response_header(response, "Content-Type", "application/json");
r = MHD_queue_response(connection, MHD_HTTP_OK, response);
MHD_destroy_response(response);

return r;
return MHD_queue_response(connection, MHD_HTTP_OK, response);
}

static int request_handler(
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.