From 0db8836cb63f60799ac7b4c5222ba9e12ca5659c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 26 Nov 2009 12:04:13 +0100 Subject: hookCCpp: check total dump dir size and delete the largest/oldest one Signed-off-by: Denys Vlasenko --- src/Hooks/CCpp.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) (limited to 'src/Hooks/CCpp.cpp') diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp index 138d0d6..de74bdf 100644 --- a/src/Hooks/CCpp.cpp +++ b/src/Hooks/CCpp.cpp @@ -29,6 +29,8 @@ #define VAR_RUN_PID_FILE VAR_RUN"/abrt.pid" +using namespace std; + static char* get_executable(pid_t pid) { char buf[PATH_MAX + 1]; @@ -282,9 +284,58 @@ int main(int argc, char** argv) * but it does not log file name */ error_msg_and_die("error saving coredump to %s", path); } - /* free(executable); - why bother? */ - /* free(cmdline); */ + free(executable); + free(cmdline); log("saved core dump of pid %u to %s (%llu bytes)", (int)pid, path, (long long)size); + path[len] = '\0'; /* path now contains directory name */ + + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_debug_dump_dir's from other concurrent + * CCpp's won't be able to delete our dump (their delete_debug_dump_dir + * will wait for us), and we won't be able to delete their dumps. + * Classic deadlock. + */ + dd.Close(); + + /* Get it from abrt.conf */ + unsigned setting_MaxCrashReportsSize = 0; + FILE *fp = fopen(CONF_DIR"/abrt.conf", "r"); + if (fp) + { + char line[256]; + while (fgets(line, sizeof(line), fp) != NULL) + { + const char *p = skip_whitespace(line); + if (strncmp(p, "MaxCrashReportsSize", sizeof("MaxCrashReportsSize")-1) == 0) + { + p = skip_whitespace(p + sizeof("MaxCrashReportsSize")-1); + if (*p != '=') + continue; + p = skip_whitespace(p + 1); + if (isdigit(*p)) + setting_MaxCrashReportsSize = (unsigned long)atoi(p); + continue; + } + /* add more 'if (strncmp(p, "xx", sizeof("xx")-1) == 0)' here... */ + } + fclose(fp); + } + if (setting_MaxCrashReportsSize > 0) + { + string worst_dir; + while (1) + { + const char *base_dirname = strrchr(path, '/') + 1; /* never NULL */ + /* We exclude our own dump from candidates for deletion (3rd param): */ + double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname); + if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "") + break; + log("Size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str()); + delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str()); + worst_dir = ""; + } + } + return 0; } catch (CABRTException& e) -- cgit From 6e4495f7f13bd498f4f232b5e234e833a4573d73 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 27 Nov 2009 16:05:39 +0100 Subject: Hooks/CCpp.cpp: add MakeCompatCore = yes/no directive. Fixes rhbz#541707 Run tested Signed-off-by: Denys Vlasenko --- src/Hooks/CCpp.cpp | 220 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 185 insertions(+), 35 deletions(-) (limited to 'src/Hooks/CCpp.cpp') diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp index de74bdf..bea24c5 100644 --- a/src/Hooks/CCpp.cpp +++ b/src/Hooks/CCpp.cpp @@ -17,7 +17,15 @@ You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ +*/ + +/* Make all file ops "large" and make off_t 64-bit. + * No need to use O_LORGEFILE anywhere + */ +#define _LARGEFILE_SOURCE +#define _LARGEFILE64_SOURCE +#define _FILE_OFFSET_BITS 64 + #include "abrtlib.h" #include "DebugDump.h" #include "ABRTException.h" @@ -31,13 +39,12 @@ using namespace std; -static char* get_executable(pid_t pid) +static char* malloc_readlink(const char *linkname) { char buf[PATH_MAX + 1]; int len; - snprintf(buf, sizeof(buf), "/proc/%u/exe", (int)pid); - len = readlink(buf, buf, sizeof(buf)-1); + len = readlink(linkname, buf, sizeof(buf)-1); if (len >= 0) { buf[len] = '\0'; @@ -46,6 +53,22 @@ static char* get_executable(pid_t pid) return NULL; } +static char* get_executable(pid_t pid) +{ + char buf[sizeof("/proc/%u/exe") + sizeof(int)*3]; + + sprintf(buf, "/proc/%u/exe", (int)pid); + return malloc_readlink(buf); +} + +static char* get_cwd(pid_t pid) +{ + char buf[sizeof("/proc/%u/cwd") + sizeof(int)*3]; + + sprintf(buf, "/proc/%u/cwd", (int)pid); + return malloc_readlink(buf); +} + static char *append_escaped(char *start, const char *s) { char hex_char_buf[] = "\\x00"; @@ -135,18 +158,18 @@ static char* get_cmdline(pid_t pid) static int daemon_is_ok() { - char pid[sizeof(pid_t)*3 + 2]; - char path[PATH_MAX]; - struct stat buff; int fd = open(VAR_RUN_PID_FILE, O_RDONLY); if (fd < 0) { return 0; } + + char pid[sizeof(pid_t)*3 + 2]; int len = read(fd, pid, sizeof(pid)-1); close(fd); if (len <= 0) return 0; + pid[len] = '\0'; *strchrnul(pid, '\n') = '\0'; /* paranoia: we don't want to check /proc//stat or /proc///stat */ @@ -154,8 +177,10 @@ static int daemon_is_ok() return 0; /* TODO: maybe readlink and check that it is "xxx/abrt"? */ - snprintf(path, sizeof(path), "/proc/%s/stat", pid); - if (stat(path, &buff) == -1) + char path[sizeof("/proc/%s/stat") + sizeof(pid)]; + sprintf(path, "/proc/%s/stat", pid); + struct stat sb; + if (stat(path, &sb) == -1) { return 0; } @@ -192,6 +217,10 @@ int main(int argc, char** argv) { error_msg_and_die("pid '%s' or uid '%s' are bogus", argv[2], argv[4]); } + + char *user_pwd = get_cwd(pid); /* may be NULL on error */ + int core_fd = STDIN_FILENO; + if (!daemon_is_ok()) { /* not an error, exit with exitcode 0 */ @@ -199,7 +228,7 @@ int main(int argc, char** argv) "/proc/sys/kernel/core_pattern contains a stale value, " "consider resetting it to 'core'" ); - return 0; + goto create_user_core; } try @@ -225,9 +254,9 @@ int main(int argc, char** argv) * On the contrary, mere files are ignored by abrtd. */ snprintf(path, sizeof(path), "%s/abrtd-coredump", dddir); - int fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0644); - off_t size = copyfd_eof(STDIN_FILENO, fd); - if (size < 0 || close(fd) != 0) + core_fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0644); + off_t size = copyfd_eof(STDIN_FILENO, core_fd); + if (size < 0 || close(core_fd) != 0) { unlink(path); /* copyfd_eof logs the error including errno string, @@ -238,16 +267,6 @@ int main(int argc, char** argv) return 0; } - /* rhbz#539551: "abrt going crazy when crashing process is respawned". - * Do we want to protect against or ameliorate this? How? Ideas: - * (1) nice ourself? - * (2) check total size of dump dir, if it overflows, either abort dump - * or delete oldest/biggest dumps? [abort would be simpler, - * abrtd already does "delete on overflow" thing] - * (3) detect parallel dumps in progress and back off - * (pause/renice further/??) - */ - char* cmdline = get_cmdline(pid); /* never NULL */ const char *signame = strsignal(atoi(signal_str)); char *reason = xasprintf("Process was terminated by signal %s (%s)", signal_str, signame ? signame : signal_str); @@ -264,18 +283,19 @@ int main(int argc, char** argv) snprintf(path + len, sizeof(path) - len, "/"FILENAME_COREDUMP); /* We need coredumps to be readable by all, because - * process producing backtraces is run under the same UID + * when abrt daemon processes coredump, + * process producing backtrace is run under the same UID * as the crashed process. * Thus 644, not 600 */ - int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644); - if (fd < 0) + core_fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644); + if (core_fd < 0) { dd.Delete(); dd.Close(); perror_msg_and_die("can't open '%s'", path); } - off_t size = copyfd_eof(STDIN_FILENO, fd); - if (size < 0 || close(fd) != 0) + off_t size = copyfd_eof(STDIN_FILENO, core_fd); + if (size < 0 || fsync(core_fd) != 0) { unlink(path); dd.Delete(); @@ -284,6 +304,8 @@ int main(int argc, char** argv) * but it does not log file name */ error_msg_and_die("error saving coredump to %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 */ free(executable); free(cmdline); log("saved core dump of pid %u to %s (%llu bytes)", (int)pid, path, (long long)size); @@ -297,29 +319,74 @@ int main(int argc, char** argv) */ dd.Close(); - /* Get it from abrt.conf */ + /* Parse abrt.conf and plugins/CCpp.conf */ unsigned setting_MaxCrashReportsSize = 0; + bool setting_MakeCompatCore = false; + bool abrt_conf = true; FILE *fp = fopen(CONF_DIR"/abrt.conf", "r"); if (fp) { char line[256]; - while (fgets(line, sizeof(line), fp) != NULL) + while (1) { + if (fgets(line, sizeof(line), fp) == NULL) + { + /* Next .conf file plz */ + if (abrt_conf) + { + abrt_conf = false; + fp = fopen(CONF_DIR"/plugins/CCpp.conf", "r"); + if (fp) + continue; + } + break; + } + + unsigned len = strlen(line); + if (len > 0 && line[len-1] == '\n') + line[--len] = '\0'; const char *p = skip_whitespace(line); - if (strncmp(p, "MaxCrashReportsSize", sizeof("MaxCrashReportsSize")-1) == 0) +#undef DIRECTIVE +#define DIRECTIVE "MaxCrashReportsSize" + if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0) { - p = skip_whitespace(p + sizeof("MaxCrashReportsSize")-1); + p = skip_whitespace(p + sizeof(DIRECTIVE)-1); if (*p != '=') continue; p = skip_whitespace(p + 1); if (isdigit(*p)) - setting_MaxCrashReportsSize = (unsigned long)atoi(p); + /* x1.25: go a bit up, so that usual in-daemon trimming + * kicks in first, and we don't "fight" with it. */ + setting_MaxCrashReportsSize = (unsigned long)atoi(p) * 5 / 4; continue; } - /* add more 'if (strncmp(p, "xx", sizeof("xx")-1) == 0)' here... */ +#undef DIRECTIVE +#define DIRECTIVE "MakeCompatCore" + if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0) + { + p = skip_whitespace(p + sizeof(DIRECTIVE)-1); + if (*p != '=') + continue; + p = skip_whitespace(p + 1); + setting_MakeCompatCore = string_to_bool(p); + continue; + } +#undef DIRECTIVE + /* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */ } fclose(fp); } + + /* rhbz#539551: "abrt going crazy when crashing process is respawned". + * Do we want to protect against or ameliorate this? How? Ideas: + * (1) nice ourself? + * (2) check total size of dump dir, if it overflows, either abort dump + * or delete oldest/biggest dumps? [abort would be simpler, + * abrtd already does "delete on overflow" thing] + * (3) detect parallel dumps in progress and back off + * (pause/renice further/??) + */ + if (setting_MaxCrashReportsSize > 0) { string worst_dir; @@ -336,7 +403,9 @@ int main(int argc, char** argv) } } - return 0; + if (!setting_MakeCompatCore) + return 0; + /* fall through to creating user core */ } catch (CABRTException& e) { @@ -346,5 +415,86 @@ int main(int argc, char** argv) { error_msg_and_die("%s", e.what()); } + + create_user_core: + /* Write a core file for user */ + + /* 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).) + */ + + errno = 0; + if (setuid(uid) != 0 + || 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.%u") + sizeof(int)*3] = "core"; + 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.%u", (int)pid); + } + + /* 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 */ + struct stat sb; + 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) , need to mimic? */ + ) { + perror_msg("%s/%s is not a regular file with link count 1", user_pwd, core_basename); + return 1; + } + + if (ftruncate(usercore_fd, 0) != 0 + || copyfd_eof(core_fd, usercore_fd) < 0 + || close(usercore_fd) != 0 + ) { + perror_msg("write error writing %s/%s", user_pwd, core_basename); + unlink(core_basename); + return 1; + } + log("saved core dump of pid %u to %s/%s", (int)pid, user_pwd, core_basename); + return 0; } -- cgit From f0bd70b3f68835983a10052337fc316a9f861811 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 27 Nov 2009 20:13:05 +0100 Subject: Hooks/CCpp.cpp: cosmetics after last big commit Signed-off-by: Denys Vlasenko --- src/Hooks/CCpp.cpp | 76 +++++++++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 38 deletions(-) (limited to 'src/Hooks/CCpp.cpp') diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp index bea24c5..78a2471 100644 --- a/src/Hooks/CCpp.cpp +++ b/src/Hooks/CCpp.cpp @@ -329,18 +329,18 @@ int main(int argc, char** argv) char line[256]; while (1) { - if (fgets(line, sizeof(line), fp) == NULL) - { - /* Next .conf file plz */ - if (abrt_conf) - { - abrt_conf = false; - fp = fopen(CONF_DIR"/plugins/CCpp.conf", "r"); - if (fp) - continue; - } - break; - } + if (fgets(line, sizeof(line), fp) == NULL) + { + /* Next .conf file plz */ + if (abrt_conf) + { + abrt_conf = false; + fp = fopen(CONF_DIR"/plugins/CCpp.conf", "r"); + if (fp) + continue; + } + break; + } unsigned len = strlen(line); if (len > 0 && line[len-1] == '\n') @@ -397,7 +397,7 @@ int main(int argc, char** argv) double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname); if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "") break; - log("Size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str()); + log("size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str()); delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str()); worst_dir = ""; } @@ -419,6 +419,28 @@ int main(int argc, char** argv) create_user_core: /* Write a core file for user */ + errno = 0; + if (setuid(uid) != 0 + || 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.%u") + sizeof(int)*3] = "core"; + 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.%u", (int)pid); + } + /* man core: * There are various circumstances in which a core dump file * is not produced: @@ -450,28 +472,6 @@ int main(int argc, char** argv) * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).) */ - errno = 0; - if (setuid(uid) != 0 - || 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.%u") + sizeof(int)*3] = "core"; - 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.%u", (int)pid); - } - /* 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 */ @@ -480,16 +480,16 @@ int main(int argc, char** argv) || 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) , need to mimic? */ + /* kernel internal dumper checks this too: if (inode->i_uid != current->fsuid) , need to mimic? */ ) { - perror_msg("%s/%s is not a regular file with link count 1", user_pwd, core_basename); - return 1; + perror_msg_and_die("%s/%s is not a regular file with link count 1", user_pwd, core_basename); } if (ftruncate(usercore_fd, 0) != 0 || copyfd_eof(core_fd, usercore_fd) < 0 || close(usercore_fd) != 0 ) { + /* perror first, otherwise unlink may trash errno */ perror_msg("write error writing %s/%s", user_pwd, core_basename); unlink(core_basename); return 1; -- cgit