Please review, errors in this code can have really nasty results -
like corrupted coredumps! :(
--
vda
diff -x '*.po' -d -urpN abrt.3/lib/Utils/xfuncs.cpp abrt.4/lib/Utils/xfuncs.cpp
--- abrt.3/lib/Utils/xfuncs.cpp 2010-04-30 11:50:37.000000000 +0200
+++ abrt.4/lib/Utils/xfuncs.cpp 2010-05-26 15:56:03.492905966 +0200
@@ -390,6 +390,18 @@ bool string_to_bool(const char *s)
return false;
}
+void xseteuid(uid_t euid)
+{
+ if (seteuid(euid) != 0)
+ perror_msg_and_die("can't set %cid %lu", 'u', (long)euid);
+}
+
+void xsetegid(gid_t egid)
+{
+ if (setegid(egid) != 0)
+ perror_msg_and_die("can't set %cid %lu", 'g', (long)egid);
+}
+
void xsetreuid(uid_t ruid, uid_t euid)
{
if (setreuid(ruid, euid) != 0)
diff -x '*.po' -d -urpN abrt.3/lib/Utils/xfuncs.h abrt.4/lib/Utils/xfuncs.h
--- abrt.3/lib/Utils/xfuncs.h 2010-04-30 11:50:37.000000000 +0200
+++ abrt.4/lib/Utils/xfuncs.h 2010-05-26 15:55:33.668905895 +0200
@@ -76,12 +76,12 @@ extern char *last_char_is(const char *s,
extern bool string_to_bool(const char *s);
+extern void xseteuid(uid_t euid);
+extern void xsetegid(gid_t egid);
extern void xsetreuid(uid_t ruid, uid_t euid);
extern void xsetregid(gid_t rgid, gid_t egid);
-/* Do not pass the returned pointer to free().
- Do not modify the contents of the returned string.
- NULL is returned in the case of failure. */
+/* Returns getpwuid(uid)->pw_dir or NULL */
extern const char *get_home_dir(uid_t uid);
#ifdef __cplusplus
diff -x '*.po' -d -urpN abrt.3/src/Hooks/abrt-hook-ccpp.cpp abrt.4/src/Hooks/abrt-hook-ccpp.cpp
--- abrt.3/src/Hooks/abrt-hook-ccpp.cpp 2010-05-21 16:05:24.000000000 +0200
+++ abrt.4/src/Hooks/abrt-hook-ccpp.cpp 2010-05-26 15:57:32.404907193 +0200
@@ -41,6 +41,97 @@ static char* malloc_readlink(const char
return NULL;
}
+/* Custom version of copyfd_xyz,
+ * one which is able to write into two descriptors at once.
+ */
+#define CONFIG_FEATURE_COPYBUF_KB 4
+static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2)
+{
+ off_t total = 0;
+ int last_was_seek = 0;
+#if CONFIG_FEATURE_COPYBUF_KB <= 4
+ char buffer[CONFIG_FEATURE_COPYBUF_KB * 1024];
+ enum { buffer_size = sizeof(buffer) };
+#else
+ char *buffer;
+ int buffer_size;
+
+ /* We want page-aligned buffer, just in case kernel is clever
+ * and can do page-aligned io more efficiently */
+ buffer = mmap(NULL, CONFIG_FEATURE_COPYBUF_KB * 1024,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON,
+ /* ignored: */ -1, 0);
+ buffer_size = CONFIG_FEATURE_COPYBUF_KB * 1024;
+ if (buffer == MAP_FAILED) {
+ buffer = alloca(4 * 1024);
+ buffer_size = 4 * 1024;
+ }
+#endif
+
+ while (1) {
+ ssize_t rd = safe_read(src_fd, buffer, buffer_size);
+ if (!rd) { /* eof */
+ if (last_was_seek) {
+ if (lseek(dst_fd1, -1, SEEK_CUR) < 0
+ || safe_write(dst_fd1, "", 1) != 1
+ || (dst_fd2 >= 0
+ && (lseek(dst_fd2, -1, SEEK_CUR) < 0
+ || safe_write(dst_fd2, "", 1) != 1
+ )
+ )
+ ) {
+ perror_msg("write error");
+ total = -1;
+ goto out;
+ }
+ }
+ /* all done */
+ goto out;
+ }
+ if (rd < 0) {
+ perror_msg("read error");
+ total = -1;
+ goto out;
+ }
+
+ /* checking sparseness */
+ ssize_t cnt = rd;
+ while (--cnt >= 0) {
+ if (buffer[cnt] != 0) {
+ /* not sparse */
+ errno = 0;
+ ssize_t wr1 = full_write(dst_fd1, buffer, rd);
+ ssize_t wr2 = (dst_fd2 >= 0 ? full_write(dst_fd2, buffer, rd) : rd);
+ if (wr1 < rd || wr2 < rd) {
+ perror_msg("write error");
+ total = -1;
+ goto out;
+ }
+ last_was_seek = 0;
+ goto adv;
+ }
+ }
+ /* sparse */
+ xlseek(dst_fd1, rd, SEEK_CUR);
+ if (dst_fd2 >= 0)
+ xlseek(dst_fd2, rd, SEEK_CUR);
+ last_was_seek = 1;
+ adv:
+ total += rd;
+ size2 -= rd;
+ if (size2 < 0)
+ dst_fd2 = -1;
+ }
+ out:
+
+#if CONFIG_FEATURE_COPYBUF_KB > 4
+ if (buffer_size != 4 * 1024)
+ munmap(buffer, buffer_size);
+#endif
+ return total;
+}
+
static char* get_executable(pid_t pid)
{
char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3];
@@ -57,16 +148,100 @@ static char* get_cwd(pid_t pid)
return malloc_readlink(buf);
}
+static char core_basename[sizeof("core.%lu") + sizeof(long)*3] = "core";
+
+static int open_user_core(const char *user_pwd, uid_t uid, pid_t pid)
+{
+ struct passwd* pw = getpwuid(uid);
+ gid_t gid = pw ? pw->pw_gid : uid;
+ xsetegid(gid);
+ xseteuid(uid);
+
+ errno = 0;
+ if (user_pwd == NULL
+ || chdir(user_pwd) != 0
+ ) {
+ perror_msg("can't cd to %s", user_pwd);
+ return 0;
+ }
+
+ /* Mimic "core.PID" if requested */
+ char buf[] = "0\n";
+ int fd = open("/proc/sys/kernel/core_uses_pid", O_RDONLY);
+ if (fd >= 0)
+ {
+ read(fd, buf, sizeof(buf));
+ close(fd);
+ }
+ if (strcmp(buf, "1\n") == 0)
+ {
+ sprintf(core_basename, "core.%lu", (long)pid);
+ }
+
+ /* man core:
+ * There are various circumstances in which a core dump file
+ * is not produced:
+ *
+ * [skipped obvious ones]
+ * The process does not have permission to write the core file.
+ * ...if a file with the same name exists and is not writable
+ * or is not a regular file (e.g., it is a directory or a symbolic link).
+ *
+ * A file with the same name already exists, but there is more
+ * than one hard link to that file.
+ *
+ * The file system where the core dump file would be created is full;
+ * or has run out of inodes; or is mounted read-only;
+ * or the user has reached their quota for the file system.
+ *
+ * The RLIMIT_CORE or RLIMIT_FSIZE resource limits for the process
+ * are set to zero.
+ * [shouldn't it be checked by kernel? 2.6.30.9-96 doesn't, still
+ * calls us even if "ulimit -c 0"]
+ *
+ * The binary being executed by the process does not have
+ * read permission enabled. [how we can check it here?]
+ *
+ * The process is executing a set-user-ID (set-group-ID) program
+ * that is owned by a user (group) other than the real
+ * user (group) ID of the process. [TODO?]
+ * (However, see the description of the prctl(2) PR_SET_DUMPABLE operation,
+ * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).)
+ */
+
+ /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */
+ struct stat sb;
+ errno = 0;
+ int user_core_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW, 0600); /* kernel makes 0600 too */
+ xsetegid(0);
+ xseteuid(0);
+ if (user_core_fd < 0
+ || fstat(user_core_fd, &sb) != 0
+ || !S_ISREG(sb.st_mode)
+ || sb.st_nlink != 1
+ /* kernel internal dumper checks this too: if (inode->i_uid != current->fsuid) <fail>, need to mimic? */
+ ) {
+ perror_msg("%s/%s is not a regular file with link count 1", user_pwd, core_basename);
+ return -1;
+ }
+ if (ftruncate(user_core_fd, 0) != 0) {
+ /* perror first, otherwise unlink may trash errno */
+ perror_msg("truncate %s/%s", user_pwd, core_basename);
+ return -1;
+ }
+
+ return user_core_fd;
+}
+
int main(int argc, char** argv)
{
- int fd;
struct stat sb;
if (argc < 5)
{
- const char* program_name = argv[0];
- error_msg_and_die("Usage: %s: DUMPDIR PID SIGNO UID CORE_SIZE_LIMIT", program_name);
+ error_msg_and_die("Usage: %s: DUMPDIR PID SIGNO UID CORE_SIZE_LIMIT", argv[0]);
}
+
openlog("abrt", LOG_PID, LOG_DAEMON);
logmode = LOGMODE_SYSLOG;
@@ -105,8 +280,10 @@ int main(int argc, char** argv)
bool setting_MakeCompatCore = false;
parse_conf(CONF_DIR"/plugins/CCpp.conf", &setting_MaxCrashReportsSize, &setting_MakeCompatCore);
- int core_fd = STDIN_FILENO;
- off_t core_size = 0;
+ /* Open a fd to compat coredump, if requested and is possible */
+ int user_core_fd = -1;
+ if (setting_MakeCompatCore && ulimit_c != 0)
+ user_core_fd = open_user_core(user_pwd, uid, pid);
const char *signame = NULL;
/* Tried to use array for this but C++ does not support v[] = { [IDX] = "str" } */
@@ -122,10 +299,7 @@ int main(int argc, char** argv)
//case SIGTRAP: signame = "TRAP"; break; //Trace/breakpoint trap
//case SIGXCPU: signame = "XCPU"; break; //CPU time limit exceeded (4.2BSD)
//case SIGXFSZ: signame = "XFSZ"; break; //File size limit exceeded (4.2BSD)
- }
- if (signame == NULL) {
- /* not a signal we care about */
- goto create_user_core;
+ default: goto create_user_core; // not a signal we care about
}
if (!daemon_is_ok())
@@ -151,7 +325,7 @@ int main(int argc, char** argv)
* if they happen too often. Else, write new marker value.
*/
snprintf(path, sizeof(path), "%s/last-ccpp", dddir);
- fd = open(path, O_RDWR | O_CREAT, 0600);
+ int fd = open(path, O_RDWR | O_CREAT, 0600);
if (fd >= 0)
{
int sz;
@@ -188,9 +362,9 @@ int main(int argc, char** argv)
* Unlike dirs, mere files are ignored by abrtd.
*/
snprintf(path, sizeof(path), "%s/abrtd-coredump", dddir);
- core_fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
- core_size = copyfd_eof(STDIN_FILENO, core_fd, COPYFD_SPARSE);
- if (core_size < 0 || close(core_fd) != 0)
+ int abrt_core_fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+ off_t core_size = copyfd_eof(STDIN_FILENO, abrt_core_fd, COPYFD_SPARSE);
+ if (core_size < 0 || fsync(abrt_core_fd) != 0)
{
unlink(path);
/* copyfd_eof logs the error including errno string,
@@ -201,58 +375,79 @@ int main(int argc, char** argv)
return 0;
}
- char* cmdline = get_cmdline(pid); /* never NULL */
- char *reason = xasprintf("Process %s was killed by signal %s (SIG%s)", executable, signal_str, signame ? signame : signal_str);
-
unsigned path_len = snprintf(path, sizeof(path), "%s/ccpp-%ld-%lu.new",
dddir, (long)time(NULL), (long)pid);
- if (path_len >= sizeof(path))
- exit(1);
+ if (path_len >= (sizeof(path) - sizeof("/"FILENAME_COREDUMP)))
+ return 1;
CDebugDump dd;
+ char *cmdline = get_cmdline(pid); /* never NULL */
+ char *reason = xasprintf("Process %s was killed by signal %s (SIG%s)", executable, signal_str, signame ? signame : signal_str);
dd.Create(path, uid);
dd.SaveText(FILENAME_ANALYZER, "CCpp");
dd.SaveText(FILENAME_EXECUTABLE, executable);
dd.SaveText(FILENAME_CMDLINE, cmdline);
dd.SaveText(FILENAME_REASON, reason);
-
- int len = strlen(path);
- snprintf(path + len, sizeof(path) - len, "/"FILENAME_COREDUMP);
+ free(cmdline);
+ free(reason);
/* We need coredumps to be readable by all, because
* when abrt daemon processes coredump,
* process producing backtrace is run under the same UID
* as the crashed process.
* Thus 644, not 600 */
- core_fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
- if (core_fd < 0)
+ strcpy(path + path_len, "/"FILENAME_COREDUMP);
+ int abrt_core_fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
+ if (abrt_core_fd < 0)
{
int sv_errno = errno;
dd.Delete();
dd.Close();
+ if (user_core_fd >= 0)
+ {
+ xchdir(user_pwd);
+ unlink(core_basename);
+ }
errno = sv_errno;
perror_msg_and_die("can't open '%s'", path);
}
-//TODO: chown to uid:abrt?
+
+ /* We write both coredumps at once.
+ * We can't write user coredump first, since it might be truncated
+ * and thus can't be copied and used as abrt coredump;
+ * and if we write abrt coredump first and then copy it as user one,
+ * then we have a race when process exits but coredump does not exist yet:
+ * $ echo -e '#include<signal.h>\nmain(){raise(SIGSEGV);}' | gcc -o test -x c -
+ * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core*
+ * 21631 Segmentation fault (core dumped) ./test
+ * ls: cannot access core*: No such file or directory <=== BAD
+ */
+//TODO: fchown abrt_core_fd to uid:abrt?
//Currently it is owned by 0:0 but is readable by anyone, so the owner
//of the crashed binary still can access it, as he has
//r-x access to the dump dir.
- core_size = copyfd_eof(STDIN_FILENO, core_fd, COPYFD_SPARSE);
- if (core_size < 0 || fsync(core_fd) != 0)
+ off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c);
+ if (core_size < 0 || fsync(abrt_core_fd) != 0)
{
unlink(path);
dd.Delete();
dd.Close();
- /* copyfd_eof logs the error including errno string,
+ if (user_core_fd >= 0)
+ {
+ xchdir(user_pwd);
+ unlink(core_basename);
+ }
+ /* copyfd_sparse logs the error including errno string,
* but it does not log file name */
- error_msg_and_die("error saving coredump to %s", path);
+ error_msg_and_die("error writing %s", path);
}
- lseek(core_fd, 0, SEEK_SET);
- /* note: core_fd is still open, we may use it later to copy core to user's dir */
log("saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
- free(executable);
- free(cmdline);
- path[len] = '\0'; /* path now contains directory name */
+ if (user_core_fd >= 0 && core_size >= ulimit_c)
+ {
+ /* user coredump is too big, nuke it */
+ xchdir(user_pwd);
+ unlink(core_basename);
+ }
/* We close dumpdir before we start catering for crash storm case.
* Otherwise, delete_debug_dump_dir's from other concurrent
@@ -261,8 +456,9 @@ int main(int argc, char** argv)
* Classic deadlock.
*/
dd.Close();
+ path[path_len] = '\0'; /* path now contains only directory name */
char *newpath = xstrndup(path, path_len - (sizeof(".new")-1));
- if (rename(path, newpath) != 0)
+ if (rename(path, newpath) == 0)
strcpy(path, newpath);
free(newpath);
@@ -272,7 +468,7 @@ int main(int argc, char** argv)
trim_debug_dumps(setting_MaxCrashReportsSize, path);
}
- /* fall through to creating user core */
+ return 0;
}
catch (CABRTException& e)
{
@@ -283,108 +479,26 @@ int main(int argc, char** argv)
error_msg_and_die("%s", e.what());
}
-
+ /* We didn't create abrt dump, but may need to create compat coredump */
create_user_core:
- if (!setting_MakeCompatCore)
- return 0;
-
- /* note: core_size may be == 0 ("unknown") */
- if (core_size > ulimit_c || ulimit_c == 0)
+ if (user_core_fd < 0)
return 0;
- /* Write a core file for user */
-
- struct passwd* pw = getpwuid(uid);
- gid_t gid = pw ? pw->pw_gid : uid;
- setgroups(1, &gid);
- xsetregid(gid, gid);
- xsetreuid(uid, uid);
-
- errno = 0;
- if (user_pwd == NULL
- || chdir(user_pwd) != 0
- ) {
- perror_msg_and_die("can't cd to %s", user_pwd);
- }
-
- /* Mimic "core.PID" if requested */
- char core_basename[sizeof("core.%lu") + sizeof(long)*3] = "core";
- char buf[] = "0\n";
- fd = open("/proc/sys/kernel/core_uses_pid", O_RDONLY);
- if (fd >= 0)
- {
- read(fd, buf, sizeof(buf));
- close(fd);
- }
- if (strcmp(buf, "1\n") == 0)
- {
- sprintf(core_basename, "core.%lu", (long)pid);
- }
-
- /* man core:
- * There are various circumstances in which a core dump file
- * is not produced:
- *
- * [skipped obvious ones]
- * The process does not have permission to write the core file.
- * ...if a file with the same name exists and is not writable
- * or is not a regular file (e.g., it is a directory or a symbolic link).
- *
- * A file with the same name already exists, but there is more
- * than one hard link to that file.
- *
- * The file system where the core dump file would be created is full;
- * or has run out of inodes; or is mounted read-only;
- * or the user has reached their quota for the file system.
- *
- * The RLIMIT_CORE or RLIMIT_FSIZE resource limits for the process
- * are set to zero.
- * [shouldn't it be checked by kernel? 2.6.30.9-96 doesn't, still
- * calls us even if "ulimit -c 0"]
- *
- * The binary being executed by the process does not have
- * read permission enabled. [how we can check it here?]
- *
- * The process is executing a set-user-ID (set-group-ID) program
- * that is owned by a user (group) other than the real
- * user (group) ID of the process. [TODO?]
- * (However, see the description of the prctl(2) PR_SET_DUMPABLE operation,
- * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).)
- */
-
- /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */
- errno = 0;
- int usercore_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW, 0600); /* kernel makes 0600 too */
- if (usercore_fd < 0
- || fstat(usercore_fd, &sb) != 0
- || !S_ISREG(sb.st_mode)
- || sb.st_nlink != 1
- /* kernel internal dumper checks this too: if (inode->i_uid != current->fsuid) <fail>, need to mimic? */
- ) {
- perror_msg_and_die("%s/%s is not a regular file with link count 1", user_pwd, core_basename);
- }
-
- /* Note: we do not copy more than ulimit_c */
- off_t size;
- if (ftruncate(usercore_fd, 0) != 0
- || (size = copyfd_size(core_fd, usercore_fd, ulimit_c, COPYFD_SPARSE)) < 0
- || close(usercore_fd) != 0
- ) {
+ off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE);
+ if (core_size < 0 || fsync(user_core_fd) != 0) {
/* perror first, otherwise unlink may trash errno */
- perror_msg("write error writing %s/%s", user_pwd, core_basename);
+ perror_msg("error writing %s/%s", user_pwd, core_basename);
+ xchdir(user_pwd);
unlink(core_basename);
return 1;
}
- if (size == ulimit_c && size != core_size)
+ if (core_size >= ulimit_c)
{
- /* We copied exactly ulimit_c bytes (and it doesn't accidentally match
- * core_size (imagine exactly 1MB coredump with "ulimit -c 1M" - that'd be ok)),
- * it means that core is larger than ulimit_c. Abort and delete the dump.
- */
+ xchdir(user_pwd);
unlink(core_basename);
return 1;
}
- log("saved core dump of pid %lu to %s/%s (%llu bytes)", (long)pid, user_pwd, core_basename, (long long)size);
+ log("saved core dump of pid %lu to %s/%s (%llu bytes)", (long)pid, user_pwd, core_basename, (long long)core_size);
return 0;
}
