picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

coredump: do not try to access unitialized CONTEXT_COMM field

Browse source 
Most of the fields in the context array come from the kernel (passed
through argv), but two are special: comm and exe. We allocate them
ourselves. We forgot to initialize context[CONTEXT_COMM] with the value
we allocated (introduced in 9aa8202314).
To simplify things, just set context[CONTEXT_COMM] and context[CONTEXT_EXE],
and free those two fields at the end.

Fixes #5442.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2017-02-25 14:00:39 -05:00
parent a6a73a10e8
commit ea5cc2a8f6
1 changed files with 53 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
src/coredump/coredump.c
View file

@ -78,8 +78,22 @@
assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX); assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX);
enum { enum {
/* We use this as array indexes for a couple of special fields we use for naming coredumping files, and /* We use this as array indexes for a couple of special fields we use for
* attaching xattrs */ * naming coredump files, and attaching xattrs, and for indexing argv[].
* Our pattern for man:systectl(1) kernel.core_pattern is such that the
* kernel passes fields until CONTEXT_RLIMIT as arguments in argv[]. After
* that it gets complicated: the kernel passes "comm" as one or more fields
* starting at index CONTEXT_COMM (in other words, full "comm" is under index
* CONTEXT_COMM when it does not contain spaces, which is the common
* case). This mapping is not reversible, so we prefer to retrieve "comm"
* from /proc. We only fall back to argv[CONTEXT_COMM...] when that fails.
*
* In the internal context[] array, fields before CONTEXT_COMM are the
* strings from argv[], so they should not be freed. The strings at indices
* CONTEXT_COMM and higher are allocated by us and should be freed at the
* end.
*/
CONTEXT_PID, CONTEXT_PID,
CONTEXT_UID, CONTEXT_UID,
CONTEXT_GID, CONTEXT_GID,
@ -784,7 +798,7 @@ log:
return 0; return 0;
} }
static void map_context_fields(const struct iovec *iovec, const char *context[]) { static void map_context_fields(const struct iovec *iovec, const char* context[]) {
static const char * const context_field_names[_CONTEXT_MAX] = { static const char * const context_field_names[_CONTEXT_MAX] = {
[CONTEXT_PID] = "COREDUMP_PID=", [CONTEXT_PID] = "COREDUMP_PID=",
@ -1054,9 +1068,8 @@ static char* set_iovec_field_free(struct iovec iovec[27], size_t *n_iovec, const
} }
static int gather_pid_metadata_and_process_special_crash( static int gather_pid_metadata_and_process_special_crash(
const char *context[_CONTEXT_MAX], char* context[_CONTEXT_MAX],
char **comm_fallback, char **comm_fallback,
char **comm_ret,
struct iovec *iovec, size_t *n_iovec) { struct iovec *iovec, size_t *n_iovec) {
/* We need 26 empty slots in iovec! /* We need 26 empty slots in iovec!
@ -1064,7 +1077,6 @@ static int gather_pid_metadata_and_process_special_crash(
* Note that if we fail on oom later on, we do not roll-back changes to the iovec structure. (It remains valid, * Note that if we fail on oom later on, we do not roll-back changes to the iovec structure. (It remains valid,
* with the first n_iovec fields initialized.) */ * with the first n_iovec fields initialized.) */
_cleanup_free_ char *exe = NULL, *comm = NULL;
uid_t owner_uid; uid_t owner_uid;
pid_t pid; pid_t pid;
char *t; char *t;
@ -1075,15 +1087,15 @@ static int gather_pid_metadata_and_process_special_crash(
if (r < 0) if (r < 0)
return log_error_errno(r, "Failed to parse PID \"%s\": %m", context[CONTEXT_PID]); return log_error_errno(r, "Failed to parse PID \"%s\": %m", context[CONTEXT_PID]);
r = get_process_comm(pid, &comm); r = get_process_comm(pid, &context[CONTEXT_COMM]);
if (r < 0) { if (r < 0) {
log_warning_errno(r, "Failed to get COMM, falling back to the command line: %m"); log_warning_errno(r, "Failed to get COMM, falling back to the command line: %m");
comm = strv_join(comm_fallback, " "); context[CONTEXT_COMM] = strv_join(comm_fallback, " ");
if (!comm) if (!context[CONTEXT_COMM])
return log_oom(); return log_oom();
} }
r = get_process_exe(pid, &exe); r = get_process_exe(pid, &context[CONTEXT_EXE]);
if (r < 0) if (r < 0)
log_warning_errno(r, "Failed to get EXE, ignoring: %m"); log_warning_errno(r, "Failed to get EXE, ignoring: %m");
@ -1099,7 +1111,7 @@ static int gather_pid_metadata_and_process_special_crash(
* are unlikely to work then. */ * are unlikely to work then. */
if (STR_IN_SET(t, SPECIAL_JOURNALD_SERVICE, SPECIAL_INIT_SCOPE)) { if (STR_IN_SET(t, SPECIAL_JOURNALD_SERVICE, SPECIAL_INIT_SCOPE)) {
free(t); free(t);
r = process_special_crash(context, STDIN_FILENO); r = process_special_crash((const char**) context, STDIN_FILENO);
if (r < 0) if (r < 0)
return r; return r;
@ -1132,11 +1144,11 @@ static int gather_pid_metadata_and_process_special_crash(
if (!set_iovec_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT])) if (!set_iovec_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT]))
return log_oom(); return log_oom();
if (!set_iovec_field(iovec, n_iovec, "COREDUMP_COMM=", comm)) if (!set_iovec_field(iovec, n_iovec, "COREDUMP_COMM=", context[CONTEXT_COMM]))
return log_oom(); return log_oom();
if (exe && if (context[CONTEXT_EXE] &&
!set_iovec_field(iovec, n_iovec, "COREDUMP_EXE=", exe)) !set_iovec_field(iovec, n_iovec, "COREDUMP_EXE=", context[CONTEXT_EXE]))
return log_oom(); return log_oom();
if (sd_pid_get_session(pid, &t) >= 0) if (sd_pid_get_session(pid, &t) >= 0)
@ -1206,17 +1218,12 @@ static int gather_pid_metadata_and_process_special_crash(
if (safe_atoi(context[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo)) 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_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo));
if (comm_ret) {
*comm_ret = comm;
comm = NULL;
}
return 0; /* == 0 means: we successfully acquired all metadata */ return 0; /* == 0 means: we successfully acquired all metadata */
} }
static int process_kernel(int argc, char* argv[]) { static int process_kernel(int argc, char* argv[]) {
const char *context[_CONTEXT_MAX]; char* context[_CONTEXT_MAX] = {};
struct iovec iovec[28]; struct iovec iovec[28];
size_t i, n_iovec, n_to_free = 0; size_t i, n_iovec, n_to_free = 0;
int r; int r;
@ -1228,14 +1235,14 @@ static int process_kernel(int argc, char* argv[]) {
return -EINVAL; return -EINVAL;
} }
context[CONTEXT_PID] = argv[CONTEXT_PID + 1]; context[CONTEXT_PID] = argv[1 + CONTEXT_PID];
context[CONTEXT_UID] = argv[CONTEXT_UID + 1]; context[CONTEXT_UID] = argv[1 + CONTEXT_UID];
context[CONTEXT_GID] = argv[CONTEXT_GID + 1]; context[CONTEXT_GID] = argv[1 + CONTEXT_GID];
context[CONTEXT_SIGNAL] = argv[CONTEXT_SIGNAL + 1]; context[CONTEXT_SIGNAL] = argv[1 + CONTEXT_SIGNAL];
context[CONTEXT_TIMESTAMP] = argv[CONTEXT_TIMESTAMP + 1]; context[CONTEXT_TIMESTAMP] = argv[1 + CONTEXT_TIMESTAMP];
context[CONTEXT_RLIMIT] = argv[CONTEXT_RLIMIT + 1]; context[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT];
r = gather_pid_metadata_and_process_special_crash(context, argv + CONTEXT_COMM + 1, NULL, iovec, &n_to_free); r = gather_pid_metadata_and_process_special_crash(context, argv + 1 + CONTEXT_COMM, iovec, &n_to_free);
if (r != 0) if (r != 0)
/* Error, or a a special crash, which has already been processed. */ /* Error, or a a special crash, which has already been processed. */
goto finish; goto finish;
@ -1255,12 +1262,16 @@ static int process_kernel(int argc, char* argv[]) {
for (i = 0; i < n_to_free; i++) for (i = 0; i < n_to_free; i++)
free(iovec[i].iov_base); free(iovec[i].iov_base);
/* Those fields are allocated by gather_pid_metadata_and_process_special_crash */
free(context[CONTEXT_COMM]);
free(context[CONTEXT_EXE]);
return r; return r;
} }
static int process_backtrace(int argc, char *argv[]) { static int process_backtrace(int argc, char *argv[]) {
const char *context[_CONTEXT_MAX]; char *context[_CONTEXT_MAX] = {};
_cleanup_free_ char *comm = NULL, *message = NULL; _cleanup_free_ char *message = NULL;
_cleanup_free_ struct iovec *iovec = NULL; _cleanup_free_ struct iovec *iovec = NULL;
size_t n_iovec, n_allocated, n_to_free = 0, i; size_t n_iovec, n_allocated, n_to_free = 0, i;
int r; int r;
@ -1275,19 +1286,19 @@ static int process_backtrace(int argc, char *argv[]) {
return -EINVAL; return -EINVAL;
} }
context[CONTEXT_PID] = argv[CONTEXT_PID + 2]; context[CONTEXT_PID] = argv[2 + CONTEXT_PID];
context[CONTEXT_UID] = argv[CONTEXT_UID + 2]; context[CONTEXT_UID] = argv[2 + CONTEXT_UID];
context[CONTEXT_GID] = argv[CONTEXT_GID + 2]; context[CONTEXT_GID] = argv[2 + CONTEXT_GID];
context[CONTEXT_SIGNAL] = argv[CONTEXT_SIGNAL + 2]; context[CONTEXT_SIGNAL] = argv[2 + CONTEXT_SIGNAL];
context[CONTEXT_TIMESTAMP] = argv[CONTEXT_TIMESTAMP + 2]; context[CONTEXT_TIMESTAMP] = argv[2 + CONTEXT_TIMESTAMP];
context[CONTEXT_RLIMIT] = argv[CONTEXT_RLIMIT + 2]; context[CONTEXT_RLIMIT] = argv[2 + CONTEXT_RLIMIT];
n_allocated = 33; /* 25 metadata, 2 static, +unknown input, rounded up */ n_allocated = 33; /* 25 metadata, 2 static, +unknown input, rounded up */
iovec = new(struct iovec, n_allocated); iovec = new(struct iovec, n_allocated);
if (!iovec) if (!iovec)
return log_oom(); return log_oom();
r = gather_pid_metadata_and_process_special_crash(context, argv + CONTEXT_COMM + 2, &comm, iovec, &n_to_free); r = gather_pid_metadata_and_process_special_crash(context, argv + 2 + CONTEXT_COMM, iovec, &n_to_free);
if (r < 0) if (r < 0)
goto finish; goto finish;
if (r > 0) { if (r > 0) {
@ -1313,7 +1324,8 @@ static int process_backtrace(int argc, char *argv[]) {
if (journal_importer_eof(&importer)) { if (journal_importer_eof(&importer)) {
log_warning("Did not receive a full journal entry on stdin, ignoring message sent by reporter"); log_warning("Did not receive a full journal entry on stdin, ignoring message sent by reporter");
message = strjoin("MESSAGE=Process ", context[CONTEXT_PID], " (", comm, ")" message = strjoin("MESSAGE=Process ", context[CONTEXT_PID],
" (", context[CONTEXT_COMM], ")"
" of user ", context[CONTEXT_UID], " of user ", context[CONTEXT_UID],
" failed with ", context[CONTEXT_SIGNAL]); " failed with ", context[CONTEXT_SIGNAL]);
if (!message) { if (!message) {
@ -1340,6 +1352,10 @@ static int process_backtrace(int argc, char *argv[]) {
for (i = 0; i < n_to_free; i++) for (i = 0; i < n_to_free; i++)
free(iovec[i].iov_base); free(iovec[i].iov_base);
/* Those fields are allocated by gather_pid_metadata_and_process_special_crash */
free(context[CONTEXT_COMM]);
free(context[CONTEXT_EXE]);
return r; return r;
} }