diff --git a/0047-testsuite-make-report_python-os-release-test-more-ve.patch b/0047-testsuite-make-report_python-os-release-test-more-ve.patch new file mode 100644 index 0000000..4244605 --- /dev/null +++ b/0047-testsuite-make-report_python-os-release-test-more-ve.patch @@ -0,0 +1,31 @@ +From 38159845d684571ac39558015012110e0b7b24a3 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 2 Mar 2015 08:34:08 +0100 +Subject: [PATCH] testsuite: make report_python os-release test more verbose + +Signed-off-by: Jakub Filak +--- + tests/report_python.at | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/tests/report_python.at b/tests/report_python.at +index 5569b1f..e71e169 100644 +--- a/tests/report_python.at ++++ b/tests/report_python.at +@@ -23,11 +23,11 @@ if not os.path.exists("/etc/os-release"): + + exit_code = 0 + if report.getProduct_fromOSRELEASE() != report.getProduct(): +- print "getProduct() did not return PRODUCT from /etc/os-release" ++ print "getProduct('{0}') did not return PRODUCT='{1}' from /etc/os-release".format(report.getProduct(), report.getProduct_fromOSRELEASE()) + exit_code += 1 + + if report.getVersion_fromOSRELEASE() != report.getVersion(): +- print "getVersion() did not return PRODUCT from /etc/os-release" ++ print "getVersion('{0}') did not return PRODUCT_VERSION='{1}' from /etc/os-release".format(report.getVersion(), report.getVersion_fromOSRELEASE()) + exit_code += 1 + + sys.exit(exit_code) +-- +2.1.0 + diff --git a/0048-report-python-fix-getVersion_fromOSRELEASE.patch b/0048-report-python-fix-getVersion_fromOSRELEASE.patch new file mode 100644 index 0000000..a8fa52c --- /dev/null +++ b/0048-report-python-fix-getVersion_fromOSRELEASE.patch @@ -0,0 +1,154 @@ +From cb9e67f8045126a6d13db5ad48fdec2cdc14452b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 4 Mar 2015 11:38:45 +0100 +Subject: [PATCH] report-python: fix getVersion_fromOSRELEASE + +Related to rhbz#1198551 + +Signed-off-by: Jakub Filak +--- + src/report-python/__init__.py | 5 +++- + tests/osinfo.at | 24 +++++++-------- + tests/report_python.at | 68 +++++++++++++++++++++++++++++++++++++++++++ + 3 files changed, 84 insertions(+), 13 deletions(-) + +diff --git a/src/report-python/__init__.py b/src/report-python/__init__.py +index 6c75eb1..2af46e2 100644 +--- a/src/report-python/__init__.py ++++ b/src/report-python/__init__.py +@@ -36,7 +36,7 @@ SYSTEM_RELEASE_PATHS = ["/etc/system-release","/etc/redhat-release"] + SYSTEM_RELEASE_DEPS = ["system-release", "redhat-release"] + SYSTEM_OS_RELEASE_FILE = "/etc/os-release" + OS_RELEASE_PRODUCT_FIELDS = ["REDHAT_BUGZILLA_PRODUCT", "REDHAT_SUPPORT_PRODUCT", "NAME"] +-OS_RELEASE_VERSION_FIELDS = ["REDHAT_BUGZILLA_VERSION", "REDHAT_SUPPORT_VERSION", "NAME"] ++OS_RELEASE_VERSION_FIELDS = ["REDHAT_BUGZILLA_PRODUCT_VERSION", "REDHAT_SUPPORT_PRODUCT_VERSION", "VERSION_ID"] + + _hardcoded_default_product = "" + _hardcoded_default_version = "" +@@ -75,6 +75,9 @@ def parse_os_release_lines(osreleaselines): + osrel = {} + + for line in osreleaselines: ++ if line.endswith("\n"): ++ line = line[:-1] ++ + kvp = line.split('=') + if len(kvp) < 2: + continue +diff --git a/tests/osinfo.at b/tests/osinfo.at +index 868a9a2..6ece180 100644 +--- a/tests/osinfo.at ++++ b/tests/osinfo.at +@@ -408,18 +408,18 @@ report = __import__("report-python", globals(), locals(), [], -1) + sys.modules["report"] = report + + lines = [ +- 'NAME=fedora', +- 'VERSION="20 (Heisenbug)"', +- 'ID=fedora', +- 'VERSION_ID=20', +- 'PRETTY_NAME="Fedora 20 (Heisenbug)"', +- 'ANSI_COLOR="0;34"', +- 'CPE_NAME="cpe:/o:fedoraproject:fedora:20"', +- 'HOME_URL="https://fedoraproject.org/"', +- 'BUG_REPORT_URL="https://bugzilla.redhat.com/"', +- 'REDHAT_BUGZILLA_PRODUCT="Fedora"', +- 'REDHAT_BUGZILLA_PRODUCT_VERSION=20', +- 'REDHAT_SUPPORT_PRODUCT="Fedora"', ++ 'NAME=fedora\n', ++ 'VERSION="20 (Heisenbug)"\n', ++ 'ID=fedora\n', ++ 'VERSION_ID=20\n', ++ 'PRETTY_NAME="Fedora 20 (Heisenbug)"\n', ++ 'ANSI_COLOR="0;34"\n', ++ 'CPE_NAME="cpe:/o:fedoraproject:fedora:20"\n', ++ 'HOME_URL="https://fedoraproject.org/"\n', ++ 'BUG_REPORT_URL="https://bugzilla.redhat.com/"\n', ++ 'REDHAT_BUGZILLA_PRODUCT="Fedora"\n', ++ 'REDHAT_BUGZILLA_PRODUCT_VERSION=20\n', ++ 'REDHAT_SUPPORT_PRODUCT="Fedora"\n', + 'REDHAT_SUPPORT_PRODUCT_VERSION=20', + ] + +diff --git a/tests/report_python.at b/tests/report_python.at +index e71e169..d41fe16 100644 +--- a/tests/report_python.at ++++ b/tests/report_python.at +@@ -2,6 +2,74 @@ + + AT_BANNER([report_python]) + ++## ------------------------- ## ++## arbitrary_etc_os_releases ## ++## ------------------------- ## ++ ++AT_PYTESTFUN([arbitrary_etc_os_releases], ++[[import sys ++import tempfile ++import os ++ ++sys.path.insert(0, "../../../src/report-python") ++sys.path.insert(0, "../../../src/report-python/.libs") ++ ++report = __import__("report-python", globals(), locals(), [], -1) ++sys.modules["report"] = report ++ ++ ++PRODUCT_TEST_CASES = [ ++ ("REDHAT_BUGZILLA_PRODUCT", "bugzilla-product"), ++ ("REDHAT_SUPPORT_PRODUCT", "support-product"), ++ ("NAME", "os-name") ++] ++ ++VERSION_TEST_CASES = [ ++ ("REDHAT_BUGZILLA_PRODUCT_VERSION", "bugzilla-product-version"), ++ ("REDHAT_SUPPORT_PRODUCT_VERSION", "support-product-version"), ++ ("VERSION_ID", "os-version-id") ++] ++ ++def run_test(fields, getter, expected): ++ retval = True ++ ++ osrelf = tempfile.NamedTemporaryFile(delete=False) ++ osrelf.write("ID=\"field-id\"\n") ++ ++ for (field, value) in fields: ++ osrelf.write("%s=%s\n" %(field, value)) ++ ++ osrelf.write("PRETTY_NAME=\"field-pretty-name\"\n") ++ osrelf.close() ++ ++ result = getter(file_path=osrelf.name) ++ if result != expected: ++ print("expected: '%s'" % (expected)) ++ print("result : '%s'" % (result)) ++ retval = False ++ ++ os.remove(osrelf.name) ++ return retval ++ ++ ++def verify_information_type(test_cases, stuffing, getter): ++ retval = 0 ++ for i in xrange(0, len(test_cases)): ++ for j in xrange(len(test_cases), i, -1): ++ if not run_test(stuffing + test_cases[i:j], getter, test_cases[i][1]): ++ print("field : '%s'" % (test_cases[i][0])) ++ retval += 1 ++ ++ ++def main(): ++ verify_information_type(PRODUCT_TEST_CASES, VERSION_TEST_CASES, report.getProduct_fromOSRELEASE) ++ verify_information_type(VERSION_TEST_CASES, PRODUCT_TEST_CASES, report.getVersion_fromOSRELEASE) ++ ++ ++if __name__ == "__main__": ++ sys.exit(main()) ++]]) ++ + ## ----------------------- ## + ## get_from_etc_os_release ## + ## ----------------------- ## +-- +2.1.0 + diff --git a/0049-testsuite-report_python-dump-etc-os-relase-to-stdout.patch b/0049-testsuite-report_python-dump-etc-os-relase-to-stdout.patch new file mode 100644 index 0000000..2b0d46e --- /dev/null +++ b/0049-testsuite-report_python-dump-etc-os-relase-to-stdout.patch @@ -0,0 +1,30 @@ +From 5d7e17ab618d104454a9c23335e7a6045b544b5e Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 12 Mar 2015 18:37:20 +0100 +Subject: [PATCH] testsuite: report_python: dump /etc/os-relase to stdout upon + failures + +Signed-off-by: Jakub Filak +--- + tests/report_python.at | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/tests/report_python.at b/tests/report_python.at +index d41fe16..ee797dd 100644 +--- a/tests/report_python.at ++++ b/tests/report_python.at +@@ -98,5 +98,11 @@ if report.getVersion_fromOSRELEASE() != report.getVersion(): + print "getVersion('{0}') did not return PRODUCT_VERSION='{1}' from /etc/os-release".format(report.getVersion(), report.getVersion_fromOSRELEASE()) + exit_code += 1 + ++if exit_code != 0: ++ print "++++ /etc/os-release ++++" ++ with open("/etc/os-release") as osrel: ++ sys.stdout.write(osrel.read()) ++ print "^^^^ /etc/os-release ^^^^" ++ + sys.exit(exit_code) + ]]) +-- +2.1.0 + diff --git a/0050-lib-add-proc-pid-utils.patch b/0050-lib-add-proc-pid-utils.patch new file mode 100644 index 0000000..08c4457 --- /dev/null +++ b/0050-lib-add-proc-pid-utils.patch @@ -0,0 +1,178 @@ +From 23fcd4616606eca71ffe5f862828163d23490cc0 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 8 Dec 2014 15:26:56 +0100 +Subject: [PATCH] lib: add /proc/[pid]/utils + +Related to abrt/abrt#548 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 12 +++++ + src/lib/get_cmdline.c | 101 +++++++++++++++++++++++++++++++++++++++ + src/lib/read_write.c | 14 ++++++ + 3 files changed, 127 insertions(+) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index d664fa4..fca8138 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -178,6 +178,8 @@ void* xmalloc_read(int fd, size_t *maxsz_p); + void* xmalloc_open_read_close(const char *filename, size_t *maxsz_p); + #define xmalloc_xopen_read_close libreport_xmalloc_xopen_read_close + void* xmalloc_xopen_read_close(const char *filename, size_t *maxsz_p); ++#define malloc_readlink libreport_malloc_readlink ++char* malloc_readlink(const char *linkname); + + + /* Returns malloc'ed block */ +@@ -625,6 +627,16 @@ struct strbuf *strbuf_prepend_strfv(struct strbuf *strbuf, + char* get_cmdline(pid_t pid); + #define get_environ libreport_get_environ + char* get_environ(pid_t pid); ++#define get_executable libreport_get_executable ++char *get_executable(pid_t pid); ++#define get_cwd libreport_get_cwd ++char* get_cwd(pid_t pid); ++#define get_rootdir libreport_get_rootdir ++char* get_rootdir(pid_t pid); ++#define get_fsuid libreport_get_fsuid ++int get_fsuid(const char *proc_pid_status); ++#define dump_fd_info libreport_dump_fd_info ++int dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs); + + /* Takes ptr to time_t, or NULL if you want to use current time. + * Returns "YYYY-MM-DD-hh:mm:ss" string. +diff --git a/src/lib/get_cmdline.c b/src/lib/get_cmdline.c +index 55c49ea..7ceba2f 100644 +--- a/src/lib/get_cmdline.c ++++ b/src/lib/get_cmdline.c +@@ -148,3 +148,104 @@ char* get_environ(pid_t pid) + snprintf(path, sizeof(path), "/proc/%lu/environ", (long)pid); + return get_escaped(path, '\n'); + } ++ ++char* get_executable(pid_t pid) ++{ ++ char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3]; ++ ++ sprintf(buf, "/proc/%lu/exe", (long)pid); ++ char *executable = malloc_readlink(buf); ++ if (!executable) ++ return NULL; ++ /* find and cut off " (deleted)" from the path */ ++ char *deleted = executable + strlen(executable) - strlen(" (deleted)"); ++ if (deleted > executable && strcmp(deleted, " (deleted)") == 0) ++ { ++ *deleted = '\0'; ++ log_info("File '%s' seems to be deleted", executable); ++ } ++ /* find and cut off prelink suffixes from the path */ ++ char *prelink = executable + strlen(executable) - strlen(".#prelink#.XXXXXX"); ++ if (prelink > executable && strncmp(prelink, ".#prelink#.", strlen(".#prelink#.")) == 0) ++ { ++ log_info("File '%s' seems to be a prelink temporary file", executable); ++ *prelink = '\0'; ++ } ++ return executable; ++} ++ ++char* get_cwd(pid_t pid) ++{ ++ char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3]; ++ sprintf(buf, "/proc/%lu/cwd", (long)pid); ++ return malloc_readlink(buf); ++} ++ ++char* get_rootdir(pid_t pid) ++{ ++ char buf[sizeof("/proc/%lu/root") + sizeof(long)*3]; ++ sprintf(buf, "/proc/%lu/root", (long)pid); ++ return malloc_readlink(buf); ++} ++ ++int get_fsuid(const char *proc_pid_status) ++{ ++ int real, euid, saved; ++ /* if we fail to parse the uid, then make it root only readable to be safe */ ++ int fs_uid = 0; ++ ++ const char *line = proc_pid_status; /* never NULL */ ++ for (;;) ++ { ++ if (strncmp(line, "Uid", 3) == 0) ++ { ++ int n = sscanf(line, "Uid:\t%d\t%d\t%d\t%d\n", &real, &euid, &saved, &fs_uid); ++ if (n != 4) ++ return -1; ++ break; ++ } ++ line = strchr(line, '\n'); ++ if (!line) ++ break; ++ line++; ++ } ++ ++ return fs_uid; ++} ++ ++int dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs) ++{ ++ FILE *fp = fopen(dest_filename, "w"); ++ if (!fp) ++ return 0; ++ ++ /*TODO: BUG: there might be holes as programs can close any fd at any time*/ ++ unsigned fd = 0; ++ while (fd <= 99999) /* paranoia check */ ++ { ++ sprintf(source_filename + source_base_ofs, "fd/%u", fd); ++ char *name = malloc_readlink(source_filename); ++ if (!name) ++ break; ++ fprintf(fp, "%u:%s\n", fd, name); ++ free(name); ++ ++ sprintf(source_filename + source_base_ofs, "fdinfo/%u", fd); ++ fd++; ++ FILE *in = fopen(source_filename, "r"); ++ if (!in) ++ continue; ++ char buf[128]; ++ while (fgets(buf, sizeof(buf)-1, in)) ++ { ++ /* in case the line is not terminated, terminate it */ ++ char *eol = strchrnul(buf, '\n'); ++ eol[0] = '\n'; ++ eol[1] = '\0'; ++ fputs(buf, fp); ++ } ++ fclose(in); ++ } ++ fclose(fp); ++ return 1; ++} +diff --git a/src/lib/read_write.c b/src/lib/read_write.c +index 7ce2097..3f3bd1e 100644 +--- a/src/lib/read_write.c ++++ b/src/lib/read_write.c +@@ -191,3 +191,17 @@ void* xmalloc_xopen_read_close(const char *filename, size_t *maxsz_p) + perror_msg_and_die("Can't read '%s'", filename); + return buf; + } ++ ++char* malloc_readlink(const char *linkname) ++{ ++ char buf[PATH_MAX + 1]; ++ int len; ++ ++ len = readlink(linkname, buf, sizeof(buf)-1); ++ if (len >= 0) ++ { ++ buf[len] = '\0'; ++ return xstrdup(buf); ++ } ++ return NULL; ++} +-- +2.1.0 + diff --git a/0051-dump_dir-add-function-for-creating-a-dump-dir-in-dum.patch b/0051-dump_dir-add-function-for-creating-a-dump-dir-in-dum.patch new file mode 100644 index 0000000..80470a1 --- /dev/null +++ b/0051-dump_dir-add-function-for-creating-a-dump-dir-in-dum.patch @@ -0,0 +1,189 @@ +From ebad4b05134fcf1e7a1a2f4a2f5434ed9284d4e6 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 11 Dec 2014 16:15:35 +0100 +Subject: [PATCH] dump_dir: add function for creating a dump dir in dump + location + +Related to abrt/abrt#548 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 11 +++++ + src/include/problem_data.h | 9 ++++ + src/lib/create_dump_dir.c | 101 +++++++++++++++++++++++++++------------------ + 3 files changed, 81 insertions(+), 40 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index ed4c873..fbe2e31 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -165,6 +165,17 @@ int dump_dir_stat_for_uid(const char *dirname, uid_t uid); + */ + int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason); + ++typedef int (*save_data_call_back)(struct dump_dir *, void *args); ++ ++/* Saves data in a new dump directory ++ * ++ * Creates a new dump directory in "problem dump location", adds the basic ++ * information to the new directory, calls given callback to allow callees to ++ * customize the dump dir contents (save problem data) and commits the dump ++ * directory (makes the directory visible for a problem daemon). ++ */ ++struct dump_dir *create_dump_dir(const char *base_dir_name, const char *type, ++ uid_t uid, save_data_call_back save_data, void *args); + #ifdef __cplusplus + } + #endif +diff --git a/src/include/problem_data.h b/src/include/problem_data.h +index 7a65d6c..661d31e 100644 +--- a/src/include/problem_data.h ++++ b/src/include/problem_data.h +@@ -132,6 +132,15 @@ problem_data_t *create_problem_data_for_reporting(const char *dump_dir_name); + */ + struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name); + ++/** ++ @brief Saves the problem data object in opened dump directory ++ ++ @param dd Dump directory ++ @param problem_data Problem data object to save ++ @return 0 on success; otherwise non-zero value ++ */ ++int save_problem_data_in_dump_dir(struct dump_dir *dd, problem_data_t *problem_data); ++ + #ifdef __cplusplus + } + #endif +diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c +index 4f67523..9f312e0 100644 +--- a/src/lib/create_dump_dir.c ++++ b/src/lib/create_dump_dir.c +@@ -30,36 +30,10 @@ static struct dump_dir *try_dd_create(const char *base_dir_name, const char *dir + return dd; + } + +-struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name) ++struct dump_dir *create_dump_dir(const char *base_dir_name, const char *type, uid_t uid, save_data_call_back save_data, void *args) + { + INITIALIZE_LIBREPORT(); + +- char *type = problem_data_get_content_or_NULL(problem_data, FILENAME_ANALYZER); +- +- if (!type) +- { +- error_msg(_("Missing required item: '%s'"), FILENAME_ANALYZER); +- return NULL; +- } +- +- uid_t uid = (uid_t)-1L; +- char *uid_str = problem_data_get_content_or_NULL(problem_data, FILENAME_UID); +- +- if (uid_str) +- { +- char *endptr; +- errno = 0; +- long val = strtol(uid_str, &endptr, 10); +- +- if (errno != 0 || endptr == uid_str || *endptr != '\0' || INT_MAX < val) +- { +- error_msg(_("uid value is not valid: '%s'"), uid_str); +- return NULL; +- } +- +- uid = (uid_t)val; +- } +- + struct timeval tv; + if (gettimeofday(&tv, NULL) < 0) + { +@@ -99,6 +73,34 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + if (!dd) /* try_dd_create() already emitted the error message */ + goto ret; + ++ if (save_data(dd, args)) ++ { ++ dd_delete(dd); ++ dd = NULL; ++ goto ret; ++ } ++ ++ /* need to create basic files AFTER we save the pd to dump_dir ++ * otherwise we can't skip already created files like in case when ++ * reporting from anaconda where we can't read /etc/{system,redhat}-release ++ * and os_release is taken from anaconda ++ */ ++ dd_create_basic_files(dd, uid, NULL); ++ ++ problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0'; ++ char* new_path = concat_path_file(base_dir_name, problem_id); ++ log_info("Renaming from '%s' to '%s'", dd->dd_dirname, new_path); ++ dd_rename(dd, new_path); ++ ++ ret: ++ free(problem_id); ++ return dd; ++} ++ ++int save_problem_data_in_dump_dir(struct dump_dir *dd, problem_data_t *problem_data) ++{ ++ INITIALIZE_LIBREPORT(); ++ + GHashTableIter iter; + char *name; + struct problem_item *value; +@@ -129,19 +131,38 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + dd_save_text(dd, name, value->content); + } + +- /* need to create basic files AFTER we save the pd to dump_dir +- * otherwise we can't skip already created files like in case when +- * reporting from anaconda where we can't read /etc/{system,redhat}-release +- * and os_release is taken from anaconda +- */ +- dd_create_basic_files(dd, uid, NULL); ++ return 0; ++} + +- problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0'; +- char* new_path = concat_path_file(base_dir_name, problem_id); +- log_info("Renaming from '%s' to '%s'", dd->dd_dirname, new_path); +- dd_rename(dd, new_path); ++struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name) ++{ ++ INITIALIZE_LIBREPORT(); + +- ret: +- free(problem_id); +- return dd; ++ char *type = problem_data_get_content_or_NULL(problem_data, FILENAME_ANALYZER); ++ ++ if (!type) ++ { ++ error_msg(_("Missing required item: '%s'"), FILENAME_ANALYZER); ++ return NULL; ++ } ++ ++ uid_t uid = (uid_t)-1L; ++ char *uid_str = problem_data_get_content_or_NULL(problem_data, FILENAME_UID); ++ ++ if (uid_str) ++ { ++ char *endptr; ++ errno = 0; ++ long val = strtol(uid_str, &endptr, 10); ++ ++ if (errno != 0 || endptr == uid_str || *endptr != '\0' || INT_MAX < val) ++ { ++ error_msg(_("uid value is not valid: '%s'"), uid_str); ++ return NULL; ++ } ++ ++ uid = (uid_t)val; ++ } ++ ++ return create_dump_dir(base_dir_name, type, uid, (save_data_call_back)save_problem_data_in_dump_dir, problem_data); + } +-- +2.1.0 + diff --git a/0052-Whitespace-fix.patch b/0052-Whitespace-fix.patch new file mode 100644 index 0000000..bb23775 --- /dev/null +++ b/0052-Whitespace-fix.patch @@ -0,0 +1,25 @@ +From 96e8155c15b9e07302ecabaa1f30b5eb4dedceec Mon Sep 17 00:00:00 2001 +From: Martin Milata +Date: Tue, 20 Jan 2015 16:51:55 +0100 +Subject: [PATCH] Whitespace fix + +Signed-off-by: Martin Milata +--- + src/lib/dump_dir.c | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 15e7dcd..6c05916 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -244,7 +244,6 @@ int create_symlink_lockfile(const char* lock_file, const char* pid) + } + + static const char *dd_check(struct dump_dir *dd) +- + { + unsigned dirname_len = strlen(dd->dd_dirname); + char filename_buf[FILENAME_MAX+1]; +-- +2.1.0 + diff --git a/0053-dump_dir-introduce-dd_copy_file.patch b/0053-dump_dir-introduce-dd_copy_file.patch new file mode 100644 index 0000000..173ea3f --- /dev/null +++ b/0053-dump_dir-introduce-dd_copy_file.patch @@ -0,0 +1,74 @@ +From c47224c6d911815643e4eb2c4840f2305bf14e1b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 12 Dec 2014 09:17:59 +0100 +Subject: [PATCH] dump_dir: introduce dd_copy_file() + +Related to abrt/abrt#548 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 1 + + src/lib/create_dump_dir.c | 10 +--------- + src/lib/dump_dir.c | 16 ++++++++++++++++ + 3 files changed, 18 insertions(+), 9 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index fbe2e31..3a5ab36 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -82,6 +82,7 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla + char* dd_load_text(const struct dump_dir *dd, const char *name); + void dd_save_text(struct dump_dir *dd, const char *name, const char *data); + void dd_save_binary(struct dump_dir *dd, const char *name, const char *data, unsigned size); ++int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path); + /* Returns value less than 0 if any error occured; otherwise returns size of an + * item in Bytes. If an item does not exist returns 0 instead of an error + * value. +diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c +index 9f312e0..6aee370 100644 +--- a/src/lib/create_dump_dir.c ++++ b/src/lib/create_dump_dir.c +@@ -109,15 +109,7 @@ int save_problem_data_in_dump_dir(struct dump_dir *dd, problem_data_t *problem_d + { + if (value->flags & CD_FLAG_BIN) + { +- char *dest = concat_path_file(dd->dd_dirname, name); +- log_info("copying '%s' to '%s'", value->content, dest); +- off_t copied = copy_file(value->content, dest, DEFAULT_DUMP_DIR_MODE | S_IROTH); +- if (copied < 0) +- error_msg("Can't copy %s to %s", value->content, dest); +- else +- log_info("copied %li bytes", (unsigned long)copied); +- free(dest); +- ++ dd_copy_file(dd, name, value->content); + continue; + } + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 6c05916..0dea02e 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -1298,3 +1298,19 @@ int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason) + dd_save_text(dd, FILENAME_NOT_REPORTABLE, reason); + return 0; + } ++ ++int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) ++{ ++ char *dest = concat_path_file(dd->dd_dirname, name); ++ ++ log_debug("copying '%s' to '%s'", source_path, dest); ++ ++ off_t copied = copy_file(source_path, dest, DEFAULT_DUMP_DIR_MODE | S_IROTH); ++ if (copied < 0) ++ error_msg("Can't copy %s to %s", source_path, dest); ++ else ++ log_debug("copied %li bytes", (unsigned long)copied); ++ ++ free(dest); ++ return copied < 0; ++} +-- +2.1.0 + diff --git a/0054-dump_dir-introduce-dd_copy_file_unpack.patch b/0054-dump_dir-introduce-dd_copy_file_unpack.patch new file mode 100644 index 0000000..e842c6a --- /dev/null +++ b/0054-dump_dir-introduce-dd_copy_file_unpack.patch @@ -0,0 +1,272 @@ +From 48ac9fdcc9d1e12a669d8664eeef1ed08c0f3969 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 21 Jan 2015 07:15:45 +0100 +Subject: [PATCH] dump_dir: introduce dd_copy_file_unpack() + +Related to abrt/abrt#548 + +Signed-off-by: Jakub Filak +--- + configure.ac | 1 + + src/include/dump_dir.h | 1 + + src/include/internal_libreport.h | 5 ++ + src/lib/Makefile.am | 3 + + src/lib/compress.c | 152 +++++++++++++++++++++++++++++++++++++++ + src/lib/dump_dir.c | 17 +++++ + 6 files changed, 179 insertions(+) + create mode 100644 src/lib/compress.c + +diff --git a/configure.ac b/configure.ac +index c925513..01d9f83 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -189,6 +189,7 @@ PKG_CHECK_MODULES([PROXY], [libproxy-1.0], [ + PKG_CHECK_MODULES([SATYR], [satyr]) + PKG_CHECK_MODULES([JOURNAL], [libsystemd-journal]) + PKG_CHECK_MODULES([AUGEAS], [augeas]) ++PKG_CHECK_MODULES([LZMA], [liblzma]) + + + AC_ARG_WITH(newt, +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index 3a5ab36..fcd57c6 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -83,6 +83,7 @@ char* dd_load_text(const struct dump_dir *dd, const char *name); + void dd_save_text(struct dump_dir *dd, const char *name, const char *data); + void dd_save_binary(struct dump_dir *dd, const char *name, const char *data, unsigned size); + int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path); ++int dd_copy_file_unpack(struct dump_dir *dd, const char *name, const char *source_path); + /* Returns value less than 0 if any error occured; otherwise returns size of an + * item in Bytes. If an item does not exist returns 0 instead of an error + * value. +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index fca8138..7cda793 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -158,6 +158,11 @@ off_t copy_file(const char *src_name, const char *dst_name, int mode); + #define copy_file_recursive libreport_copy_file_recursive + int copy_file_recursive(const char *source, const char *dest); + ++#define decompress_fd libreport_decompress_fd ++int decompress_fd(int fdi, int fdo); ++#define decompress_file libreport_decompress_file ++int decompress_file(const char *path_in, const char *path_out, mode_t mode_out); ++ + // NB: will return short read on error, not -1, + // if some data was read before error occurred + #define xread libreport_xread +diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am +index 2ec3f5a..4964da7 100644 +--- a/src/lib/Makefile.am ++++ b/src/lib/Makefile.am +@@ -20,6 +20,7 @@ libreport_la_SOURCES = \ + logging.c \ + copyfd.c \ + copy_file_recursive.c \ ++ compress.c \ + concat_path_file.c \ + append_to_malloced_string.c \ + overlapping_strcpy.c \ +@@ -73,6 +74,7 @@ libreport_la_CPPFLAGS = \ + -DDUMP_DIR_OWNED_BY_USER=$(DUMP_DIR_OWNED_BY_USER) \ + -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \ + $(GLIB_CFLAGS) \ ++ $(LZMA_CFLAGS) \ + $(GOBJECT_CFLAGS) \ + $(AUGEAS_CFLAGS) \ + -D_GNU_SOURCE +@@ -80,6 +82,7 @@ libreport_la_LDFLAGS = \ + -version-info 0:1:0 + libreport_la_LIBADD = \ + $(GLIB_LIBS) \ ++ $(LZMA_LIBS) \ + $(JOURNAL_LIBS) \ + $(GOBJECT_LIBS) \ + $(AUGEAS_LIBS) +diff --git a/src/lib/compress.c b/src/lib/compress.c +new file mode 100644 +index 0000000..eec155a +--- /dev/null ++++ b/src/lib/compress.c +@@ -0,0 +1,152 @@ ++/* ++ Copyright (C) 2015 ABRT team ++ Copyright (C) 2015 RedHat Inc ++ ++ This program is free software; you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation; either version 2 of the License, or ++ (at your option) any later version. ++ ++ This program is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ 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. ++*/ ++#include "internal_libreport.h" ++ ++#include ++ ++static const uint8_t s_xz_magic[6] = { 0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00 }; ++ ++static bool ++is_format(const char *name, const uint8_t *header, size_t hl, const uint8_t *magic, size_t ml) ++{ ++ if (hl < ml) ++ { ++ log_warning("Too short header to detect '%s' file format.", name); ++ return false; ++ } ++ ++ return memcmp(header, magic, ml) == 0; ++} ++ ++static int ++decompress_fd_xz(int fdi, int fdo) ++{ ++ uint8_t buf_in[BUFSIZ]; ++ uint8_t buf_out[BUFSIZ]; ++ ++ lzma_stream strm = LZMA_STREAM_INIT; ++ lzma_ret ret = lzma_stream_decoder(&strm, UINT64_MAX, 0); ++ if (ret != LZMA_OK) ++ { ++ close(fdi); ++ close(fdo); ++ log_error("Failed to initialize XZ decoder: code %d", ret); ++ return -ENOMEM; ++ } ++ ++ lzma_action action = LZMA_RUN; ++ ++ strm.next_out = buf_out; ++ strm.avail_out = sizeof(buf_out); ++ ++ for (;;) ++ { ++ if (strm.avail_in == 0 && action == LZMA_RUN) ++ { ++ strm.next_in = buf_in; ++ strm.avail_in = safe_read(fdi, buf_in, sizeof(buf_in)); ++ ++ if (strm.avail_in < 0) ++ { ++ perror_msg("Failed to read source core file"); ++ close(fdi); ++ close(fdo); ++ lzma_end(&strm); ++ return -1; ++ } ++ ++ if (strm.avail_in == 0) ++ action = LZMA_FINISH; ++ } ++ ++ ret = lzma_code(&strm, action); ++ ++ if (strm.avail_out == 0 || ret == LZMA_STREAM_END) ++ { ++ const ssize_t n = sizeof(buf_out) - strm.avail_out; ++ if (n != safe_write(fdo, buf_out, n)) ++ { ++ perror_msg("Failed to write decompressed data"); ++ close(fdi); ++ close(fdo); ++ lzma_end(&strm); ++ return -1; ++ } ++ ++ if (ret == LZMA_STREAM_END) ++ { ++ log_debug("Successfully decompressed coredump."); ++ break; ++ } ++ ++ strm.next_out = buf_out; ++ strm.avail_out = sizeof(buf_out); ++ } ++ } ++ ++ return 0; ++} ++ ++int ++decompress_fd(int fdi, int fdo) ++{ ++ uint8_t header[6]; ++ ++ if (sizeof(header) != safe_read(fdi, header, sizeof(header))) ++ { ++ perror_msg("Failed to read header bytes"); ++ return -1; ++ } ++ ++ xlseek(fdi, 0, SEEK_SET); ++ ++ if (is_format("xz", header, sizeof(header), s_xz_magic, sizeof(s_xz_magic))) ++ return decompress_fd_xz(fdi, fdo); ++ ++ error_msg("Unsupported file format"); ++ return -1; ++} ++ ++int ++decompress_file(const char *path_in, const char *path_out, mode_t mode_out) ++{ ++ int fdi = open(path_in, O_RDONLY | O_CLOEXEC); ++ if (fdi < 0) ++ { ++ perror_msg("Could not open file: %s", path_in); ++ return -1; ++ } ++ ++ int fdo = open(path_out, O_WRONLY | O_CLOEXEC | O_EXCL | O_CREAT, mode_out); ++ if (fdo < 0) ++ { ++ close(fdi); ++ perror_msg("Could not create file: %s", path_out); ++ return -1; ++ } ++ ++ int ret = decompress_fd(fdi, fdo); ++ close(fdi); ++ close(fdo); ++ ++ if (ret != 0) ++ unlink(path_out); ++ ++ return ret; ++} +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 0dea02e..d50ebf7 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -1314,3 +1314,20 @@ int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + free(dest); + return copied < 0; + } ++ ++int dd_copy_file_unpack(struct dump_dir *dd, const char *name, const char *source_path) ++{ ++ char *dest = concat_path_file(dd->dd_dirname, name); ++ ++ log_debug("unpacking '%s' to '%s'", source_path, dest); ++ ++ off_t copied = decompress_file(source_path, dest, DEFAULT_DUMP_DIR_MODE | S_IROTH); ++ if (copied != 0) ++ error_msg("Can't copy %s to %s", source_path, dest); ++ else ++ log_debug("unpackaged file '%s'", dest); ++ ++ free(dest); ++ return copied < 0; ++ ++} +-- +2.1.0 + diff --git a/0056-rewrite-dump_fd_info.patch b/0056-rewrite-dump_fd_info.patch new file mode 100644 index 0000000..28d9e25 --- /dev/null +++ b/0056-rewrite-dump_fd_info.patch @@ -0,0 +1,187 @@ +From c16ef7b59b8327332b4252a9c7e70616c363b868 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 21 Jan 2015 09:37:30 +0100 +Subject: [PATCH] rewrite dump_fd_info() + +Now, the function is same as from systemd coredump's function compose_open_fds() + +Closes #320 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 4 +- + src/lib/get_cmdline.c | 105 ++++++++++++++++++++++++++++++--------- + src/lib/read_write.c | 7 ++- + 3 files changed, 90 insertions(+), 26 deletions(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 7cda793..00ff7a1 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -185,6 +185,8 @@ void* xmalloc_open_read_close(const char *filename, size_t *maxsz_p); + void* xmalloc_xopen_read_close(const char *filename, size_t *maxsz_p); + #define malloc_readlink libreport_malloc_readlink + char* malloc_readlink(const char *linkname); ++#define malloc_readlinkat libreport_malloc_readlinkat ++char* malloc_readlinkat(int dir_fd, const char *linkname); + + + /* Returns malloc'ed block */ +@@ -641,7 +643,7 @@ char* get_rootdir(pid_t pid); + #define get_fsuid libreport_get_fsuid + int get_fsuid(const char *proc_pid_status); + #define dump_fd_info libreport_dump_fd_info +-int dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs); ++int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path); + + /* Takes ptr to time_t, or NULL if you want to use current time. + * Returns "YYYY-MM-DD-hh:mm:ss" string. +diff --git a/src/lib/get_cmdline.c b/src/lib/get_cmdline.c +index 7ceba2f..2e362c5 100644 +--- a/src/lib/get_cmdline.c ++++ b/src/lib/get_cmdline.c +@@ -213,39 +213,96 @@ int get_fsuid(const char *proc_pid_status) + return fs_uid; + } + +-int dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs) ++int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path) + { +- FILE *fp = fopen(dest_filename, "w"); +- if (!fp) +- return 0; ++ DIR *proc_fd_dir = NULL; ++ int proc_fdinfo_fd = -1; ++ char *buffer = NULL; ++ FILE *stream = NULL; ++ const char *fddelim = ""; ++ struct dirent *dent = NULL; ++ int r = 0; + +- /*TODO: BUG: there might be holes as programs can close any fd at any time*/ +- unsigned fd = 0; +- while (fd <= 99999) /* paranoia check */ ++ proc_fd_dir = opendir(proc_pid_fd_path); ++ if (!proc_fd_dir) + { +- sprintf(source_filename + source_base_ofs, "fd/%u", fd); +- char *name = malloc_readlink(source_filename); +- if (!name) +- break; +- fprintf(fp, "%u:%s\n", fd, name); +- free(name); ++ r = -errno; ++ goto dumpfd_cleanup; ++ } + +- sprintf(source_filename + source_base_ofs, "fdinfo/%u", fd); +- fd++; +- FILE *in = fopen(source_filename, "r"); +- if (!in) ++ proc_fdinfo_fd = openat(dirfd(proc_fd_dir), "../fdinfo", O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC|O_PATH); ++ if (proc_fdinfo_fd < 0) ++ { ++ r = -errno; ++ goto dumpfd_cleanup; ++ } ++ ++ stream = fopen(dest_filename, "w"); ++ if (!stream) ++ { ++ r = -ENOMEM; ++ goto dumpfd_cleanup; ++ } ++ ++ while (1) ++ { ++ errno = 0; ++ dent = readdir(proc_fd_dir); ++ if (dent == NULL) ++ { ++ if (errno > 0) ++ { ++ r = -errno; ++ goto dumpfd_cleanup; ++ } ++ break; ++ } ++ else if (dot_or_dotdot(dent->d_name)) + continue; +- char buf[128]; +- while (fgets(buf, sizeof(buf)-1, in)) ++ ++ FILE *fdinfo = NULL; ++ char *fdname = NULL; ++ char line[LINE_MAX]; ++ int fd; ++ ++ fdname = malloc_readlinkat(dirfd(proc_fd_dir), dent->d_name); ++ ++ fprintf(stream, "%s%s:%s\n", fddelim, dent->d_name, fdname); ++ fddelim = "\n"; ++ ++ /* Use the directory entry from /proc/[pid]/fd with /proc/[pid]/fdinfo */ ++ fd = openat(proc_fdinfo_fd, dent->d_name, O_NOFOLLOW|O_CLOEXEC|O_RDONLY); ++ if (fd < 0) ++ goto dumpfd_next_fd; ++ ++ fdinfo = fdopen(fd, "re"); ++ if (fdinfo == NULL) ++ goto dumpfd_next_fd; ++ ++ while (fgets(line, sizeof(line)-1, fdinfo)) + { + /* in case the line is not terminated, terminate it */ +- char *eol = strchrnul(buf, '\n'); ++ char *eol = strchrnul(line, '\n'); + eol[0] = '\n'; + eol[1] = '\0'; +- fputs(buf, fp); ++ fputs(line, stream); + } +- fclose(in); ++ ++dumpfd_next_fd: ++ fclose(fdinfo); ++ free(fdname); + } +- fclose(fp); +- return 1; ++ ++dumpfd_cleanup: ++ errno = 0; ++ fclose(stream); ++ ++ if (r == 0 && errno != 0) ++ r = -errno; ++ ++ closedir(proc_fd_dir); ++ close(proc_fdinfo_fd); ++ free(buffer); ++ ++ return r; + } +diff --git a/src/lib/read_write.c b/src/lib/read_write.c +index 3f3bd1e..657adb0 100644 +--- a/src/lib/read_write.c ++++ b/src/lib/read_write.c +@@ -194,10 +194,15 @@ void* xmalloc_xopen_read_close(const char *filename, size_t *maxsz_p) + + char* malloc_readlink(const char *linkname) + { ++ return malloc_readlinkat(AT_FDCWD, linkname); ++} ++ ++char* malloc_readlinkat(int dir_fd, const char *linkname) ++{ + char buf[PATH_MAX + 1]; + int len; + +- len = readlink(linkname, buf, sizeof(buf)-1); ++ len = readlinkat(dir_fd, linkname, buf, sizeof(buf)-1); + if (len >= 0) + { + buf[len] = '\0'; +-- +2.1.0 + diff --git a/0057-disable-gdk-deprecation-warnings.patch b/0057-disable-gdk-deprecation-warnings.patch new file mode 100644 index 0000000..938f2d9 --- /dev/null +++ b/0057-disable-gdk-deprecation-warnings.patch @@ -0,0 +1,38 @@ +From 468dc56a6ee3c16eacfa2cdd63295b03fe1ba98d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 21 Jan 2015 17:38:05 +0100 +Subject: [PATCH] disable gdk deprecation warnings + +Signed-off-by: Jakub Filak +--- + src/gtk-helpers/Makefile.am | 1 + + src/gui-wizard-gtk/Makefile.am | 1 + + 2 files changed, 2 insertions(+) + +diff --git a/src/gtk-helpers/Makefile.am b/src/gtk-helpers/Makefile.am +index a7cc554..80f1981 100644 +--- a/src/gtk-helpers/Makefile.am ++++ b/src/gtk-helpers/Makefile.am +@@ -31,6 +31,7 @@ libreport_gtk_la_CPPFLAGS = \ + $(GLIB_CFLAGS) \ + $(GIO_CFLAGS) \ + -DWORKFLOWS_DIR=\"$(WORKFLOWS_DIR)\" \ ++ -DGDK_DISABLE_DEPRECATION_WARNINGS \ + -D_GNU_SOURCE + libreport_gtk_la_LDFLAGS = \ + -version-info 0:1:0 +diff --git a/src/gui-wizard-gtk/Makefile.am b/src/gui-wizard-gtk/Makefile.am +index 37e7ba6..fce33cb 100644 +--- a/src/gui-wizard-gtk/Makefile.am ++++ b/src/gui-wizard-gtk/Makefile.am +@@ -22,6 +22,7 @@ report_gtk_CFLAGS = \ + -DICON_DIR=\"${datadir}/abrt/icons/hicolor/48x48/status\" \ + -DWORKFLOWS_DIR=\"$(WORKFLOWS_DIR)\" \ + -DLIBEXEC_DIR=\"$(libexecdir)\" \ ++ -DGDK_DISABLE_DEPRECATION_WARNINGS \ + $(GLIB_CFLAGS) \ + $(GTK_CFLAGS) \ + -D_GNU_SOURCE +-- +2.1.0 + diff --git a/0058-fix-several-memory-leaks.patch b/0058-fix-several-memory-leaks.patch new file mode 100644 index 0000000..dc0bc2c --- /dev/null +++ b/0058-fix-several-memory-leaks.patch @@ -0,0 +1,149 @@ +From e1cdcccab0a3861d0c465cb4b75fede605a7ec17 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Sun, 25 Jan 2015 14:19:43 +0100 +Subject: [PATCH] fix several memory leaks + +I discovered these leaks while chasing a bug related to rhbz#1163579. + +The most interesting one is the missing semicolon in src/lib/file_obj.c + +Signed-off-by: Jakub Filak +--- + src/gtk-helpers/problem_details_widget.c | 1 + + src/gui-wizard-gtk/main.c | 7 ++++--- + src/gui-wizard-gtk/wizard.c | 10 +++++++--- + src/lib/file_obj.c | 4 ++-- + src/lib/workflow.c | 4 +++- + 5 files changed, 17 insertions(+), 9 deletions(-) + +diff --git a/src/gtk-helpers/problem_details_widget.c b/src/gtk-helpers/problem_details_widget.c +index c314d4d..fed65b2 100644 +--- a/src/gtk-helpers/problem_details_widget.c ++++ b/src/gtk-helpers/problem_details_widget.c +@@ -305,6 +305,7 @@ problem_details_widget_populate(ProblemDetailsWidget *self) + line = xasprintf("%s", uid ? uid : username); + + problem_details_widget_add_single_line(self, "user", line); ++ free(line); + } + + { /* Type/Analyzer: CCpp */ +diff --git a/src/gui-wizard-gtk/main.c b/src/gui-wizard-gtk/main.c +index 41a8089..9780c8d 100644 +--- a/src/gui-wizard-gtk/main.c ++++ b/src/gui-wizard-gtk/main.c +@@ -195,9 +195,9 @@ int main(int argc, char **argv) + load_workflow_config_data(WORKFLOWS_DIR); + + /* list of workflows applicable to the currently processed problem */ +- GHashTable *possible_workflows = load_workflow_config_data_from_list( +- list_possible_events_glist(g_dump_dir_name, "workflow"), +- WORKFLOWS_DIR); ++ GList *possible_names = list_possible_events_glist(g_dump_dir_name, "workflow"); ++ GHashTable *possible_workflows = load_workflow_config_data_from_list(possible_names, WORKFLOWS_DIR); ++ g_list_free_full(possible_names, free); + + /* if we have only 1 workflow, we can use the events from it as default */ + if (!expert_mode && g_auto_event_list == NULL && g_hash_table_size(possible_workflows) == 1) +@@ -212,6 +212,7 @@ int main(int argc, char **argv) + g_auto_event_list = wf_get_event_names((workflow_t *)value); + } + } ++ g_hash_table_destroy(possible_workflows); + + problem_data_reload_from_dump_dir(); + +diff --git a/src/gui-wizard-gtk/wizard.c b/src/gui-wizard-gtk/wizard.c +index ab6123f..513f0a3 100644 +--- a/src/gui-wizard-gtk/wizard.c ++++ b/src/gui-wizard-gtk/wizard.c +@@ -2424,14 +2424,14 @@ static bool highligh_words_in_textview(int page, GtkTextView *tev, GList *words, + SEARCH_COLUMN_ITEM, &word, + -1); + ++ free(text); ++ + if (word->buffer == buffer) + { + buffer_removing = true; + + valid = gtk_list_store_remove(g_ls_sensitive_list, &iter); + +- free(text); +- + if (word == g_current_highlighted_word) + g_current_highlighted_word = NULL; + +@@ -2794,9 +2794,12 @@ static void add_workflow_buttons(GtkBox *box, GHashTable *workflows, GCallback f + { + gtk_container_foreach(GTK_CONTAINER(box), &remove_child_widget, NULL); + ++ GList *possible_workflows = list_possible_events_glist(g_dump_dir_name, "workflow"); + GHashTable *workflow_table = load_workflow_config_data_from_list( +- list_possible_events_glist(g_dump_dir_name, "workflow"), ++ possible_workflows, + WORKFLOWS_DIR); ++ g_list_free_full(possible_workflows, free); ++ g_object_set_data_full(G_OBJECT(box), "workflows", workflow_table, (GDestroyNotify)g_hash_table_destroy); + + GList *wf_list = g_hash_table_get_values(workflow_table); + wf_list = g_list_sort(wf_list, (GCompareFunc)wf_priority_compare); +@@ -2817,6 +2820,7 @@ static void add_workflow_buttons(GtkBox *box, GHashTable *workflows, GCallback f + gtk_widget_set_margin_start(label, 40); + #endif + gtk_widget_set_margin_bottom(label, 10); ++ g_list_free(children); + free(btn_label); + g_signal_connect(button, "clicked", func, w); + gtk_box_pack_start(box, button, true, false, 2); +diff --git a/src/lib/file_obj.c b/src/lib/file_obj.c +index bbb5b9f..0821155 100644 +--- a/src/lib/file_obj.c ++++ b/src/lib/file_obj.c +@@ -30,7 +30,7 @@ file_obj_t *new_file_obj(const char* fullpath, const char* filename) + void free_file_obj(file_obj_t *f) + { + if (f == NULL) +- return ++ return; + + free(f->fullpath); + free(f->filename); +@@ -45,4 +45,4 @@ const char *fo_get_fullpath(file_obj_t *fo) + const char *fo_get_filename(file_obj_t *fo) + { + return fo->filename; +-} +\ No newline at end of file ++} +diff --git a/src/lib/workflow.c b/src/lib/workflow.c +index c6eedf4..a985834 100644 +--- a/src/lib/workflow.c ++++ b/src/lib/workflow.c +@@ -90,7 +90,7 @@ static void load_workflow_config(const char *name, + workflow_t *workflow = new_workflow(file->filename); + load_workflow_description_from_file(workflow, file->fullpath); + log_notice("Adding '%s' to workflows\n", file->filename); +- g_hash_table_insert(wf_list, file->filename, workflow); ++ g_hash_table_insert(wf_list, xstrdup(file->filename), workflow); + } + } + +@@ -104,12 +104,14 @@ GHashTable *load_workflow_config_data_from_list(GList *wf_names, + g_free, + (GDestroyNotify) free_workflow + ); ++ + GList *workflow_files = get_file_list(path, "xml"); + while(wfs) + { + load_workflow_config((const char *)wfs->data, workflow_files, wf_list); + wfs = g_list_next(wfs); + } ++ free_file_list(workflow_files); + + return wf_list; + } +-- +2.1.0 + diff --git a/0059-utils-make-arguments-of-a-list-func-const.patch b/0059-utils-make-arguments-of-a-list-func-const.patch new file mode 100644 index 0000000..5d5a03e --- /dev/null +++ b/0059-utils-make-arguments-of-a-list-func-const.patch @@ -0,0 +1,153 @@ +From ecbc26cd7ceb416db501aab405adca75523bb97b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 29 Jan 2015 11:24:39 +0100 +Subject: [PATCH] utils: make arguments of a list func const + +Related to #316 + +Signed-off-by: Jakub Filak +--- + src/gtk-helpers/problem_details_widget.c | 6 +++--- + src/include/internal_libreport.h | 4 ++-- + src/lib/is_in_string_list.c | 4 ++-- + src/lib/make_descr.c | 6 +++--- + src/lib/problem_data.c | 8 ++++---- + 5 files changed, 14 insertions(+), 14 deletions(-) + +diff --git a/src/gtk-helpers/problem_details_widget.c b/src/gtk-helpers/problem_details_widget.c +index fed65b2..d8d78e9 100644 +--- a/src/gtk-helpers/problem_details_widget.c ++++ b/src/gtk-helpers/problem_details_widget.c +@@ -232,7 +232,7 @@ static void + problem_data_entry_to_grid_row_one_line(const char *item_name, problem_item *item, ProblemDetailsWidget *self) + { + if (((item->flags & CD_FLAG_TXT) && (strchr(item->content, '\n') == NULL)) +- && !is_in_string_list(item_name, (char **)items_auto_blacklist)) ++ && !is_in_string_list(item_name, items_auto_blacklist)) + problem_details_widget_add_single_line(self, item_name, item->content); + } + +@@ -240,7 +240,7 @@ static void + problem_data_entry_to_grid_row_multi_line(const char *item_name, problem_item *item, ProblemDetailsWidget *self) + { + if (((item->flags & CD_FLAG_TXT) && (strchr(item->content, '\n') != NULL)) +- && !is_in_string_list(item_name, (char **)items_auto_blacklist)) ++ && !is_in_string_list(item_name, items_auto_blacklist)) + problem_details_widget_add_multi_line(self, item_name, item->content); + } + +@@ -248,7 +248,7 @@ static void + problem_data_entry_to_grid_row_binary(const char *item_name, problem_item *item, ProblemDetailsWidget *self) + { + if ((item->flags & CD_FLAG_BIN) +- && !is_in_string_list(item_name, (char **)items_auto_blacklist)) ++ && !is_in_string_list(item_name, items_auto_blacklist)) + problem_details_widget_add_binary(self, item_name, item->content); + } + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 00ff7a1..72ff240 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -294,10 +294,10 @@ char *run_in_shell_and_save_output(int flags, + /* Random utility functions */ + + #define is_in_string_list libreport_is_in_string_list +-bool is_in_string_list(const char *name, char **v); ++bool is_in_string_list(const char *name, const char *const *v); + + #define index_of_string_in_list libreport_index_of_string_in_list +-int index_of_string_in_list(const char *name, char **v); ++int index_of_string_in_list(const char *name, const char *const *v); + + #define is_in_comma_separated_list libreport_is_in_comma_separated_list + bool is_in_comma_separated_list(const char *value, const char *list); +diff --git a/src/lib/is_in_string_list.c b/src/lib/is_in_string_list.c +index e0ee26b..b75abe4 100644 +--- a/src/lib/is_in_string_list.c ++++ b/src/lib/is_in_string_list.c +@@ -18,7 +18,7 @@ + */ + #include "internal_libreport.h" + +-bool is_in_string_list(const char *name, char **v) ++bool is_in_string_list(const char *name, const char *const *v) + { + while (*v) + { +@@ -29,7 +29,7 @@ bool is_in_string_list(const char *name, char **v) + return false; + } + +-int index_of_string_in_list(const char *name, char **v) ++int index_of_string_in_list(const char *name, const char *const *v) + { + for(int i = 0; v[i]; ++i) + { +diff --git a/src/lib/make_descr.c b/src/lib/make_descr.c +index 2bcbebd..912b87f 100644 +--- a/src/lib/make_descr.c ++++ b/src/lib/make_descr.c +@@ -20,7 +20,7 @@ + + static bool rejected_name(const char *name, char **v, int flags) + { +- bool r = is_in_string_list(name, v); ++ bool r = is_in_string_list(name, (const char *const *)v); + if (flags & MAKEDESC_WHITELIST) + r = !r; + return r; +@@ -59,8 +59,8 @@ static int list_cmp(const char *s1, const char *s2) + FILENAME_COUNT , + NULL + }; +- int s1_index = index_of_string_in_list(s1, (char**) list_order); +- int s2_index = index_of_string_in_list(s2, (char**) list_order); ++ int s1_index = index_of_string_in_list(s1, list_order); ++ int s2_index = index_of_string_in_list(s2, list_order); + + if(s1_index < 0 && s2_index < 0) + return strcmp(s1, s2); +diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c +index c57e57f..6b8bb01 100644 +--- a/src/lib/problem_data.c ++++ b/src/lib/problem_data.c +@@ -267,7 +267,7 @@ static const char *const editable_files[] = { + }; + static bool is_editable_file(const char *file_name) + { +- return is_in_string_list(file_name, (char**)editable_files); ++ return is_in_string_list(file_name, editable_files); + } + + /* When is_text_file() returns this special pointer value, +@@ -317,7 +317,7 @@ static char* is_text_file(const char *name, ssize_t *sz) + if (base) + { + base++; +- if (is_in_string_list(base, (char**)always_text_files)) ++ if (is_in_string_list(base, always_text_files)) + goto text; + } + +@@ -387,7 +387,7 @@ void problem_data_load_from_dump_dir(problem_data_t *problem_data, struct dump_d + dd_init_next_file(dd); + while (dd_get_next_file(dd, &short_name, &full_name)) + { +- if (excluding && is_in_string_list(short_name, excluding)) ++ if (excluding && is_in_string_list(short_name, (const char *const *)excluding)) + { + //log("Excluded:'%s'", short_name); + goto next; +@@ -458,7 +458,7 @@ void problem_data_load_from_dump_dir(problem_data_t *problem_data, struct dump_d + FILENAME_REASON , + NULL + }; +- if (is_in_string_list(short_name, (char**)list_files)) ++ if (is_in_string_list(short_name, list_files)) + flags |= CD_FLAG_LIST; + + if (strcmp(short_name, FILENAME_TIME) == 0) +-- +2.1.0 + diff --git a/0060-includes-move-stdbool.h-to-libreport_types.h.patch b/0060-includes-move-stdbool.h-to-libreport_types.h.patch new file mode 100644 index 0000000..41d01d2 --- /dev/null +++ b/0060-includes-move-stdbool.h-to-libreport_types.h.patch @@ -0,0 +1,40 @@ +From 02d45bde212d5fc6b1fe6b6d0d3940a76546287c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 29 Jan 2015 11:28:01 +0100 +Subject: [PATCH] includes: move stdbool.h to libreport_types.h + +Related to #316 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 1 - + src/include/libreport_types.h | 1 + + 2 files changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 72ff240..11c18d9 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -45,7 +45,6 @@ + #include + #include + #include +-#include + /* Try to pull in PATH_MAX */ + #include + #include +diff --git a/src/include/libreport_types.h b/src/include/libreport_types.h +index 41ed5d7..9eeeb93 100644 +--- a/src/include/libreport_types.h ++++ b/src/include/libreport_types.h +@@ -19,6 +19,7 @@ + #ifndef LIBREPORT_TYPES_H_ + #define LIBREPORT_TYPES_H_ + ++#include + #include + + typedef gchar **string_vector_ptr_t; +-- +2.1.0 + diff --git a/0061-ignored-words-ignore-lxqt-openssh-askpass.patch b/0061-ignored-words-ignore-lxqt-openssh-askpass.patch new file mode 100644 index 0000000..8693486 --- /dev/null +++ b/0061-ignored-words-ignore-lxqt-openssh-askpass.patch @@ -0,0 +1,26 @@ +From 5a063069b206b9f1630f6fece2e5c42b0605b984 Mon Sep 17 00:00:00 2001 +From: Lubomir Rintel +Date: Tue, 3 Mar 2015 21:40:54 +0100 +Subject: [PATCH] ignored words: ignore lxqt-openssh-askpass + +This was seen in the environment: +SSH_ASKPASS=/usr/libexec/openssh/lxqt-openssh-askpass +--- + src/gui-wizard-gtk/ignored_words.conf | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/gui-wizard-gtk/ignored_words.conf b/src/gui-wizard-gtk/ignored_words.conf +index 19578b4..c9b9605 100644 +--- a/src/gui-wizard-gtk/ignored_words.conf ++++ b/src/gui-wizard-gtk/ignored_words.conf +@@ -49,6 +49,7 @@ libpciaccess.so + libsecret + login.so + libkeyutils ++lxqt-openssh-askpass + PassOwnPtr + PassRefPtr + passed +-- +2.1.0 + diff --git a/0062-problem_data-add-a-new-function-problem_item_get_siz.patch b/0062-problem_data-add-a-new-function-problem_item_get_siz.patch new file mode 100644 index 0000000..351a3cf --- /dev/null +++ b/0062-problem_data-add-a-new-function-problem_item_get_siz.patch @@ -0,0 +1,90 @@ +From 3cb1e134607371c4356fe8a1be1e1f2858169c70 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 16 Mar 2015 08:15:00 +0100 +Subject: [PATCH] problem_data: add a new function problem_item_get_size + +Signed-off-by: Jakub Filak +--- + src/include/problem_data.h | 1 + + src/lib/make_descr.c | 12 ++++-------- + src/lib/problem_data.c | 19 +++++++++++++++++++ + 3 files changed, 24 insertions(+), 8 deletions(-) + +diff --git a/src/include/problem_data.h b/src/include/problem_data.h +index 661d31e..7058198 100644 +--- a/src/include/problem_data.h ++++ b/src/include/problem_data.h +@@ -59,6 +59,7 @@ typedef struct problem_item problem_item; + + char *problem_item_format(struct problem_item *item); + ++int problem_item_get_size(struct problem_item *item, unsigned long *size); + + /* In-memory problem data structure and accessors */ + +diff --git a/src/lib/make_descr.c b/src/lib/make_descr.c +index 912b87f..fd180a9 100644 +--- a/src/lib/make_descr.c ++++ b/src/lib/make_descr.c +@@ -216,12 +216,8 @@ char *make_description(problem_data_t *problem_data, char **names_to_skip, + strbuf_append_char(buf_dsc, '\n'); + append_empty_line = false; + +- struct stat statbuf; +- int stat_err = 0; +- if (item->flags & CD_FLAG_BIN) +- stat_err = stat(item->content, &statbuf); +- else +- statbuf.st_size = strlen(item->content); ++ unsigned long size = 0; ++ int stat_err = problem_item_get_size(item, &size); + + /* We don't print item->content for CD_FLAG_BIN, as it is + * always "/path/to/dump/dir/KEY" - not informative. +@@ -229,11 +225,11 @@ char *make_description(problem_data_t *problem_data, char **names_to_skip, + int pad = 16 - (strlen(key) + 2); + if (pad < 0) pad = 0; + strbuf_append_strf(buf_dsc, +- (!stat_err ? "%s: %*s%s file, %llu bytes\n" : "%s: %*s%s file\n"), ++ (!stat_err ? "%s: %*s%s file, %lu bytes\n" : "%s: %*s%s file\n"), + key, + pad, "", + ((item->flags & CD_FLAG_BIN) ? "Binary" : "Text"), +- (long long)statbuf.st_size ++ size + ); + empty = false; + } +diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c +index 6b8bb01..697ecd1 100644 +--- a/src/lib/problem_data.c ++++ b/src/lib/problem_data.c +@@ -52,6 +52,25 @@ char *problem_item_format(struct problem_item *item) + return NULL; + } + ++int problem_item_get_size(struct problem_item *item, unsigned long *size) ++{ ++ if (item->flags & CD_FLAG_TXT) ++ { ++ *size = strlen(item->content); ++ return 0; ++ } ++ ++ /* else if (item->flags & CD_FLAG_BIN) */ ++ struct stat statbuf; ++ statbuf.st_size = 0; ++ ++ if (stat(item->content, &statbuf) != 0) ++ return -errno; ++ ++ *size = statbuf.st_size; ++ return 0; ++} ++ + /* problem_data["name"] = { "content", CD_FLAG_foo_bits } */ + + problem_data_t *problem_data_new(void) +-- +2.1.0 + diff --git a/0063-problem_data-cache-problem_item-size.patch b/0063-problem_data-cache-problem_item-size.patch new file mode 100644 index 0000000..cd41ad9 --- /dev/null +++ b/0063-problem_data-cache-problem_item-size.patch @@ -0,0 +1,113 @@ +From 75141a6a90fdc46a10c6c28db41c648b7fdfcd9c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 24 Mar 2015 18:05:59 +0100 +Subject: [PATCH] problem_data: cache problem_item size + +This is necessary for problem_data gotten from D-Bus where the +underlying files might not be directly accessible. + +Signed-off-by: Jakub Filak +--- + src/include/problem_data.h | 8 ++++++++ + src/lib/problem_data.c | 27 +++++++++++++++++++++++---- + 2 files changed, 31 insertions(+), 4 deletions(-) + +diff --git a/src/include/problem_data.h b/src/include/problem_data.h +index 7058198..9722562 100644 +--- a/src/include/problem_data.h ++++ b/src/include/problem_data.h +@@ -46,9 +46,12 @@ enum { + CD_FLAG_BIGTXT = (1 << 6), + }; + ++#define PROBLEM_ITEM_UNINITIALIZED_SIZE ((unsigned long)-1) ++ + struct problem_item { + char *content; + unsigned flags; ++ unsigned long size; + /* Used by UI for presenting "item allowed/not allowed" checkboxes: */ + int selected_by_user; /* 0 "don't know", -1 "no", 1 "yes" */ + int allowed_by_reporter; /* 0 "no", 1 "yes" */ +@@ -82,6 +85,11 @@ void problem_data_add(problem_data_t *problem_data, + const char *name, + const char *content, + unsigned flags); ++struct problem_item *problem_data_add_ext(problem_data_t *problem_data, ++ const char *name, ++ const char *content, ++ unsigned flags, ++ unsigned long size); + void problem_data_add_text_noteditable(problem_data_t *problem_data, + const char *name, + const char *content); +diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c +index 697ecd1..ef76406 100644 +--- a/src/lib/problem_data.c ++++ b/src/lib/problem_data.c +@@ -54,20 +54,27 @@ char *problem_item_format(struct problem_item *item) + + int problem_item_get_size(struct problem_item *item, unsigned long *size) + { ++ if (item->size != PROBLEM_ITEM_UNINITIALIZED_SIZE) ++ { ++ *size = item->size; ++ return 0; ++ } ++ + if (item->flags & CD_FLAG_TXT) + { +- *size = strlen(item->content); ++ *size = item->size = strlen(item->content); + return 0; + } + + /* else if (item->flags & CD_FLAG_BIN) */ ++ + struct stat statbuf; + statbuf.st_size = 0; + + if (stat(item->content, &statbuf) != 0) + return -errno; + +- *size = statbuf.st_size; ++ *size = item->size = statbuf.st_size; + return 0; + } + +@@ -181,10 +188,11 @@ void problem_data_add_current_process_data(problem_data_t *pd) + } + } + +-void problem_data_add(problem_data_t *problem_data, ++struct problem_item *problem_data_add_ext(problem_data_t *problem_data, + const char *name, + const char *content, +- unsigned flags) ++ unsigned flags, ++ unsigned long size) + { + if (!(flags & CD_FLAG_BIN)) + flags |= CD_FLAG_TXT; +@@ -194,7 +202,18 @@ void problem_data_add(problem_data_t *problem_data, + struct problem_item *item = (struct problem_item *)xzalloc(sizeof(*item)); + item->content = xstrdup(content); + item->flags = flags; ++ item->size = size; + g_hash_table_replace(problem_data, xstrdup(name), item); ++ ++ return item; ++} ++ ++void problem_data_add(problem_data_t *problem_data, ++ const char *name, ++ const char *content, ++ unsigned flags) ++{ ++ problem_data_add_ext(problem_data, name, content, flags, PROBLEM_ITEM_UNINITIALIZED_SIZE); + } + + void problem_data_add_text_noteditable(problem_data_t *problem_data, +-- +2.1.0 + diff --git a/0064-report-client-provide-cpio-log-when-unpacking-fails.patch b/0064-report-client-provide-cpio-log-when-unpacking-fails.patch new file mode 100644 index 0000000..9d2618f --- /dev/null +++ b/0064-report-client-provide-cpio-log-when-unpacking-fails.patch @@ -0,0 +1,52 @@ +From 2576d07ba40b0b5d8a032f84ee1907c6d710d96f Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Wed, 22 Apr 2015 14:51:24 +0200 +Subject: [PATCH] report client: provide cpio log when unpacking fails + +Related to rhbz#1169774 rhbz#1213485 rhbz#1036918 + +Signed-off-by: Matej Habrnal +--- + src/client-python/debuginfo.py | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/src/client-python/debuginfo.py b/src/client-python/debuginfo.py +index 421a0aa..d292417 100644 +--- a/src/client-python/debuginfo.py ++++ b/src/client-python/debuginfo.py +@@ -9,6 +9,7 @@ import time + import errno + import shutil + from subprocess import Popen ++import tempfile + + from yum import _, YumBase + from yum.callbacks import DownloadBaseCallback +@@ -121,17 +122,22 @@ def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files= + file_patterns += "." + filename + " " + cpio_args = ["cpio", "-idu", file_patterns.strip()] + +- with open("/dev/null", "w") as null: ++ with tempfile.NamedTemporaryFile(prefix='abrt-unpacking-', dir='/tmp', ++ delete=False) as log_file: ++ log_file_name = log_file.name + cpio = Popen(cpio_args, cwd=destdir, bufsize=-1, +- stdin=unpacked_cpio, stdout=null, stderr=null) ++ stdin=unpacked_cpio, stdout=log_file, stderr=log_file) + retcode = cpio.wait() + + if retcode == 0: + log1("files extracted OK") + #print _("Removing temporary cpio file") ++ os.unlink(log_file_name) + os.unlink(unpacked_cpio_path) + else: + print _("Can't extract files from '{0}'").format(unpacked_cpio_path) ++ print(_("Can't extract files from '{0}'. For more information see '{1}'") ++ .format(unpacked_cpio_path, log_file_name)) + return RETURN_FAILURE + + def clean_up(): +-- +2.1.0 + diff --git a/0065-report-client-fix-close-an-unclosed-file.patch b/0065-report-client-fix-close-an-unclosed-file.patch new file mode 100644 index 0000000..ad6ceac --- /dev/null +++ b/0065-report-client-fix-close-an-unclosed-file.patch @@ -0,0 +1,26 @@ +From 5289c3eb39d2575bd6cbe1b99cb3f2caaa5b18a6 Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Wed, 22 Apr 2015 16:10:45 +0200 +Subject: [PATCH] report client: fix - close an unclosed file + +Signed-off-by: Matej Habrnal +--- + src/client-python/debuginfo.py | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/client-python/debuginfo.py b/src/client-python/debuginfo.py +index d292417..a019d25 100644 +--- a/src/client-python/debuginfo.py ++++ b/src/client-python/debuginfo.py +@@ -129,6 +129,8 @@ def unpack_rpm(package_file_name, files, tmp_dir, destdir, keeprpm, exact_files= + stdin=unpacked_cpio, stdout=log_file, stderr=log_file) + retcode = cpio.wait() + ++ unpacked_cpio.close() ++ + if retcode == 0: + log1("files extracted OK") + #print _("Removing temporary cpio file") +-- +2.1.0 + diff --git a/0066-report-client-check-owner-of-var-cache-abrt-di-when-.patch b/0066-report-client-check-owner-of-var-cache-abrt-di-when-.patch new file mode 100644 index 0000000..5d65177 --- /dev/null +++ b/0066-report-client-check-owner-of-var-cache-abrt-di-when-.patch @@ -0,0 +1,56 @@ +From 9e047438fa16f7e06d6049c35fcf1db2c48f6645 Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Tue, 12 May 2015 14:54:20 +0200 +Subject: [PATCH] report client: check owner of /var/cache/abrt-di when + unpacking fails + +If unpacking of debuginfo fails and '/vat/cache/abrt-di' is not owned by +abrt.abrt the client provides a solution how to fix the issue. + +Related to rhbz#1213485 + +Signed-off-by: Matej Habrnal +--- + src/client-python/debuginfo.py | 12 ++++++++++-- + 1 file changed, 10 insertions(+), 2 deletions(-) + +diff --git a/src/client-python/debuginfo.py b/src/client-python/debuginfo.py +index a019d25..98bfd0c 100644 +--- a/src/client-python/debuginfo.py ++++ b/src/client-python/debuginfo.py +@@ -5,6 +5,7 @@ + + import sys + import os ++import pwd + import time + import errno + import shutil +@@ -30,7 +31,6 @@ def ensure_abrt_uid(fn): + the function. + """ + +- import pwd + current_uid = os.getuid() + current_gid = os.getgid() + abrt = pwd.getpwnam("abrt") +@@ -499,7 +499,15 @@ class DebugInfoDownload(YumBase): + exact_files=download_exact_files) + if unpack_result == RETURN_FAILURE: + # recursively delete the temp dir on failure +- print _("Unpacking failed, aborting download...") ++ print(_("Unpacking failed, aborting download...")) ++ ++ s = os.stat(self.cachedir) ++ abrt = pwd.getpwnam("abrt") ++ if (s.st_uid != abrt.pw_uid) or (s.st_gid != abrt.pw_gid): ++ print(_("'{0}' must be owned by abrt. " ++ "Please run '# chown -R abrt.abrt {0}' " ++ "to fix the issue.").format(self.cachedir)) ++ + clean_up() + return RETURN_FAILURE + +-- +2.1.0 + diff --git a/0067-dumpdir-fix-initialization-of-dd_gid.patch b/0067-dumpdir-fix-initialization-of-dd_gid.patch new file mode 100644 index 0000000..1427e3e --- /dev/null +++ b/0067-dumpdir-fix-initialization-of-dd_gid.patch @@ -0,0 +1,26 @@ +From 11e716e10816de44f19fd7b09092e1d6acc1000a Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 21 May 2015 07:15:43 +0200 +Subject: [PATCH] dumpdir: fix initialization of dd_gid + +Signed-off-by: Jakub Filak +--- + src/lib/dump_dir.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index d50ebf7..d0e42cb 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -582,7 +582,7 @@ struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode) + if (uid != (uid_t)-1L) + { + dd->dd_uid = 0; +- dd->dd_uid = 0; ++ dd->dd_gid = 0; + + #if DUMP_DIR_OWNED_BY_USER > 0 + /* Check crashed application's uid */ +-- +2.1.0 + diff --git a/0068-lib-make-the-dump-proc-data-functions-more-robust.patch b/0068-lib-make-the-dump-proc-data-functions-more-robust.patch new file mode 100644 index 0000000..987d8b4 --- /dev/null +++ b/0068-lib-make-the-dump-proc-data-functions-more-robust.patch @@ -0,0 +1,92 @@ +From 688fb0e85551eb6f05e54c3df20ae325f4b88224 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 16 Apr 2015 11:17:20 +0200 +Subject: [PATCH] lib: make the dump proc data functions more robust + +dump_fd_info and dump_proc_diff are being called from processes running +under root permissions, so these functions must allow callers to +atomically created the destination file and update the ownership of that +file. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 2 ++ + src/lib/get_cmdline.c | 29 ++++++++++++++++++++++++++--- + 2 files changed, 28 insertions(+), 3 deletions(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 11c18d9..99f2fe1 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -641,6 +641,8 @@ char* get_cwd(pid_t pid); + char* get_rootdir(pid_t pid); + #define get_fsuid libreport_get_fsuid + int get_fsuid(const char *proc_pid_status); ++#define dump_fd_info_ext libreport_dump_fd_info_ext ++int dump_fd_info_ext(const char *dest_filename, const char *proc_pid_fd_path, uid_t uid, gid_t gid); + #define dump_fd_info libreport_dump_fd_info + int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path); + +diff --git a/src/lib/get_cmdline.c b/src/lib/get_cmdline.c +index 2e362c5..c55de30 100644 +--- a/src/lib/get_cmdline.c ++++ b/src/lib/get_cmdline.c +@@ -213,7 +213,7 @@ int get_fsuid(const char *proc_pid_status) + return fs_uid; + } + +-int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path) ++int dump_fd_info_ext(const char *dest_filename, const char *proc_pid_fd_path, uid_t uid, gid_t gid) + { + DIR *proc_fd_dir = NULL; + int proc_fdinfo_fd = -1; +@@ -237,7 +237,7 @@ int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path) + goto dumpfd_cleanup; + } + +- stream = fopen(dest_filename, "w"); ++ stream = fopen(dest_filename, "wex"); + if (!stream) + { + r = -ENOMEM; +@@ -295,7 +295,25 @@ dumpfd_next_fd: + + dumpfd_cleanup: + errno = 0; +- fclose(stream); ++ ++ if (stream != NULL) ++ { ++ if (uid != (uid_t)-1L) ++ { ++ const int stream_fd = fileno(stream); ++ r = fchown(stream_fd, uid, gid); ++ if (r < 0) ++ { ++ perror_msg("Can't change '%s' ownership to %lu:%lu", dest_filename, (long)uid, (long)gid); ++ fclose(stream); ++ unlink(dest_filename); ++ stream = NULL; ++ } ++ } ++ ++ if (stream != NULL) ++ fclose(stream); ++ } + + if (r == 0 && errno != 0) + r = -errno; +@@ -306,3 +324,8 @@ dumpfd_cleanup: + + return r; + } ++ ++int dump_fd_info(const char *dest_filename, const char *proc_pid_fd_path) ++{ ++ return dump_fd_info_ext(dest_filename, proc_pid_fd_path, /*UID*/-1, /*GID*/-1); ++} +-- +2.1.0 + diff --git a/0069-lib-introduce-a-new-function-copy_file_ext.patch b/0069-lib-introduce-a-new-function-copy_file_ext.patch new file mode 100644 index 0000000..6c78720 --- /dev/null +++ b/0069-lib-introduce-a-new-function-copy_file_ext.patch @@ -0,0 +1,80 @@ +From 1506a4e28f04d2a578b3a11b08c555fc0c5dbd90 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 15:17:47 +0200 +Subject: [PATCH] lib: introduce a new function copy_file_ext + +The new function allows to specify UID, GID and open() flags for both +source and destination files. + +This function is need to avoid race conditions and symbolic link issues. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 2 ++ + src/lib/copyfd.c | 21 ++++++++++++++++++--- + 2 files changed, 20 insertions(+), 3 deletions(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 99f2fe1..e66ca96 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -152,6 +152,8 @@ off_t copyfd_eof(int src_fd, int dst_fd, int flags); + off_t copyfd_size(int src_fd, int dst_fd, off_t size, int flags); + #define copyfd_exact_size libreport_copyfd_exact_size + void copyfd_exact_size(int src_fd, int dst_fd, off_t size); ++#define copy_file_ext libreport_copy_file_ext ++off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags); + #define copy_file libreport_copy_file + off_t copy_file(const char *src_name, const char *dst_name, int mode); + #define copy_file_recursive libreport_copy_file_recursive +diff --git a/src/lib/copyfd.c b/src/lib/copyfd.c +index e9f429d..64fece7 100644 +--- a/src/lib/copyfd.c ++++ b/src/lib/copyfd.c +@@ -149,16 +149,16 @@ off_t copyfd_eof(int fd1, int fd2, int flags) + return full_fd_action(fd1, fd2, 0, flags); + } + +-off_t copy_file(const char *src_name, const char *dst_name, int mode) ++off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags) + { + off_t r; +- int src = open(src_name, O_RDONLY); ++ int src = open(src_name, src_flags); + if (src < 0) + { + perror_msg("Can't open '%s'", src_name); + return -1; + } +- int dst = open(dst_name, O_WRONLY | O_TRUNC | O_CREAT, mode); ++ int dst = open(dst_name, dst_flags, mode); + if (dst < 0) + { + close(src); +@@ -167,6 +167,21 @@ off_t copy_file(const char *src_name, const char *dst_name, int mode) + } + r = copyfd_eof(src, dst, /*flags:*/ 0); + close(src); ++ if (uid != (uid_t)-1L) ++ { ++ if (fchown(dst, uid, gid) == -1) ++ { ++ perror_msg("Can't change '%s' ownership to %lu:%lu", dst_name, (long)uid, (long)gid); ++ close(dst); ++ unlink(dst_name); ++ return -1; ++ } ++ } + close(dst); + return r; + } ++ ++off_t copy_file(const char *src_name, const char *dst_name, int mode) ++{ ++ return copy_file_ext(src_name, dst_name, mode, -1, -1, O_RDONLY, O_WRONLY | O_TRUNC | O_CREAT); ++} +-- +2.1.0 + diff --git a/0070-dump_dir-allow-creating-of-a-new-dir-w-o-chowning-it.patch b/0070-dump_dir-allow-creating-of-a-new-dir-w-o-chowning-it.patch new file mode 100644 index 0000000..b063e89 --- /dev/null +++ b/0070-dump_dir-allow-creating-of-a-new-dir-w-o-chowning-it.patch @@ -0,0 +1,105 @@ +From 97033e727a5d8cc03f1ba9b0960fdd6470d22b5d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 15:19:40 +0200 +Subject: [PATCH] dump_dir: allow creating of a new dir w/o chowning it + +Split dd_create() in to dd_create_skeleton() creating the directory and +intializing struct dd* and dd_reset_ownership() updating UID and GUI to +the deemed values. + +We need this because we have to avoid situations where root is using a +directory owned by a regular user. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 2 ++ + src/lib/dump_dir.c | 42 +++++++++++++++++++++++++++++++++++------- + 2 files changed, 37 insertions(+), 7 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index fcd57c6..9bce008 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -66,6 +66,8 @@ struct dump_dir { + void dd_close(struct dump_dir *dd); + + struct dump_dir *dd_opendir(const char *dir, int flags); ++struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode); ++int dd_reset_ownership(struct dump_dir *dd); + /* Pass uid = (uid_t)-1L to disable chown'ing of newly created files + * (IOW: if you aren't running under root): + */ +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index d0e42cb..0c9d871 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -467,7 +467,10 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + return dd; + } + +-/* Create a fresh empty debug dump dir. ++/* Create a fresh empty debug dump dir which is owned bu the calling user. If ++ * you want to create the directory with meaningful ownership you should ++ * consider using dd_create() function or you can modify the ownership ++ * afterwards by calling dd_reset_ownership() function. + * + * ABRT owns dump dir: + * We should not allow users to write new files or write into existing ones, +@@ -523,7 +526,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + * this runs under 0:0 + * - clients: setroubleshootd, abrt python + */ +-struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode) ++struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode) + { + /* a little trick to copy read bits from file mode to exec bit of dir mode*/ + mode_t dir_mode = mode | ((mode & 0444) >> 2); +@@ -613,13 +616,38 @@ struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode) + else + error_msg("User %lu does not exist, using gid 0", (long)uid); + #endif ++ } + +- if (lchown(dir, dd->dd_uid, dd->dd_gid) == -1) +- { +- perror_msg("Can't change '%s' ownership to %lu:%lu", dir, +- (long)dd->dd_uid, (long)dd->dd_gid); +- } ++ return dd; ++} ++ ++/* Resets ownership of the given directory to UID and GID according to values ++ * in dd_create_skeleton(). ++ */ ++int dd_reset_ownership(struct dump_dir *dd) ++{ ++ if (!dd->locked) ++ error_msg_and_die("dump_dir is not opened"); /* bug */ ++ ++ const int r =lchown(dd->dd_dirname, dd->dd_uid, dd->dd_gid); ++ if (r < 0) ++ { ++ perror_msg("Can't change '%s' ownership to %lu:%lu", dd->dd_dirname, ++ (long)dd->dd_uid, (long)dd->dd_gid); + } ++ return r; ++} ++ ++/* Calls dd_create_skeleton() and dd_reset_ownership(). ++ */ ++struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode) ++{ ++ struct dump_dir *dd = dd_create_skeleton(dir, uid, mode); ++ if (dd == NULL) ++ return NULL; ++ ++ /* ignore results */ ++ dd_reset_ownership(dd); + + return dd; + } +-- +2.1.0 + diff --git a/0071-dump_dir-allow-hooks-to-create-dump-directory-withou.patch b/0071-dump_dir-allow-hooks-to-create-dump-directory-withou.patch new file mode 100644 index 0000000..df55406 --- /dev/null +++ b/0071-dump_dir-allow-hooks-to-create-dump-directory-withou.patch @@ -0,0 +1,83 @@ +From e59765d191d538385a20d0c72c98e253d747f040 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 17:41:49 +0200 +Subject: [PATCH] dump_dir: allow hooks to create dump directory without + parents + +With a centralized model of handling problems like ABRT, there is a need +to ensure that every dump directory is a descendant of some central +directory (database). This commit together with other security commits +makes code of the tools creating the dump directories in the central +directory more robust by ensuring that no tool accidentally creates the +central directory and all tools creates exactly one directory. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 4 +++- + src/lib/dump_dir.c | 12 +++++++++--- + 2 files changed, 12 insertions(+), 4 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index 9bce008..46349ed 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -43,6 +43,8 @@ enum { + DD_OPEN_READONLY = (1 << 3), + DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE = (1 << 4), + DD_DONT_WAIT_FOR_LOCK = (1 << 5), ++ /* Create the new dump directory with parent directories (mkdir -p)*/ ++ DD_CREATE_PARENTS = (1 << 6), + }; + + struct dump_dir { +@@ -66,7 +68,7 @@ struct dump_dir { + void dd_close(struct dump_dir *dd); + + struct dump_dir *dd_opendir(const char *dir, int flags); +-struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode); ++struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int flags); + int dd_reset_ownership(struct dump_dir *dd); + /* Pass uid = (uid_t)-1L to disable chown'ing of newly created files + * (IOW: if you aren't running under root): +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 0c9d871..25a8aeb 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -526,7 +526,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + * this runs under 0:0 + * - clients: setroubleshootd, abrt python + */ +-struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode) ++struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int flags) + { + /* a little trick to copy read bits from file mode to exec bit of dir mode*/ + mode_t dir_mode = mode | ((mode & 0444) >> 2); +@@ -559,7 +559,13 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode) + * the user to replace any file in the directory, changing security-sensitive data + * (e.g. "uid", "analyzer", "executable") + */ +- if (g_mkdir_with_parents(dd->dd_dirname, dir_mode) != 0) ++ int r; ++ if ((flags & DD_CREATE_PARENTS)) ++ r = g_mkdir_with_parents(dd->dd_dirname, dir_mode); ++ else ++ r = mkdir(dd->dd_dirname, dir_mode); ++ ++ if (r != 0) + { + perror_msg("Can't create directory '%s'", dir); + dd_close(dd); +@@ -642,7 +648,7 @@ int dd_reset_ownership(struct dump_dir *dd) + */ + struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode) + { +- struct dump_dir *dd = dd_create_skeleton(dir, uid, mode); ++ struct dump_dir *dd = dd_create_skeleton(dir, uid, mode, DD_CREATE_PARENTS); + if (dd == NULL) + return NULL; + +-- +2.1.0 + diff --git a/0072-lib-add-a-function-checking-file-names.patch b/0072-lib-add-a-function-checking-file-names.patch new file mode 100644 index 0000000..474eccd --- /dev/null +++ b/0072-lib-add-a-function-checking-file-names.patch @@ -0,0 +1,128 @@ +From ce207ac7cd523109e2478ce831a9034e0ecd2a94 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 13:21:41 +0200 +Subject: [PATCH] lib: add a function checking file names + +Move the code from ABRT and extend it a bit: +* allow only 64 characters +* allow '.' in names (vmcore_dmesg.txt) +* forbid '/' +* forbid "." +* forbid ".." + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 6 ++++++ + src/lib/concat_path_file.c | 25 ++++++++++++++++++++++ + tests/dump_dir.at | 46 ++++++++++++++++++++++++++++++++++++++++ + 3 files changed, 77 insertions(+) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index e66ca96..a4303de 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -131,6 +131,12 @@ char *concat_path_file(const char *path, const char *filename); + #define concat_path_basename libreport_concat_path_basename + char *concat_path_basename(const char *path, const char *filename); + ++/* Allows all printable characters except '/', ++ * the string must not exceed 64 characters of length ++ * and must not equal neither "." nor ".." (these strings may appear in the string) */ ++#define str_is_correct_filename libreport_str_is_correct_filename ++bool str_is_correct_filename(const char *str); ++ + /* A-la fgets, but malloced and of unlimited size */ + #define xmalloc_fgets libreport_xmalloc_fgets + char *xmalloc_fgets(FILE *file); +diff --git a/src/lib/concat_path_file.c b/src/lib/concat_path_file.c +index 39ae07a..24e4cbd 100644 +--- a/src/lib/concat_path_file.c ++++ b/src/lib/concat_path_file.c +@@ -57,3 +57,28 @@ char *concat_path_basename(const char *path, const char *filename) + free(abspath); + return name; + } ++ ++bool str_is_correct_filename(const char *str) ++{ ++#define NOT_PRINTABLE(c) (c < ' ' || c == 0x7f) ++ ++ if (NOT_PRINTABLE(*str) || *str == '/' || *str == '\0') ++ return false; ++ ++str; ++ ++ if (NOT_PRINTABLE(*str) || *str =='/' || (*str == '\0' && *(str-1) == '.')) ++ return false; ++ ++str; ++ ++ if (NOT_PRINTABLE(*str) || *str =='/' || (*str == '\0' && *(str-1) == '.' && *(str-2) == '.')) ++ return false; ++ ++str; ++ ++ for (unsigned i = 0; *str != '\0' && i < 61; ++str, ++i) ++ if (NOT_PRINTABLE(*str) || *str == '/') ++ return false; ++ ++ return *str == '\0'; ++ ++#undef NOT_PRINTABLE ++} +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index efec63f..19584d1 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -49,3 +49,49 @@ int main(int argc, char **argv) + return 0; + } + ]]) ++ ++## ----------------------- ## ++## str_is_correct_filename ## ++## ----------------------- ## ++ ++AT_TESTFUN([str_is_correct_filename], ++[[ ++#include "internal_libreport.h" ++#include ++# ++int main(void) ++{ ++ g_verbose = 3; ++ ++ assert(str_is_correct_filename("") == false); ++ assert(str_is_correct_filename("/") == false); ++ assert(str_is_correct_filename("//") == false); ++ assert(str_is_correct_filename(".") == false); ++ assert(str_is_correct_filename(".") == false); ++ assert(str_is_correct_filename("..") == false); ++ assert(str_is_correct_filename("..") == false); ++ assert(str_is_correct_filename("/.") == false); ++ assert(str_is_correct_filename("//.") == false); ++ assert(str_is_correct_filename("./") == false); ++ assert(str_is_correct_filename(".//") == false); ++ assert(str_is_correct_filename("/./") == false); ++ assert(str_is_correct_filename("/..") == false); ++ assert(str_is_correct_filename("//..") == false); ++ assert(str_is_correct_filename("../") == false); ++ assert(str_is_correct_filename("..//") == false); ++ assert(str_is_correct_filename("/../") == false); ++ assert(str_is_correct_filename("/.././") == false); ++ ++ assert(str_is_correct_filename("looks-good-but-evil/") == false); ++ assert(str_is_correct_filename("looks-good-but-evil/../../") == false); ++ ++ assert(str_is_correct_filename(".meta-data") == true); ++ assert(str_is_correct_filename("..meta-meta-data") == true); ++ assert(str_is_correct_filename("meta-..-data") == true); ++ ++ assert(str_is_correct_filename("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890+-") == true); ++ assert(str_is_correct_filename("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890+-=") == false); ++ ++ return 0; ++} ++]]) +-- +2.1.0 + diff --git a/0073-dd-harden-functions-against-directory-traversal-issu.patch b/0073-dd-harden-functions-against-directory-traversal-issu.patch new file mode 100644 index 0000000..1274d72 --- /dev/null +++ b/0073-dd-harden-functions-against-directory-traversal-issu.patch @@ -0,0 +1,148 @@ +From 14872ca4ac094205519dcbadde9b7f1ff28eda9a Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 13:30:15 +0200 +Subject: [PATCH] dd: harden functions against directory traversal issues + +Test correctness of all accessed dump dir files in all dd* functions. +Before this commit, the callers were allowed to pass strings like +"../../etc/shadow" in the filename argument of all dd* functions. + +Related: #1214457 + +Signed-off-by: Jakub Filak +--- + src/lib/create_dump_dir.c | 15 ++++++++++----- + src/lib/dump_dir.c | 30 ++++++++++++++++++++++++++++++ + 2 files changed, 40 insertions(+), 5 deletions(-) + +diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c +index 6aee370..d2cdd29 100644 +--- a/src/lib/create_dump_dir.c ++++ b/src/lib/create_dump_dir.c +@@ -107,16 +107,15 @@ int save_problem_data_in_dump_dir(struct dump_dir *dd, problem_data_t *problem_d + g_hash_table_iter_init(&iter, problem_data); + while (g_hash_table_iter_next(&iter, (void**)&name, (void**)&value)) + { +- if (value->flags & CD_FLAG_BIN) ++ if (!str_is_correct_filename(name)) + { +- dd_copy_file(dd, name, value->content); ++ error_msg("Problem data field name contains disallowed chars: '%s'", name); + continue; + } + +- /* only files should contain '/' and those are handled earlier */ +- if (name[0] == '.' || strchr(name, '/')) ++ if (value->flags & CD_FLAG_BIN) + { +- error_msg("Problem data field name contains disallowed chars: '%s'", name); ++ dd_copy_file(dd, name, value->content); + continue; + } + +@@ -138,6 +137,12 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + return NULL; + } + ++ if (!str_is_correct_filename(type)) ++ { ++ error_msg(_("'%s' is not correct file name"), FILENAME_TYPE); ++ return NULL; ++ } ++ + uid_t uid = (uid_t)-1L; + char *uid_str = problem_data_get_content_or_NULL(problem_data, FILENAME_UID); + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 25a8aeb..017a9c1 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -357,6 +357,9 @@ static inline struct dump_dir *dd_init(void) + + int dd_exist(const struct dump_dir *dd, const char *path) + { ++ if (!str_is_correct_filename(path)) ++ error_msg_and_die("Cannot test existence. '%s' is not a valid file name", path); ++ + char *full_path = concat_path_file(dd->dd_dirname, path); + int ret = exist_file_dir(full_path); + free(full_path); +@@ -1059,6 +1062,15 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla + // if (!dd->locked) + // error_msg_and_die("dump_dir is not opened"); /* bug */ + ++ if (!str_is_correct_filename(name)) ++ { ++ error_msg("Cannot load text. '%s' is not a valid file name", name); ++ if ((flags & DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE)) ++ return NULL; ++ ++ xfunc_die(); ++ } ++ + /* Compat with old abrt dumps. Remove in abrt-2.1 */ + if (strcmp(name, "release") == 0) + name = FILENAME_OS_RELEASE; +@@ -1080,6 +1092,9 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot save text. '%s' is not a valid file name", name); ++ + char *full_path = concat_path_file(dd->dd_dirname, name); + save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode); + free(full_path); +@@ -1090,6 +1105,9 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot save binary. '%s' is not a valid file name", name); ++ + char *full_path = concat_path_file(dd->dd_dirname, name); + save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid, dd->mode); + free(full_path); +@@ -1097,6 +1115,9 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns + + long dd_get_item_size(struct dump_dir *dd, const char *name) + { ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot get item size. '%s' is not a valid file name", name); ++ + long size = -1; + char *iname = concat_path_file(dd->dd_dirname, name); + struct stat statbuf; +@@ -1121,6 +1142,9 @@ int dd_delete_item(struct dump_dir *dd, const char *name) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot delete item. '%s' is not a valid file name", name); ++ + char *path = concat_path_file(dd->dd_dirname, name); + int res = unlink(path); + +@@ -1335,6 +1359,9 @@ int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason) + + int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + { ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); ++ + char *dest = concat_path_file(dd->dd_dirname, name); + + log_debug("copying '%s' to '%s'", source_path, dest); +@@ -1351,6 +1378,9 @@ int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + + int dd_copy_file_unpack(struct dump_dir *dd, const char *name, const char *source_path) + { ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); ++ + char *dest = concat_path_file(dd->dd_dirname, name); + + log_debug("unpacking '%s' to '%s'", source_path, dest); +-- +2.1.0 + diff --git a/0074-lib-allow-creating-root-owned-problem-directories-fr.patch b/0074-lib-allow-creating-root-owned-problem-directories-fr.patch new file mode 100644 index 0000000..4f968f5 --- /dev/null +++ b/0074-lib-allow-creating-root-owned-problem-directories-fr.patch @@ -0,0 +1,77 @@ +From 9669e34df23959fa18c2ce8920140e717440947f Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 16:33:00 +0200 +Subject: [PATCH] lib: allow creating root owned problem directories from + problem data + +Without this patch libreport sets the owner of new problem directory +according to FILENAME_UID. This approach is not sufficient because ABRT +has introduced PrivateReports that should ensure that all problem +directories are owned by root. So ABRT needs a way to tell libreport to +create the new problem directory with uid=0. + +Signed-off-by: Jakub Filak +--- + src/include/problem_data.h | 1 + + src/lib/create_dump_dir.c | 14 +++++++++++--- + 2 files changed, 12 insertions(+), 3 deletions(-) + +diff --git a/src/include/problem_data.h b/src/include/problem_data.h +index 9722562..5e271d3 100644 +--- a/src/include/problem_data.h ++++ b/src/include/problem_data.h +@@ -140,6 +140,7 @@ problem_data_t *create_problem_data_for_reporting(const char *dump_dir_name); + @param base_dir_name Location to store the problem data + */ + struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name); ++struct dump_dir *create_dump_dir_from_problem_data_ext(problem_data_t *problem_data, const char *base_dir_name, uid_t uid); + + /** + @brief Saves the problem data object in opened dump directory +diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c +index d2cdd29..dec75fa 100644 +--- a/src/lib/create_dump_dir.c ++++ b/src/lib/create_dump_dir.c +@@ -85,7 +85,8 @@ struct dump_dir *create_dump_dir(const char *base_dir_name, const char *type, ui + * reporting from anaconda where we can't read /etc/{system,redhat}-release + * and os_release is taken from anaconda + */ +- dd_create_basic_files(dd, uid, NULL); ++ const uid_t crashed_uid = dd_exist(dd, FILENAME_UID) ? /*uid already saved*/-1 : uid; ++ dd_create_basic_files(dd, crashed_uid, NULL); + + problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0'; + char* new_path = concat_path_file(base_dir_name, problem_id); +@@ -125,7 +126,7 @@ int save_problem_data_in_dump_dir(struct dump_dir *dd, problem_data_t *problem_d + return 0; + } + +-struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name) ++struct dump_dir *create_dump_dir_from_problem_data_ext(problem_data_t *problem_data, const char *base_dir_name, uid_t uid) + { + INITIALIZE_LIBREPORT(); + +@@ -143,6 +144,13 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + return NULL; + } + ++ return create_dump_dir(base_dir_name, type, uid, (save_data_call_back)save_problem_data_in_dump_dir, problem_data); ++} ++ ++struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, const char *base_dir_name) ++{ ++ INITIALIZE_LIBREPORT(); ++ + uid_t uid = (uid_t)-1L; + char *uid_str = problem_data_get_content_or_NULL(problem_data, FILENAME_UID); + +@@ -161,5 +169,5 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + uid = (uid_t)val; + } + +- return create_dump_dir(base_dir_name, type, uid, (save_data_call_back)save_problem_data_in_dump_dir, problem_data); ++ return create_dump_dir_from_problem_data_ext(problem_data, base_dir_name, uid); + } +-- +2.1.0 + diff --git a/0075-lib-fix-races-in-dump-directory-handling-code.patch b/0075-lib-fix-races-in-dump-directory-handling-code.patch new file mode 100644 index 0000000..77a082b --- /dev/null +++ b/0075-lib-fix-races-in-dump-directory-handling-code.patch @@ -0,0 +1,1323 @@ +From c7f7a2f3f6c4847bcf4ac2285b0efbfb281334b5 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 24 Apr 2015 10:57:23 +0200 +Subject: [PATCH] lib: fix races in dump directory handling code + +Florian Weimer : + + dd_opendir() should keep a file handle (opened with O_DIRECTORY) and + use openat() and similar functions to access files in it. + + ... + + The file system manipulation functions should guard against hard + links (check that link count is <= 1, just as in the user coredump + code in abrt-hook-ccpp), possibly after opening the file + with O_PATH first to avoid side effects on open/close. + +Related: #1214745 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 7 + + src/include/internal_libreport.h | 15 +- + src/lib/compress.c | 23 +- + src/lib/copyfd.c | 19 +- + src/lib/dump_dir.c | 456 +++++++++++++++++++++++---------------- + src/lib/problem_data.c | 6 +- + src/lib/xfuncs.c | 22 +- + tests/dump_dir.at | 156 ++++++++++++++ + 8 files changed, 499 insertions(+), 205 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index 46349ed..669f7fd 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -34,6 +34,12 @@ extern "C" { + + /* Utility function */ + int create_symlink_lockfile(const char *filename, const char *pid_str); ++int create_symlink_lockfile_at(int dir_fd, const char *filename, const char *pid_str); ++ ++/* Opens filename for reading relatively to a directory represented by dir_fd. ++ * The function fails if the file is symbolic link, directory or hard link. ++ */ ++int secure_openat_read(int dir_fd, const char *filename); + + enum { + DD_FAIL_QUIETLY_ENOENT = (1 << 0), +@@ -63,6 +69,7 @@ struct dump_dir { + * lock but are not able to unlock the dump directory. + */ + int owns_lock; ++ int dd_fd; + }; + + void dd_close(struct dump_dir *dd); +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index a4303de..66f91e9 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -158,10 +158,14 @@ off_t copyfd_eof(int src_fd, int dst_fd, int flags); + off_t copyfd_size(int src_fd, int dst_fd, off_t size, int flags); + #define copyfd_exact_size libreport_copyfd_exact_size + void copyfd_exact_size(int src_fd, int dst_fd, off_t size); +-#define copy_file_ext libreport_copy_file_ext +-off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags); ++#define copy_file_ext_at libreport_copy_file_ext_at ++off_t copy_file_ext_at(const char *src_name, int dir_fd, const char *name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags); ++#define copy_file_ext(src_name, dst_name, mode, uid, gid, src_flags, dst_flags) \ ++ copy_file_ext_at(src_name, AT_FDCWD, dst_name, mode, uid, gid, src_flags, dst_flags) + #define copy_file libreport_copy_file + off_t copy_file(const char *src_name, const char *dst_name, int mode); ++#define copy_file_at libreport_copy_file_at ++off_t copy_file_at(const char *src_name, int dir_fd, const char *name, int mode); + #define copy_file_recursive libreport_copy_file_recursive + int copy_file_recursive(const char *source, const char *dest); + +@@ -169,6 +173,9 @@ int copy_file_recursive(const char *source, const char *dest); + int decompress_fd(int fdi, int fdo); + #define decompress_file libreport_decompress_file + int decompress_file(const char *path_in, const char *path_out, mode_t mode_out); ++#define decompress_file_ext_at libreport_decompress_file_ext_at ++int decompress_file_ext_at(const char *path_in, int dir_fd, const char *path_out, ++ mode_t mode_out, uid_t uid, gid_t gid, int src_flags, int dst_flags); + + // NB: will return short read on error, not -1, + // if some data was read before error occurred +@@ -414,6 +421,8 @@ int xopen3(const char *pathname, int flags, int mode); + int xopen(const char *pathname, int flags); + #define xunlink libreport_xunlink + void xunlink(const char *pathname); ++#define xunlinkat libreport_xunlinkat ++void xunlinkat(int dir_fd, const char *pathname, int flags); + + /* Just testing dent->d_type == DT_REG is wrong: some filesystems + * do not report the type, they report DT_UNKNOWN for every dirent +@@ -423,6 +432,8 @@ void xunlink(const char *pathname); + */ + #define is_regular_file libreport_is_regular_file + int is_regular_file(struct dirent *dent, const char *dirname); ++#define is_regular_file_at libreport_is_regular_file_at ++int is_regular_file_at(struct dirent *dent, int dir_fd); + + #define dot_or_dotdot libreport_dot_or_dotdot + bool dot_or_dotdot(const char *filename); +diff --git a/src/lib/compress.c b/src/lib/compress.c +index eec155a..86343b3 100644 +--- a/src/lib/compress.c ++++ b/src/lib/compress.c +@@ -124,16 +124,17 @@ decompress_fd(int fdi, int fdo) + } + + int +-decompress_file(const char *path_in, const char *path_out, mode_t mode_out) ++decompress_file_ext_at(const char *path_in, int dir_fd, const char *path_out, mode_t mode_out, ++ uid_t uid, gid_t gid, int src_flags, int dst_flags) + { +- int fdi = open(path_in, O_RDONLY | O_CLOEXEC); ++ int fdi = open(path_in, src_flags); + if (fdi < 0) + { + perror_msg("Could not open file: %s", path_in); + return -1; + } + +- int fdo = open(path_out, O_WRONLY | O_CLOEXEC | O_EXCL | O_CREAT, mode_out); ++ int fdo = openat(dir_fd, path_out, dst_flags, mode_out); + if (fdo < 0) + { + close(fdi); +@@ -143,10 +144,24 @@ decompress_file(const char *path_in, const char *path_out, mode_t mode_out) + + int ret = decompress_fd(fdi, fdo); + close(fdi); ++ if (uid != (uid_t)-1L) ++ { ++ if (fchown(fdo, uid, gid) == -1) ++ { ++ perror_msg("Can't change ownership of '%s' to %lu:%lu", path_out, (long)uid, (long)gid); ++ ret = -1; ++ } ++ } + close(fdo); + + if (ret != 0) +- unlink(path_out); ++ unlinkat(dir_fd, path_out, /*only files*/0); + + return ret; + } ++ ++int decompress_file(const char *path_in, const char *path_out, mode_t mode_out) ++{ ++ return decompress_file_ext_at(path_in, AT_FDCWD, path_out, mode_out, -1, -1, ++ O_RDONLY, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC); ++} +diff --git a/src/lib/copyfd.c b/src/lib/copyfd.c +index 64fece7..45c0d2f 100644 +--- a/src/lib/copyfd.c ++++ b/src/lib/copyfd.c +@@ -149,7 +149,7 @@ off_t copyfd_eof(int fd1, int fd2, int flags) + return full_fd_action(fd1, fd2, 0, flags); + } + +-off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags) ++off_t copy_file_ext_at(const char *src_name, int dir_fd, const char *name, int mode, uid_t uid, gid_t gid, int src_flags, int dst_flags) + { + off_t r; + int src = open(src_name, src_flags); +@@ -158,11 +158,11 @@ off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t + perror_msg("Can't open '%s'", src_name); + return -1; + } +- int dst = open(dst_name, dst_flags, mode); ++ int dst = openat(dir_fd, name, dst_flags, mode); + if (dst < 0) + { + close(src); +- perror_msg("Can't open '%s'", dst_name); ++ perror_msg("Can't open '%s'", name); + return -1; + } + r = copyfd_eof(src, dst, /*flags:*/ 0); +@@ -171,17 +171,24 @@ off_t copy_file_ext(const char *src_name, const char *dst_name, int mode, uid_t + { + if (fchown(dst, uid, gid) == -1) + { +- perror_msg("Can't change '%s' ownership to %lu:%lu", dst_name, (long)uid, (long)gid); ++ perror_msg("Can't change ownership of '%s' to %lu:%lu", name, (long)uid, (long)gid); + close(dst); +- unlink(dst_name); + return -1; + } + } + close(dst); ++ + return r; + } + ++off_t copy_file_at(const char *src_name, int dir_fd, const char *name, int mode) ++{ ++ return copy_file_ext_at(src_name, dir_fd, name, mode, -1, -1, ++ O_RDONLY, O_WRONLY | O_TRUNC | O_CREAT); ++} ++ + off_t copy_file(const char *src_name, const char *dst_name, int mode) + { +- return copy_file_ext(src_name, dst_name, mode, -1, -1, O_RDONLY, O_WRONLY | O_TRUNC | O_CREAT); ++ return copy_file_ext(src_name, dst_name, mode, -1, -1, ++ O_RDONLY, O_WRONLY | O_TRUNC | O_CREAT); + } +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 017a9c1..7cc918a 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -85,6 +85,7 @@ + + + static char *load_text_file(const char *path, unsigned flags); ++static char *load_text_file_at(int dir_fd, const char *name, unsigned flags); + static void copy_file_from_chroot(struct dump_dir* dd, const char *name, + const char *chroot_dir, const char *file_path); + +@@ -98,10 +99,10 @@ static bool isdigit_str(const char *str) + return true; + } + +-static bool exist_file_dir(const char *path) ++static bool exist_file_dir_at(int dir_fd, const char *name) + { + struct stat buf; +- if (stat(path, &buf) == 0) ++ if (fstatat(dir_fd, name, &buf, AT_SYMLINK_NOFOLLOW) == 0) + { + if (S_ISDIR(buf.st_mode) || S_ISREG(buf.st_mode)) + { +@@ -111,15 +112,68 @@ static bool exist_file_dir(const char *path) + return false; + } + ++/* Opens the file in the three following steps: ++ * 1. open the file with O_PATH (get a file descriptor for operations with ++ * inode) and O_NOFOLLOW (do not dereference symbolick links) ++ * 2. stat the resulting file descriptor and fail if the opened file is not a ++ * regular file or if the number of links is greater than 1 (that means that ++ * the inode has more names (hard links)) ++ * 3. "re-open" the file descriptor retrieved in the first step with O_RDONLY ++ * by opening /proc/self/fd/$fd (then close the former file descriptor and ++ * return the new one). ++ */ ++int secure_openat_read(int dir_fd, const char *filename) ++{ ++ if (strchr(filename, '/')) ++ { ++ error_msg("Path must be file name without directory: '%s'", filename); ++ errno = EFAULT; ++ return -1; ++ } ++ ++ static char reopen_buf[sizeof("/proc/self/fd/") + 3*sizeof(int) + 1]; ++ ++ int path_fd = openat(dir_fd, filename, O_PATH | O_NOFOLLOW); ++ if (path_fd < 0) ++ return -1; ++ ++ struct stat path_sb; ++ int r = fstat(path_fd, &path_sb); ++ if (r < 0) ++ { ++ perror_msg("stat"); ++ close(path_fd); ++ return -1; ++ } ++ ++ if (!S_ISREG(path_sb.st_mode) || path_sb.st_nlink > 1) ++ { ++ log_notice("Path isn't a regular file or has more links (%lu)", (unsigned long)path_sb.st_nlink); ++ errno = EINVAL; ++ close(path_fd); ++ return -1; ++ } ++ ++ if (snprintf(reopen_buf, sizeof(reopen_buf), "/proc/self/fd/%d", path_fd) >= sizeof(reopen_buf)) { ++ error_msg("BUG: too long path to a file descriptor"); ++ abort(); ++ } ++ ++ const int fd = open(reopen_buf, O_RDONLY); ++ close(path_fd); ++ ++ return fd; ++} ++ + /* Returns value less than 0 if the file is not readable or + * if the file doesn't contain valid unixt time stamp. + * + * Any possible failure will be logged. + */ +-static time_t parse_time_file(const char *filename) ++static time_t parse_time_file_at(int dir_fd, const char *filename) + { + /* Open input file, and parse it. */ +- int fd = open(filename, O_RDONLY | O_NOFOLLOW); ++ int fd = secure_openat_read(dir_fd, filename); + if (fd < 0) + { + VERB2 pwarn_msg("Can't open '%s'", filename); +@@ -183,9 +237,9 @@ static time_t parse_time_file(const char *filename) + * 0: failed to lock (someone else has it locked) + * 1: success + */ +-int create_symlink_lockfile(const char* lock_file, const char* pid) ++int create_symlink_lockfile_at(int dir_fd, const char* lock_file, const char* pid) + { +- while (symlink(pid, lock_file) != 0) ++ while (symlinkat(pid, dir_fd, lock_file) != 0) + { + if (errno != EEXIST) + { +@@ -198,7 +252,7 @@ int create_symlink_lockfile(const char* lock_file, const char* pid) + } + + char pid_buf[sizeof(pid_t)*3 + 4]; +- ssize_t r = readlink(lock_file, pid_buf, sizeof(pid_buf) - 1); ++ ssize_t r = readlinkat(dir_fd, lock_file, pid_buf, sizeof(pid_buf) - 1); + if (r < 0) + { + if (errno == ENOENT) +@@ -231,7 +285,7 @@ int create_symlink_lockfile(const char* lock_file, const char* pid) + log("Lock file '%s' was locked by process %s, but it crashed?", lock_file, pid_buf); + } + /* The file may be deleted by now by other process. Ignore ENOENT */ +- if (unlink(lock_file) != 0 && errno != ENOENT) ++ if (unlinkat(dir_fd, lock_file, /*only files*/0) != 0 && errno != ENOENT) + { + perror_msg("Can't remove stale lock file '%s'", lock_file); + errno = 0; +@@ -243,21 +297,21 @@ int create_symlink_lockfile(const char* lock_file, const char* pid) + return 1; + } + ++int create_symlink_lockfile(const char *filename, const char *pid_str) ++{ ++ return create_symlink_lockfile_at(AT_FDCWD, filename, pid_str); ++} ++ + static const char *dd_check(struct dump_dir *dd) + { +- unsigned dirname_len = strlen(dd->dd_dirname); +- char filename_buf[FILENAME_MAX+1]; +- strcpy(filename_buf, dd->dd_dirname); +- strcpy(filename_buf + dirname_len, "/"FILENAME_TIME); +- dd->dd_time = parse_time_file(filename_buf); ++ dd->dd_time = parse_time_file_at(dd->dd_fd, FILENAME_TIME); + if (dd->dd_time < 0) + { + log_debug("Missing file: "FILENAME_TIME); + return FILENAME_TIME; + } + +- strcpy(filename_buf + dirname_len, "/"FILENAME_TYPE); +- dd->dd_type = load_text_file(filename_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); ++ dd->dd_type = load_text_file_at(dd->dd_fd, FILENAME_TYPE, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); + if (!dd->dd_type || (strlen(dd->dd_type) == 0)) + { + log_debug("Missing or empty file: "FILENAME_TYPE); +@@ -275,17 +329,12 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags) + char pid_buf[sizeof(long)*3 + 2]; + snprintf(pid_buf, sizeof(pid_buf), "%lu", (long)getpid()); + +- unsigned dirname_len = strlen(dd->dd_dirname); +- char lock_buf[dirname_len + sizeof("/.lock")]; +- strcpy(lock_buf, dd->dd_dirname); +- strcpy(lock_buf + dirname_len, "/.lock"); +- + unsigned count = NO_TIME_FILE_COUNT; + + retry: + while (1) + { +- int r = create_symlink_lockfile(lock_buf, pid_buf); ++ int r = create_symlink_lockfile_at(dd->dd_fd, ".lock", pid_buf); + if (r < 0) + return r; /* error */ + if (r > 0 || errno == EALREADY) +@@ -312,9 +361,9 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags) + if (missing_file) + { + if (dd->owns_lock) +- xunlink(lock_buf); ++ xunlinkat(dd->dd_fd, ".lock", /*only files*/0); + +- log_warning("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file); ++ log_warning("Unlocked '%s' (no or corrupted '%s' file)", dd->dd_dirname, missing_file); + if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK) + { + errno = EISDIR; /* "this is an ordinary dir, not dump dir" */ +@@ -333,18 +382,13 @@ static void dd_unlock(struct dump_dir *dd) + { + if (dd->locked) + { +- unsigned dirname_len = strlen(dd->dd_dirname); +- char lock_buf[dirname_len + sizeof("/.lock")]; +- strcpy(lock_buf, dd->dd_dirname); +- strcpy(lock_buf + dirname_len, "/.lock"); +- + if (dd->owns_lock) +- xunlink(lock_buf); ++ xunlinkat(dd->dd_fd, ".lock", /*only files*/0); + + dd->owns_lock = 0; + dd->locked = 0; + +- log_info("Unlocked '%s'", lock_buf); ++ log_info("Unlocked '%s/.lock'", dd->dd_dirname); + } + } + +@@ -352,17 +396,16 @@ static inline struct dump_dir *dd_init(void) + { + struct dump_dir* dd = (struct dump_dir*)xzalloc(sizeof(struct dump_dir)); + dd->dd_time = -1; ++ dd->dd_fd = -1; + return dd; + } + +-int dd_exist(const struct dump_dir *dd, const char *path) ++int dd_exist(const struct dump_dir *dd, const char *name) + { +- if (!str_is_correct_filename(path)) +- error_msg_and_die("Cannot test existence. '%s' is not a valid file name", path); ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + +- char *full_path = concat_path_file(dd->dd_dirname, path); +- int ret = exist_file_dir(full_path); +- free(full_path); ++ const int ret = exist_file_dir_at(dd->dd_fd, name); + return ret; + } + +@@ -372,6 +415,10 @@ void dd_close(struct dump_dir *dd) + return; + + dd_unlock(dd); ++ ++ if (dd->dd_fd >= 0) ++ close(dd->dd_fd); ++ + if (dd->next_dir) + { + closedir(dd->next_dir); +@@ -396,10 +443,13 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + struct dump_dir *dd = dd_init(); + + dir = dd->dd_dirname = rm_trailing_slashes(dir); +- ++ dd->dd_fd = open(dir, O_DIRECTORY | O_NOFOLLOW); + struct stat stat_buf; +- if (stat(dir, &stat_buf) != 0) ++ if (dd->dd_fd < 0) + goto cant_access; ++ if (fstat(dd->dd_fd, &stat_buf) != 0) ++ goto cant_access; ++ + /* & 0666 should remove the executable bit */ + dd->mode = (stat_buf.st_mode & 0666); + +@@ -409,11 +459,12 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + if ((flags & DD_OPEN_READONLY) && errno == EACCES) + { + /* Directory is not writable. If it seems to be readable, +- * return "read only" dd, not NULL */ +- if (stat(dir, &stat_buf) == 0 +- && S_ISDIR(stat_buf.st_mode) +- && access(dir, R_OK) == 0 +- ) { ++ * return "read only" dd, not NULL ++ * ++ * Does the directory have 'r' flag? ++ */ ++ if (faccessat(dd->dd_fd, ".", R_OK, AT_SYMLINK_NOFOLLOW) == 0) ++ { + if(dd_check(dd) != NULL) + { + dd_close(dd); +@@ -456,10 +507,9 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + if (geteuid() == 0) + { + /* In case caller would want to create more files, he'll need uid:gid */ +- struct stat stat_buf; +- if (stat(dir, &stat_buf) != 0 || !S_ISDIR(stat_buf.st_mode)) ++ if (fstat(dd->dd_fd, &stat_buf) != 0) + { +- error_msg("Can't stat '%s', or it is not a directory", dir); ++ error_msg("Can't stat '%s'", dir); + dd_close(dd); + return NULL; + } +@@ -554,8 +604,7 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + * dd_create("dir/..") and similar are madness, refuse them. + */ + error_msg("Bad dir name '%s'", dir); +- dd_close(dd); +- return NULL; ++ goto fail; + } + + /* Was creating it with mode 0700 and user as the owner, but this allows +@@ -571,22 +620,31 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + if (r != 0) + { + perror_msg("Can't create directory '%s'", dir); +- dd_close(dd); +- return NULL; ++ goto fail; + } + +- if (dd_lock(dd, CREATE_LOCK_USLEEP, /*flags:*/ 0) < 0) ++ dd->dd_fd = open(dd->dd_dirname, O_DIRECTORY | O_NOFOLLOW); ++ if (dd->dd_fd < 0) + { +- dd_close(dd); +- return NULL; ++ perror_msg("Can't open newly created directory '%s'", dir); ++ goto fail; ++ } ++ ++ struct stat stat_sb; ++ if (fstat(dd->dd_fd, &stat_sb) < 0) ++ { ++ perror_msg("stat(%s)", dd->dd_dirname); ++ goto fail; + } + ++ if (dd_lock(dd, CREATE_LOCK_USLEEP, /*flags:*/ 0) < 0) ++ goto fail; ++ + /* mkdir's mode (above) can be affected by umask, fix it */ +- if (chmod(dir, dir_mode) == -1) ++ if (fchmod(dd->dd_fd, dir_mode) == -1) + { + perror_msg("Can't change mode of '%s'", dir); +- dd_close(dd); +- return NULL; ++ goto fail; + } + + dd->dd_uid = (uid_t)-1L; +@@ -628,6 +686,10 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + } + + return dd; ++ ++fail: ++ dd_close(dd); ++ return NULL; + } + + /* Resets ownership of the given directory to UID and GID according to values +@@ -638,7 +700,7 @@ int dd_reset_ownership(struct dump_dir *dd) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- const int r =lchown(dd->dd_dirname, dd->dd_uid, dd->dd_gid); ++ const int r = fchown(dd->dd_fd, dd->dd_uid, dd->dd_gid); + if (r < 0) + { + perror_msg("Can't change '%s' ownership to %lu:%lu", dd->dd_dirname, +@@ -755,61 +817,43 @@ void dd_sanitize_mode_and_owner(struct dump_dir *dd) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- DIR *d = opendir(dd->dd_dirname); +- if (!d) +- return; +- +- struct dirent *dent; +- while ((dent = readdir(d)) != NULL) ++ dd_init_next_file(dd); ++ char *short_name; ++ while (dd_get_next_file(dd, &short_name, /*full_name*/ NULL)) + { +- if (dent->d_name[0] == '.') /* ".lock", ".", ".."? skip */ +- continue; +- char *full_path = concat_path_file(dd->dd_dirname, dent->d_name); +- struct stat statbuf; +- if (lstat(full_path, &statbuf) == 0 && S_ISREG(statbuf.st_mode)) +- { +- if ((statbuf.st_mode & 0777) != dd->mode) +- { +- /* We open the file only for fchmod() +- * +- * We use fchmod() because chmod() changes the permissions of +- * the file specified whose pathname is given in path, which +- * is dereferenced if it is a symbolic link. +- */ +- int fd = open(full_path, O_RDONLY | O_NOFOLLOW, dd->mode); +- if (fd >= 0) +- { +- if (fchmod(fd, dd->mode) != 0) +- { +- perror_msg("Can't change '%s' mode to 0%o", full_path, +- (unsigned)dd->mode); +- } +- close(fd); +- } +- else +- { +- perror_msg("Can't open regular file '%s'", full_path); +- } +- } +- if (statbuf.st_uid != dd->dd_uid || statbuf.st_gid != dd->dd_gid) +- { +- if (lchown(full_path, dd->dd_uid, dd->dd_gid) != 0) +- { +- perror_msg("Can't change '%s' ownership to %lu:%lu", full_path, +- (long)dd->dd_uid, (long)dd->dd_gid); +- } +- } +- } +- free(full_path); ++ /* The current process has to have read access at least */ ++ int fd = secure_openat_read(dd->dd_fd, short_name); ++ if (fd < 0) ++ goto next; ++ ++ if (fchmod(fd, dd->mode) != 0) ++ perror_msg("Can't change '%s/%s' mode to 0%o", dd->dd_dirname, short_name, ++ (unsigned)dd->mode); ++ ++ if (fchown(fd, dd->dd_uid, dd->dd_gid) != 0) ++ perror_msg("Can't change '%s/%s' ownership to %lu:%lu", dd->dd_dirname, short_name, ++ (long)dd->dd_uid, (long)dd->dd_gid); ++ ++ close(fd); ++next: ++ free(short_name); + } +- closedir(d); + } + +-static int delete_file_dir(const char *dir, bool skip_lock_file) ++static int delete_file_dir(int dir_fd, bool skip_lock_file) + { +- DIR *d = opendir(dir); ++ int opendir_fd = dup(dir_fd); ++ if (opendir_fd < 0) ++ { ++ perror_msg("delete_file_dir: dup(dir_fd)"); ++ return -1; ++ } ++ ++ lseek(opendir_fd, SEEK_SET, 0); ++ DIR *d = fdopendir(opendir_fd); + if (!d) + { ++ close(opendir_fd); + /* The caller expects us to error out only if the directory + * still exists (not deleted). If directory + * *doesn't exist*, return 0 and clear errno. +@@ -833,26 +877,35 @@ static int delete_file_dir(const char *dir, bool skip_lock_file) + unlink_lock_file = true; + continue; + } +- char *full_path = concat_path_file(dir, dent->d_name); +- if (unlink(full_path) == -1 && errno != ENOENT) ++ if (unlinkat(dir_fd, dent->d_name, /*only files*/0) == -1 && errno != ENOENT) + { + int err = 0; + if (errno == EISDIR) + { + errno = 0; +- err = delete_file_dir(full_path, /*skip_lock_file:*/ false); ++ int subdir_fd = openat(dir_fd, dent->d_name, O_DIRECTORY); ++ if (subdir_fd < 0) ++ { ++ perror_msg("Can't open sub-dir'%s'", dent->d_name); ++ closedir(d); ++ return -1; ++ } ++ else ++ { ++ err = delete_file_dir(subdir_fd, /*skip_lock_file:*/ false); ++ close(subdir_fd); ++ if (err == 0) ++ unlinkat(dir_fd, dent->d_name, AT_REMOVEDIR); ++ } + } + if (errno || err) + { +- perror_msg("Can't remove '%s'", full_path); +- free(full_path); ++ perror_msg("Can't remove '%s'", dent->d_name); + closedir(d); + return -1; + } + } +- free(full_path); + } +- closedir(d); + + /* Here we know for sure that all files/subdirs we found via readdir + * were deleted successfully. If rmdir below fails, we assume someone +@@ -860,29 +913,11 @@ static int delete_file_dir(const char *dir, bool skip_lock_file) + */ + + if (unlink_lock_file) +- { +- char *full_path = concat_path_file(dir, ".lock"); +- xunlink(full_path); +- free(full_path); ++ xunlinkat(dir_fd, ".lock", /*only files*/0); + +- unsigned cnt = RMDIR_FAIL_COUNT; +- do { +- if (rmdir(dir) == 0) +- return 0; +- /* Someone locked the dir after unlink, but before rmdir. +- * This "someone" must be dd_lock(). +- * It detects this (by seeing that there is no time file) +- * and backs off at once. So we need to just retry rmdir, +- * with minimal sleep. +- */ +- usleep(RMDIR_FAIL_USLEEP); +- } while (--cnt != 0); +- } ++ closedir(d); + +- int r = rmdir(dir); +- if (r) +- perror_msg("Can't remove directory '%s'", dir); +- return r; ++ return 0; + } + + int dd_delete(struct dump_dir *dd) +@@ -893,10 +928,34 @@ int dd_delete(struct dump_dir *dd) + return -1; + } + +- int r = delete_file_dir(dd->dd_dirname, /*skip_lock_file:*/ true); ++ if (delete_file_dir(dd->dd_fd, /*skip_lock_file:*/ true) != 0) ++ { ++ perror_msg("Can't remove contents of directory '%s'", dd->dd_dirname); ++ return -2; ++ } ++ ++ unsigned cnt = RMDIR_FAIL_COUNT; ++ do { ++ if (rmdir(dd->dd_dirname) == 0) ++ break; ++ /* Someone locked the dir after unlink, but before rmdir. ++ * This "someone" must be dd_lock(). ++ * It detects this (by seeing that there is no time file) ++ * and backs off at once. So we need to just retry rmdir, ++ * with minimal sleep. ++ */ ++ usleep(RMDIR_FAIL_USLEEP); ++ } while (--cnt != 0); ++ ++ if (cnt == 0) ++ { ++ perror_msg("Can't remove directory '%s'", dd->dd_dirname); ++ return -3; ++ } ++ + dd->locked = 0; /* delete_file_dir already removed .lock */ + dd_close(dd); +- return r; ++ return 0; + } + + int dd_chown(struct dump_dir *dd, uid_t new_uid) +@@ -905,7 +964,7 @@ int dd_chown(struct dump_dir *dd, uid_t new_uid) + error_msg_and_die("dump_dir is not opened"); /* bug */ + + struct stat statbuf; +- if (!(stat(dd->dd_dirname, &statbuf) == 0 && S_ISDIR(statbuf.st_mode))) ++ if (fstat(dd->dd_fd, &statbuf) != 0) + { + perror_msg("stat('%s')", dd->dd_dirname); + return 1; +@@ -926,29 +985,37 @@ int dd_chown(struct dump_dir *dd, uid_t new_uid) + gid_t groups_gid = pw->pw_gid; + #endif + +- int chown_res = lchown(dd->dd_dirname, owners_uid, groups_gid); ++ int chown_res = fchown(dd->dd_fd, owners_uid, groups_gid); + if (chown_res) +- perror_msg("lchown('%s')", dd->dd_dirname); ++ perror_msg("fchown('%s')", dd->dd_dirname); + else + { + dd_init_next_file(dd); +- char *full_name; +- while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/ NULL, &full_name)) ++ char *short_name; ++ while (chown_res == 0 && dd_get_next_file(dd, &short_name, /*full_name*/ NULL)) + { +- log_debug("chowning %s", full_name); +- chown_res = lchown(full_name, owners_uid, groups_gid); ++ /* The current process has to have read access at least */ ++ int fd = secure_openat_read(dd->dd_fd, short_name); ++ if (fd < 0) ++ goto next; ++ ++ log_debug("chowning %s", short_name); ++ ++ chown_res = fchown(fd, owners_uid, groups_gid); + if (chown_res) +- perror_msg("lchown('%s')", full_name); +- free(full_name); ++ perror_msg("fchownat('%s')", short_name); ++ ++ close(fd); ++next: ++ free(short_name); + } + } + + return chown_res; + } + +-static char *load_text_file(const char *path, unsigned flags) ++static char *load_text_from_file_descriptor(int fd, const char *path, int flags) + { +- int fd = open(path, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW)); + if (fd == -1) + { + if (!(flags & DD_FAIL_QUIETLY_ENOENT)) +@@ -1003,6 +1070,20 @@ static char *load_text_file(const char *path, unsigned flags) + return strbuf_free_nobuf(buf_content); + } + ++static char *load_text_file_at(int dir_fd, const char *name, unsigned flags) ++{ ++ assert(name[0] != '/'); ++ ++ const int fd = openat(dir_fd, name, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW)); ++ return load_text_from_file_descriptor(fd, name, flags); ++} ++ ++static char *load_text_file(const char *path, unsigned flags) ++{ ++ const int fd = open(path, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW)); ++ return load_text_from_file_descriptor(fd, path, flags); ++} ++ + static void copy_file_from_chroot(struct dump_dir* dd, const char *name, const char *chroot_dir, const char *file_path) + { + char *chrooted_name = concat_path_file(chroot_dir, file_path); +@@ -1016,14 +1097,16 @@ static void copy_file_from_chroot(struct dump_dir* dd, const char *name, const c + } + } + +-static bool save_binary_file(const char *path, const char* data, unsigned size, uid_t uid, gid_t gid, mode_t mode) ++static bool save_binary_file_at(int dir_fd, const char *name, const char* data, unsigned size, uid_t uid, gid_t gid, mode_t mode) + { ++ assert(name[0] != '/'); ++ + /* the mode is set by the caller, see dd_create() for security analysis */ +- unlink(path); +- int fd = open(path, O_WRONLY | O_TRUNC | O_CREAT | O_NOFOLLOW, mode); ++ unlinkat(dir_fd, name, /*remove only files*/0); ++ int fd = openat(dir_fd, name, O_WRONLY | O_EXCL | O_CREAT | O_NOFOLLOW, mode); + if (fd < 0) + { +- perror_msg("Can't open file '%s'", path); ++ perror_msg("Can't open file '%s'", name); + return false; + } + +@@ -1031,7 +1114,9 @@ static bool save_binary_file(const char *path, const char* data, unsigned size, + { + if (fchown(fd, uid, gid) == -1) + { +- perror_msg("Can't change '%s' ownership to %lu:%lu", path, (long)uid, (long)gid); ++ perror_msg("Can't change '%s' ownership to %lu:%lu", name, (long)uid, (long)gid); ++ close(fd); ++ return false; + } + } + +@@ -1043,14 +1128,16 @@ static bool save_binary_file(const char *path, const char* data, unsigned size, + */ + if (fchmod(fd, mode) == -1) + { +- perror_msg("Can't change mode of '%s'", path); ++ perror_msg("Can't change mode of '%s'", name); ++ close(fd); ++ return false; + } + + unsigned r = full_write(fd, data, size); + close(fd); + if (r != size) + { +- error_msg("Can't save file '%s'", path); ++ error_msg("Can't save file '%s'", name); + return false; + } + +@@ -1075,11 +1162,7 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla + if (strcmp(name, "release") == 0) + name = FILENAME_OS_RELEASE; + +- char *full_path = concat_path_file(dd->dd_dirname, name); +- char *ret = load_text_file(full_path, flags); +- free(full_path); +- +- return ret; ++ return load_text_file_at(dd->dd_fd, name, flags); + } + + char* dd_load_text(const struct dump_dir *dd, const char *name) +@@ -1095,9 +1178,7 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data) + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot save text. '%s' is not a valid file name", name); + +- char *full_path = concat_path_file(dd->dd_dirname, name); +- save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode); +- free(full_path); ++ save_binary_file_at(dd->dd_fd, name, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode); + } + + void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, unsigned size) +@@ -1108,9 +1189,7 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot save binary. '%s' is not a valid file name", name); + +- char *full_path = concat_path_file(dd->dd_dirname, name); +- save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid, dd->mode); +- free(full_path); ++ save_binary_file_at(dd->dd_fd, name, data, size, dd->dd_uid, dd->dd_gid, dd->mode); + } + + long dd_get_item_size(struct dump_dir *dd, const char *name) +@@ -1119,21 +1198,19 @@ long dd_get_item_size(struct dump_dir *dd, const char *name) + error_msg_and_die("Cannot get item size. '%s' is not a valid file name", name); + + long size = -1; +- char *iname = concat_path_file(dd->dd_dirname, name); + struct stat statbuf; ++ int r = fstatat(dd->dd_fd, name, &statbuf, AT_SYMLINK_NOFOLLOW); + +- if (lstat(iname, &statbuf) == 0 && S_ISREG(statbuf.st_mode)) ++ if (r == 0 && S_ISREG(statbuf.st_mode)) + size = statbuf.st_size; + else + { + if (errno == ENOENT) + size = 0; + else +- perror_msg("Can't get size of file '%s'", iname); ++ perror_msg("Can't get size of file '%s'", name); + } + +- free(iname); +- + return size; + } + +@@ -1145,18 +1222,16 @@ int dd_delete_item(struct dump_dir *dd, const char *name) + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot delete item. '%s' is not a valid file name", name); + +- char *path = concat_path_file(dd->dd_dirname, name); +- int res = unlink(path); ++ int res = unlinkat(dd->dd_fd, name, /*only files*/0); + + if (res < 0) + { + if (errno == ENOENT) + errno = res = 0; + else +- perror_msg("Can't delete file '%s'", path); ++ perror_msg("Can't delete file '%s'", name); + } + +- free(path); + return res; + } + +@@ -1164,14 +1239,22 @@ DIR *dd_init_next_file(struct dump_dir *dd) + { + // if (!dd->locked) + // error_msg_and_die("dump_dir is not opened"); /* bug */ ++ int opendir_fd = dup(dd->dd_fd); ++ if (opendir_fd < 0) ++ { ++ perror_msg("dd_init_next_file: dup(dd_fd)"); ++ return NULL; ++ } + + if (dd->next_dir) + closedir(dd->next_dir); + +- dd->next_dir = opendir(dd->dd_dirname); ++ lseek(opendir_fd, SEEK_SET, 0); ++ dd->next_dir = fdopendir(opendir_fd); + if (!dd->next_dir) + { + error_msg("Can't open directory '%s'", dd->dd_dirname); ++ close(opendir_fd); + } + + return dd->next_dir; +@@ -1185,7 +1268,7 @@ int dd_get_next_file(struct dump_dir *dd, char **short_name, char **full_name) + struct dirent *dent; + while ((dent = readdir(dd->next_dir)) != NULL) + { +- if (is_regular_file(dent, dd->dd_dirname)) ++ if (is_regular_file_at(dent, dd->dd_fd)) + { + if (short_name) + *short_name = xstrdup(dent->d_name); +@@ -1250,6 +1333,7 @@ int dd_rename(struct dump_dir *dd, const char *new_path) + return -1; + } + ++ /* Keeps the opened file descriptor valid */ + int res = rename(dd->dd_dirname, new_path); + if (res == 0) + { +@@ -1362,17 +1446,17 @@ int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + +- char *dest = concat_path_file(dd->dd_dirname, name); ++ log_debug("copying '%s' to '%s' at '%s'", source_path, name, dd->dd_dirname); + +- log_debug("copying '%s' to '%s'", source_path, dest); ++ unlinkat(dd->dd_fd, name, /*remove only files*/0); ++ off_t copied = copy_file_ext_at(source_path, dd->dd_fd, name, DEFAULT_DUMP_DIR_MODE, ++ dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_TRUNC | O_EXCL | O_CREAT); + +- off_t copied = copy_file(source_path, dest, DEFAULT_DUMP_DIR_MODE | S_IROTH); + if (copied < 0) +- error_msg("Can't copy %s to %s", source_path, dest); ++ error_msg("Can't copy %s to %s at '%s'", source_path, name, dd->dd_dirname); + else + log_debug("copied %li bytes", (unsigned long)copied); + +- free(dest); + return copied < 0; + } + +@@ -1381,17 +1465,17 @@ int dd_copy_file_unpack(struct dump_dir *dd, const char *name, const char *sourc + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + +- char *dest = concat_path_file(dd->dd_dirname, name); ++ log_debug("unpacking '%s' to '%s' at '%s'", source_path, name, dd->dd_dirname); + +- log_debug("unpacking '%s' to '%s'", source_path, dest); ++ unlinkat(dd->dd_fd, name, /*remove only files*/0); ++ off_t copied = decompress_file_ext_at(source_path, dd->dd_fd, name, DEFAULT_DUMP_DIR_MODE, ++ dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_TRUNC | O_EXCL | O_CREAT); + +- off_t copied = decompress_file(source_path, dest, DEFAULT_DUMP_DIR_MODE | S_IROTH); + if (copied != 0) +- error_msg("Can't copy %s to %s", source_path, dest); ++ error_msg("Can't copy %s to %s at '%s'", source_path, name, dd->dd_dirname); + else +- log_debug("unpackaged file '%s'", dest); ++ log_debug("unpackaged file '%s'", source_path); + +- free(dest); + return copied < 0; + + } +diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c +index ef76406..185e2ae 100644 +--- a/src/lib/problem_data.c ++++ b/src/lib/problem_data.c +@@ -319,14 +319,14 @@ static const char *const always_text_files[] = { + FILENAME_OS_RELEASE, + NULL + }; +-static char* is_text_file(const char *name, ssize_t *sz) ++static char* is_text_file_at(int dir_fd, const char *name, ssize_t *sz) + { + /* We were using magic.h API to check for file being text, but it thinks + * that file containing just "0" is not text (!!) + * So, we do it ourself. + */ + +- int fd = open(name, O_RDONLY); ++ int fd = secure_openat_read(dir_fd, name); + if (fd < 0) + return NULL; /* it's not text (because it does not exist! :) */ + +@@ -439,7 +439,7 @@ void problem_data_load_from_dump_dir(problem_data_t *problem_data, struct dump_d + } + + ssize_t sz = 4*1024; +- char *text = is_text_file(full_name, &sz); ++ char *text = is_text_file_at(dd->dd_fd, short_name, &sz); + if (!text || text == HUGE_TEXT) + { + int flag = !text ? CD_FLAG_BIN : (CD_FLAG_BIN+CD_FLAG_BIGTXT); +diff --git a/src/lib/xfuncs.c b/src/lib/xfuncs.c +index 1ce44aa..979c7b8 100644 +--- a/src/lib/xfuncs.c ++++ b/src/lib/xfuncs.c +@@ -331,6 +331,12 @@ int xopen(const char *pathname, int flags) + return xopen3(pathname, flags, 0666); + } + ++void xunlinkat(int dir_fd, const char *pathname, int flags) ++{ ++ if (unlinkat(dir_fd, pathname, flags)) ++ perror_msg_and_die("Can't remove file '%s'", pathname); ++} ++ + void xunlink(const char *pathname) + { + if (unlink(pathname)) +@@ -359,21 +365,29 @@ int open_or_warn(const char *pathname, int flags) + * do not report the type, they report DT_UNKNOWN for every dirent + * (and this is not a bug in filesystem, this is allowed by standards). + */ +-int is_regular_file(struct dirent *dent, const char *dirname) ++int is_regular_file_at(struct dirent *dent, int dir_fd) + { + if (dent->d_type == DT_REG) + return 1; + if (dent->d_type != DT_UNKNOWN) + return 0; + +- char *fullname = xasprintf("%s/%s", dirname, dent->d_name); + struct stat statbuf; +- int r = lstat(fullname, &statbuf); +- free(fullname); ++ int r = fstatat(dir_fd, dent->d_name, &statbuf, AT_SYMLINK_NOFOLLOW); + + return r == 0 && S_ISREG(statbuf.st_mode); + } + ++int is_regular_file(struct dirent *dent, const char *dirname) ++{ ++ int dir_fd = open(dirname, O_DIRECTORY); ++ if (dir_fd < 0) ++ return 0; ++ int r = is_regular_file_at(dent, dir_fd); ++ close(dir_fd); ++ return r; ++} ++ + /* Is it "." or ".."? */ + /* abrtlib candidate */ + bool dot_or_dotdot(const char *filename) +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index 19584d1..31c320e 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -2,6 +2,162 @@ + + AT_BANNER([dump_dir]) + ++## --------- ## ++## dd_sanity ## ++## --------- ## ++ ++AT_TESTFUN([dd_sanity], ++[[ ++#include "internal_libreport.h" ++#include ++#include ++ ++void validate_dump_dir_contents(struct dump_dir *dd) ++{ ++ int items = 0; ++ assert(dd_exist(dd, FILENAME_TIME)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_KERNEL)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_HOSTNAME)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_ARCHITECTURE)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_OS_INFO)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_OS_RELEASE)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_OS_RELEASE)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_TYPE)); ++ ++items; ++ ++ assert(dd_exist(dd, FILENAME_LAST_OCCURRENCE)); ++ ++items; ++ ++ assert(dd_exist(dd, "at_test_text")); ++ assert(dd_get_item_size(dd, "at_test_text") == 3); ++ ++items; ++ ++ assert(dd_exist(dd, "at_test_binary")); ++ assert(dd_get_item_size(dd, "at_test_binary") == 4); ++ ++items; ++ ++ assert(dd_exist(dd, "at_test_services")); ++ ++items; ++ ++ dd_save_text(dd, "at_test_to_delete", "deleted"); ++ assert(dd_exist(dd, "at_test_to_delete")); ++ dd_delete_item(dd, "at_test_to_delete"); ++ assert(!dd_exist(dd, "at_test_to_delete")); ++ ++ DIR *d1 = dd_init_next_file(dd); ++ assert(d1 != NULL); ++ ++ int counter = 0; ++ char *short_name, *full_name; ++ while (dd_get_next_file(dd, &short_name, &full_name)) ++ { ++ ++counter; ++ ++ ++ printf("Iter = %s\n", short_name); ++ ++ assert(short_name != NULL); ++ assert(full_name != NULL); ++ assert(strcmp(short_name, strrchr(full_name, '/') + 1) == 0); ++ assert(strncmp(dd->dd_dirname, full_name, strlen(dd->dd_dirname)) == 0); ++ assert(full_name[strlen(dd->dd_dirname)] == '/'); ++ } ++ ++ printf("Items = %d, Counter = %d\n", items, counter); ++ assert(items == counter); ++ ++ DIR *d2 = dd_init_next_file(dd); ++ assert(d2 != NULL); ++ ++ while (dd_get_next_file(dd, &short_name, &full_name)) ++ --counter; ++ ++ assert(counter == 0); ++} ++ ++int main(int argc, char **argv) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX/dump_dir"; ++ ++ char *last_slash = strrchr(template, '/'); ++ *last_slash = '\0'; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ *last_slash = '/'; ++ ++ printf("Dump dir path: %s\n", template); ++ ++ fprintf(stderr, "Create new dump directory\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL || !"Cannot create new dump directory"); ++ ++ dd_create_basic_files(dd, geteuid(), NULL); ++ dd_save_text(dd, FILENAME_TYPE, "attest"); ++ ++ dd_save_text(dd, "at_test_text", "foo"); ++ assert(dd_exist(dd, "at_test_text")); ++ ++ dd_save_binary(dd, "at_test_binary", "blah", 4); ++ assert(dd_exist(dd, "at_test_binary")); ++ ++ dd_copy_file(dd, "at_test_services", "/etc/services"); ++ ++ fprintf(stderr, "Test newly created dump directory\n"); ++ validate_dump_dir_contents(dd); ++ dd_close(dd); ++ ++ ++ fprintf(stderr, "Test opened dump directory\n"); ++ dd = dd_opendir(template, /*for writing*/0); ++ assert(dd != NULL || !"Cannot open the dump directory"); ++ validate_dump_dir_contents(dd); ++ dd_close(dd); ++ ++ ++ fprintf(stderr, "Test renamed dump directory\n"); ++ dd = dd_opendir(template, /*for writing*/0); ++ assert(dd != NULL || !"Cannot open the dump directory second time"); ++ ++ *(last_slash+1) = 'X'; ++ assert(dd_rename(dd, template) == 0 || !"Cannot rename the dump directory"); ++ ++ validate_dump_dir_contents(dd); ++ dd_close(dd); ++ ++ ++ fprintf(stderr, "Test opened renamed dump directory\n"); ++ assert(dd != NULL || !"Cannot open the renamed dump directory"); ++ dd = dd_opendir(template, /*for writing*/0); ++ validate_dump_dir_contents(dd); ++ ++ assert(dd_delete(dd) == 0); ++ ++ *last_slash = '\0'; ++ assert(rmdir(template) == 0); ++ return EXIT_SUCCESS; ++} ++]]) ++ + ## -------------- ## + ## recursive_lock ## + ## -------------- ## +-- +2.1.0 + diff --git a/0076-lib-add-alternative-dd-functions-accepting-fds.patch b/0076-lib-add-alternative-dd-functions-accepting-fds.patch new file mode 100644 index 0000000..e18620b --- /dev/null +++ b/0076-lib-add-alternative-dd-functions-accepting-fds.patch @@ -0,0 +1,280 @@ +From 200edbbf40d980ad987fafa753f1c14c8778d8e1 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 24 Apr 2015 11:42:18 +0200 +Subject: [PATCH] lib: add alternative dd functions accepting fds + +Florian Weimer : + + dump_dir_accessible_by_uid() is fundamentally insecure because it + opens up a classic time-of-check-time-of-use race between this + function and and dd_opendir(). At least re-checking after + dd_opendir() with the stored file descriptor is needed. + +Related: #1214745 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 29 +++++++++++ + src/lib/dump_dir.c | 130 ++++++++++++++++++++++++++++++++----------------- + 2 files changed, 114 insertions(+), 45 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index 669f7fd..b613e0b 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -51,6 +51,11 @@ enum { + DD_DONT_WAIT_FOR_LOCK = (1 << 5), + /* Create the new dump directory with parent directories (mkdir -p)*/ + DD_CREATE_PARENTS = (1 << 6), ++ /* Initializes internal data, opens file descriptors and returns the ++ * structure. This flag is useful for testing whether a directory ++ * exists and to perform stat operations. ++ */ ++ DD_OPEN_FD_ONLY = (1 << 7), + }; + + struct dump_dir { +@@ -74,7 +79,21 @@ struct dump_dir { + + void dd_close(struct dump_dir *dd); + ++/* Opens the given path ++ */ + struct dump_dir *dd_opendir(const char *dir, int flags); ++ ++/* Re-opens a dump_dir opened with DD_OPEN_FD_ONLY. ++ * ++ * The passed dump_dir must not be used any more and the return value must be ++ * used instead. ++ * ++ * The passed flags must not contain DD_OPEN_FD_ONLY. ++ * ++ * The passed dump_dir must not be already locked. ++ */ ++struct dump_dir *dd_fdopendir(struct dump_dir *dd, int flags); ++ + struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int flags); + int dd_reset_ownership(struct dump_dir *dd); + /* Pass uid = (uid_t)-1L to disable chown'ing of newly created files +@@ -155,6 +174,11 @@ void delete_dump_dir(const char *dirname); + * Returns non zero if dump dir is accessible otherwise return 0 value. + */ + int dump_dir_accessible_by_uid(const char *dirname, uid_t uid); ++/* Returns the same information as dump_dir_accessible_by_uid ++ * ++ * The passed dump_dir can be opened with DD_OPEN_FD_ONLY ++ */ ++int dd_accessible_by_uid(struct dump_dir *dd, uid_t uid); + + enum { + DD_STAT_ACCESSIBLE_BY_UID = 1, +@@ -169,6 +193,11 @@ enum { + * Returns negative number if error occurred otherwise returns 0 or positive number. + */ + int dump_dir_stat_for_uid(const char *dirname, uid_t uid); ++/* Returns the same information as dump_dir_stat_for_uid ++ * ++ * The passed dump_dir can be opened with DD_OPEN_FD_ONLY ++ */ ++int dd_stat_for_uid(struct dump_dir *dd, uid_t uid); + + /* creates not_reportable file in the problem directory and saves the + reason to it, which prevents libreport from reporting the problem +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 7cc918a..4dcfa66 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -438,20 +438,41 @@ static char* rm_trailing_slashes(const char *dir) + return xstrndup(dir, len); + } + +-struct dump_dir *dd_opendir(const char *dir, int flags) ++static struct dump_dir *dd_do_open(struct dump_dir *dd, const char *dir, int flags) + { +- struct dump_dir *dd = dd_init(); ++ if (dir != NULL) ++ { ++ dd->dd_dirname = rm_trailing_slashes(dir); ++ /* dd_do_open validates dd_fd */ ++ dd->dd_fd = open(dd->dd_dirname, O_DIRECTORY | O_NOFOLLOW); + +- dir = dd->dd_dirname = rm_trailing_slashes(dir); +- dd->dd_fd = open(dir, O_DIRECTORY | O_NOFOLLOW); +- struct stat stat_buf; +- if (dd->dd_fd < 0) +- goto cant_access; +- if (fstat(dd->dd_fd, &stat_buf) != 0) +- goto cant_access; ++ struct stat stat_buf; ++ if (dd->dd_fd < 0) ++ goto cant_access; ++ if (fstat(dd->dd_fd, &stat_buf) != 0) ++ goto cant_access; ++ ++ /* & 0666 should remove the executable bit */ ++ dd->mode = (stat_buf.st_mode & 0666); ++ ++ dd->dd_uid = (uid_t)-1L; ++ dd->dd_gid = (gid_t)-1L; ++ if (geteuid() == 0) ++ { ++ /* In case caller would want to create more files, he'll need uid:gid */ ++ if (fstat(dd->dd_fd, &stat_buf) != 0) ++ { ++ error_msg("Can't stat '%s'", dd->dd_dirname); ++ dd_close(dd); ++ return NULL; ++ } ++ dd->dd_uid = stat_buf.st_uid; ++ dd->dd_gid = stat_buf.st_gid; ++ } + +- /* & 0666 should remove the executable bit */ +- dd->mode = (stat_buf.st_mode & 0666); ++ if ((flags & DD_OPEN_FD_ONLY)) ++ return dd; ++ } + + errno = 0; + if (dd_lock(dd, WAIT_FOR_OTHER_PROCESS_USLEEP, flags) < 0) +@@ -482,7 +503,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + * directory when run without arguments, because its option -d DIR + * defaults to "."! + */ +- error_msg("'%s' is not a problem directory", dir); ++ error_msg("'%s' is not a problem directory", dd->dd_dirname); + } + else + { +@@ -490,36 +511,41 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + if (errno == ENOENT || errno == ENOTDIR) + { + if (!(flags & DD_FAIL_QUIETLY_ENOENT)) +- error_msg("'%s' does not exist", dir); ++ error_msg("'%s' does not exist", dd->dd_dirname); + } + else + { + if (!(flags & DD_FAIL_QUIETLY_EACCES)) +- perror_msg("Can't access '%s'", dir); ++ perror_msg("Can't access '%s'", dd->dd_dirname); + } + } + dd_close(dd); + return NULL; + } + +- dd->dd_uid = (uid_t)-1L; +- dd->dd_gid = (gid_t)-1L; +- if (geteuid() == 0) +- { +- /* In case caller would want to create more files, he'll need uid:gid */ +- if (fstat(dd->dd_fd, &stat_buf) != 0) +- { +- error_msg("Can't stat '%s'", dir); +- dd_close(dd); +- return NULL; +- } +- dd->dd_uid = stat_buf.st_uid; +- dd->dd_gid = stat_buf.st_gid; +- } +- + return dd; + } + ++struct dump_dir *dd_fdopendir(struct dump_dir *dd, int flags) ++{ ++ if ((flags & DD_OPEN_FD_ONLY)) ++ error_msg_and_die("the passed flags must not contain DD_OPEN_FD_ONLY"); ++ ++ if (dd->dd_fd < 0) ++ error_msg_and_die("the dump directory was not initialized yet"); ++ ++ if (dd->locked) ++ error_msg_and_die("the dump directory is already locked"); ++ ++ return dd_do_open(dd, NULL, flags); ++} ++ ++struct dump_dir *dd_opendir(const char *dir, int flags) ++{ ++ struct dump_dir *dd = dd_init(); ++ return dd_do_open(dd, dir, flags); ++} ++ + /* Create a fresh empty debug dump dir which is owned bu the calling user. If + * you want to create the directory with meaningful ownership you should + * consider using dd_create() function or you can modify the ownership +@@ -1384,32 +1410,22 @@ static bool uid_in_group(uid_t uid, gid_t gid) + } + #endif + +-int dump_dir_stat_for_uid(const char *dirname, uid_t uid) ++int dd_stat_for_uid(struct dump_dir *dd, uid_t uid) + { +- struct stat statbuf; +- if (stat(dirname, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode)) +- { +- log_debug("can't get stat of '%s': not a problem directory", dirname); +- errno = ENOTDIR; +- return -1; +- } +- +- errno = 0; +- + int ddstat = 0; +- if (uid == 0 || (statbuf.st_mode & S_IROTH)) ++ if (uid == 0 || (dd->mode & S_IROTH)) + { +- log_debug("directory '%s' is accessible by %ld uid", dirname, (long)uid); ++ log_debug("directory is accessible by %ld uid", (long)uid); + ddstat |= DD_STAT_ACCESSIBLE_BY_UID; + } + + #if DUMP_DIR_OWNED_BY_USER > 0 +- if (uid == statbuf.st_uid) ++ if (uid == dd->dd_uid) + #else +- if (uid_in_group(uid, statbuf.st_gid)) ++ if (uid_in_group(uid, dd->dd_gid)) + #endif + { +- log_debug("%ld uid owns directory '%s'", (long)uid, dirname); ++ log_debug("%ld uid owns directory", (long)uid); + ddstat |= DD_STAT_ACCESSIBLE_BY_UID; + ddstat |= DD_STAT_OWNED_BY_UID; + } +@@ -1417,6 +1433,30 @@ int dump_dir_stat_for_uid(const char *dirname, uid_t uid) + return ddstat; + } + ++int dump_dir_stat_for_uid(const char *dirname, uid_t uid) ++{ ++ struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_FD_ONLY); ++ if (dd == NULL) ++ return -1; ++ ++ int r = dd_stat_for_uid(dd, uid); ++ dd_close(dd); ++ ++ return r; ++} ++ ++int dd_accessible_by_uid(struct dump_dir *dd, uid_t uid) ++{ ++ int ddstat = dd_stat_for_uid(dd, uid); ++ ++ if (ddstat >= 0) ++ return ddstat & DD_STAT_ACCESSIBLE_BY_UID; ++ ++ VERB3 pwarn_msg("can't determine accessibility for %ld uid", (long)uid); ++ ++ return 0; ++} ++ + int dump_dir_accessible_by_uid(const char *dirname, uid_t uid) + { + int ddstat = dump_dir_stat_for_uid(dirname, uid); +-- +2.1.0 + diff --git a/0077-build-switch-the-default-dump-dir-mode-to-0640.patch b/0077-build-switch-the-default-dump-dir-mode-to-0640.patch new file mode 100644 index 0000000..c901ad9 --- /dev/null +++ b/0077-build-switch-the-default-dump-dir-mode-to-0640.patch @@ -0,0 +1,36 @@ +From d70f04d50bc65dfd839316dbe2668ff195cf1fef Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 28 Apr 2015 12:49:38 +0200 +Subject: [PATCH] build: switch the default dump dir mode to 0640 + +The 0660 allows root escalations in ABRT. We don't really need to have +the dump directories writable for the group as ABRT processes run under +root. We introduced 0x1 for group with the switch to /var/tmp/abrt +because we thought that we will have ABRT processes run under the user +abrt, but there are no signs that we will ever pursue such a setup. + +Related: #1212861 + +Signed-off-by: Jakub Filak +--- + configure.ac | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 01d9f83..4f7e504 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -282,8 +282,8 @@ AC_PATH_PROG(AUGPARSE, augparse, no) + + AC_ARG_WITH([defaultdumpdirmode], + AS_HELP_STRING([--with-defaultdumpdirmode=OCTAL-MODE], +- [Default dump dir mode (default: 0660)]), +- [], [with_defaultdumpdirmode="0660"]) ++ [Default dump dir mode (default: 0640)]), ++ [], [with_defaultdumpdirmode="0640"]) + AC_SUBST([DEFAULT_DUMP_DIR_MODE], [$with_defaultdumpdirmode]) + + DUMP_DIR_OWNED_BY_USER=1 +-- +2.1.0 + diff --git a/0078-lib-parse-fsgid-and-don-t-fall-back-to-0.patch b/0078-lib-parse-fsgid-and-don-t-fall-back-to-0.patch new file mode 100644 index 0000000..ba8dc83 --- /dev/null +++ b/0078-lib-parse-fsgid-and-don-t-fall-back-to-0.patch @@ -0,0 +1,90 @@ +From b3f2d99ce6ab19b21a115decac0495d659153837 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 14 May 2015 10:11:36 +0200 +Subject: [PATCH] lib: parse fsgid and don't fall back to 0 + +Let users of API to fall back to 0 on their own and provide them with a +function for getting fsgid from /proc/[pid]/status. + +Thanks Florian Weimer + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 2 ++ + src/lib/get_cmdline.c | 32 ++++++++++++++++++++++++-------- + 2 files changed, 26 insertions(+), 8 deletions(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 66f91e9..25af681 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -660,6 +660,8 @@ char* get_cwd(pid_t pid); + char* get_rootdir(pid_t pid); + #define get_fsuid libreport_get_fsuid + int get_fsuid(const char *proc_pid_status); ++#define get_fsgid libreport_get_fsgid ++int get_fsgid(const char *proc_pid_status); + #define dump_fd_info_ext libreport_dump_fd_info_ext + int dump_fd_info_ext(const char *dest_filename, const char *proc_pid_fd_path, uid_t uid, gid_t gid); + #define dump_fd_info libreport_dump_fd_info +diff --git a/src/lib/get_cmdline.c b/src/lib/get_cmdline.c +index c55de30..5547877 100644 +--- a/src/lib/get_cmdline.c ++++ b/src/lib/get_cmdline.c +@@ -188,21 +188,26 @@ char* get_rootdir(pid_t pid) + return malloc_readlink(buf); + } + +-int get_fsuid(const char *proc_pid_status) ++static int get_proc_fs_id(const char *proc_pid_status, char type) + { +- int real, euid, saved; +- /* if we fail to parse the uid, then make it root only readable to be safe */ +- int fs_uid = 0; ++ char id_type[] = "_id"; ++ id_type[0] = type; ++ ++ int real, e_id, saved; ++ int fs_id = 0; + + const char *line = proc_pid_status; /* never NULL */ + for (;;) + { +- if (strncmp(line, "Uid", 3) == 0) ++ if (strncmp(line, id_type, 3) == 0) + { +- int n = sscanf(line, "Uid:\t%d\t%d\t%d\t%d\n", &real, &euid, &saved, &fs_uid); ++ int n = sscanf(line, "%*cid:\t%d\t%d\t%d\t%d\n", &real, &e_id, &saved, &fs_id); + if (n != 4) ++ { ++ error_msg("Failed to parser /proc/[pid]/status: invalid format of '%cui:' line", type); + return -1; +- break; ++ } ++ return fs_id; + } + line = strchr(line, '\n'); + if (!line) +@@ -210,7 +215,18 @@ int get_fsuid(const char *proc_pid_status) + line++; + } + +- return fs_uid; ++ error_msg("Failed to parser /proc/[pid]/status: not found '%cui:' line", type); ++ return -2; ++} ++ ++int get_fsuid(const char *proc_pid_status) ++{ ++ return get_proc_fs_id(proc_pid_status, /*UID*/'U'); ++} ++ ++int get_fsgid(const char *proc_pid_status) ++{ ++ return get_proc_fs_id(proc_pid_status, /*GID*/'G'); + } + + int dump_fd_info_ext(const char *dest_filename, const char *proc_pid_fd_path, uid_t uid, gid_t gid) +-- +2.1.0 + diff --git a/0079-dd-add-support-for-meta-data.patch b/0079-dd-add-support-for-meta-data.patch new file mode 100644 index 0000000..0bc7927 --- /dev/null +++ b/0079-dd-add-support-for-meta-data.patch @@ -0,0 +1,931 @@ +From fdc64c5aafe435e34c4369ae8504d98ff57d7350 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 28 May 2015 12:32:01 +0200 +Subject: [PATCH] dd: add support for meta-data + +Store the meta-data as plain text files in the dump directory's +sub-directory called '.libreport'. + +I chose this name to allow other tools (which do not exist yet) to create +their own directories and files. + +Related: #354 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 4 + + src/lib/dump_dir.c | 511 ++++++++++++++++++++++++++++++++++++++++++------- + tests/dump_dir.at | 171 +++++++++++++++++ + 3 files changed, 618 insertions(+), 68 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index b613e0b..f33eb99 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -75,6 +75,10 @@ struct dump_dir { + */ + int owns_lock; + int dd_fd; ++ /* Never use this member directly, it is intialized on demand in ++ * dd_get_meta_data_dir_fd() ++ */ ++ int dd_md_fd; + }; + + void dd_close(struct dump_dir *dd); +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 4dcfa66..aed2de0 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -83,6 +83,24 @@ + #define RMDIR_FAIL_USLEEP (10*1000) + #define RMDIR_FAIL_COUNT 50 + ++// A sub-directory of a dump directory where the meta-data such as owner are ++// stored. The meta-data directory must have same owner, group and mode as its ++// parent dump directory. It is not a fatal error, if the meta-data directory ++// does not exist (backward compatibility). ++#define META_DATA_DIR_NAME ".libreport" ++ ++enum { ++ /* Try to create meta-data dir if it does not exist */ ++ DD_MD_GET_CREATE = 1 << 0, ++}; ++ ++// a little trick to copy read bits from file mode to exec bit of dir mode ++// * mode of dump directory is in the form of 640 (no X) because we create ++// files more often then we play with directories ++// * so if we want to get real mode of the directory we have to copy the read ++// bits ++#define DD_MODE_TO_DIR_MODE(mode) ((mode) | (((mode) & 0444) >> 2)) ++ + + static char *load_text_file(const char *path, unsigned flags); + static char *load_text_file_at(int dir_fd, const char *name, unsigned flags); +@@ -112,6 +130,12 @@ static bool exist_file_dir_at(int dir_fd, const char *name) + return false; + } + ++/* A valid dump dir element name is correct filename and is not a name of any ++ * internal file or directory. ++ */ ++#define dd_validate_element_name(name) \ ++ (str_is_correct_filename(name) && (strcmp(META_DATA_DIR_NAME, name) != 0)) ++ + /* Opens the file in the three following steps: + * 1. open the file with O_PATH (get a file descriptor for operations with + * inode) and O_NOFOLLOW (do not dereference symbolick links) +@@ -397,18 +421,28 @@ static inline struct dump_dir *dd_init(void) + struct dump_dir* dd = (struct dump_dir*)xzalloc(sizeof(struct dump_dir)); + dd->dd_time = -1; + dd->dd_fd = -1; ++ dd->dd_md_fd = -1; + return dd; + } + + int dd_exist(const struct dump_dir *dd, const char *name) + { +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + + const int ret = exist_file_dir_at(dd->dd_fd, name); + return ret; + } + ++static void dd_close_meta_data_dir(struct dump_dir *dd) ++{ ++ if (dd->dd_md_fd < 0) ++ return; ++ ++ close(dd->dd_md_fd); ++ dd->dd_md_fd = -1; ++} ++ + void dd_close(struct dump_dir *dd) + { + if (!dd) +@@ -419,6 +453,8 @@ void dd_close(struct dump_dir *dd) + if (dd->dd_fd >= 0) + close(dd->dd_fd); + ++ dd_close_meta_data_dir(dd); ++ + if (dd->next_dir) + { + closedir(dd->next_dir); +@@ -430,6 +466,255 @@ void dd_close(struct dump_dir *dd) + free(dd); + } + ++static int dd_create_subdir(int dd_fd, const char *dirname, uid_t dd_uid, gid_t dd_gid, mode_t dd_mode) ++{ ++ if (mkdirat(dd_fd, dirname, dd_mode) < 0) ++ { ++ perror_msg("Can't create directory '%s'", dirname); ++ return -1; ++ } ++ ++ int dd_md_fd = openat(dd_fd, dirname, O_DIRECTORY | O_NOFOLLOW); ++ if (dd_md_fd < 0) ++ { ++ perror_msg("Can't open newly created directory '%s'", dirname); ++ goto fail_open; ++ } ++ ++ if (dd_uid != (uid_t)-1) ++ { ++ if (fchown(dd_md_fd, dd_uid, dd_gid) != 0) ++ { ++ perror_msg("Can't change owner and group of '%s'", dirname); ++ goto fail_modify; ++ } ++ } ++ ++ /* mkdir's mode (above) can be affected by umask, fix it */ ++ if (fchmod(dd_md_fd, dd_mode) == -1) ++ { ++ perror_msg("Can't change mode of '%s'", dirname); ++ goto fail_modify; ++ } ++ ++ return dd_md_fd; ++fail_modify: ++ close(dd_md_fd); ++fail_open: ++ if (unlinkat(dd_fd, dirname, AT_REMOVEDIR) < 0) ++ perror_msg("Fialed to unlink '%s' while cleaning up after failure", dirname); ++ return -1; ++} ++ ++/* Opens the meta-data directory, checks its file system attributes and returns ++ * its file descriptor. ++ * ++ * The meta-data directory must have the same file system attributes as the ++ * parent dump directory in order to avoid unexpected situations and detects ++ * program errors (it is an error to modify bits of the dump directory and ++ * forgot to update the meta-data directory). ++ * ++ * Keep on mind that the old dump directories might miss the meta-data directory ++ * so the return value -ENOENT does not necessarily need to be fatal. ++ */ ++static int dd_open_meta_data_dir(struct dump_dir *dd) ++{ ++ int md_dir_fd = openat(dd->dd_fd, META_DATA_DIR_NAME, O_DIRECTORY | O_NOFOLLOW); ++ if (md_dir_fd < 0) ++ { ++ md_dir_fd = -errno; ++ ++ /* ENOENT is not critical */ ++ if (errno != ENOENT) ++ log_warning("Can't open meta-data '"META_DATA_DIR_NAME"'"); ++ else ++ log_info("The dump dir doesn't contain '"META_DATA_DIR_NAME"'"); ++ ++ goto finito; ++ } ++ ++ struct stat md_sb; ++ if (fstat(md_dir_fd, &md_sb) < 0) ++ { ++ log_debug("Can't stat '"META_DATA_DIR_NAME"'"); ++ goto fail; ++ } ++ ++ /* Test only permission bits, ignore SUID, SGID, etc. */ ++ const mode_t md_mode = md_sb.st_mode & 0777; ++ const mode_t dd_mode = DD_MODE_TO_DIR_MODE(dd->mode); ++ ++ if ( md_sb.st_uid != dd->dd_uid ++ || md_sb.st_gid != dd->dd_gid ++ || md_mode != dd_mode) ++ { ++ log_debug("'"META_DATA_DIR_NAME"' has different attributes than the dump dir, '%d'='%d', '%d'='%d', %o = %o", ++ md_sb.st_uid, dd->dd_uid, md_sb.st_gid, dd->dd_gid, md_mode, dd_mode); ++ goto fail; ++ } ++ ++finito: ++ return md_dir_fd; ++ ++fail: ++ close(md_dir_fd); ++ ++ return -EINVAL; ++} ++ ++/* Returns a file descriptor to the meta-data directory. Can be configured to ++ * create the directory if it does not exist. ++ * ++ * This function enables lazy initialization of the meta-data directory. ++ */ ++static int dd_get_meta_data_dir_fd(struct dump_dir *dd, int flags) ++{ ++ if (dd->dd_md_fd < 0) ++ { ++ dd->dd_md_fd = dd_open_meta_data_dir(dd); ++ ++ if ( dd->dd_md_fd == -ENOENT ++ && (flags & DD_MD_GET_CREATE)) ++ { ++ dd->dd_md_fd = dd_create_subdir(dd->dd_fd, ++ META_DATA_DIR_NAME, ++ dd->dd_uid, ++ dd->dd_gid, ++ DD_MODE_TO_DIR_MODE(dd->mode)); ++ } ++ } ++ ++ return dd->dd_md_fd; ++} ++ ++/* A helper function useful for traversing directories. ++ * ++ * DIR* d opendir(dir_fd); ... closedir(d); closes also dir_fd but we want to ++ * keep it opened. ++ */ ++static int fdreopen(int dir_fd, DIR **d) ++{ ++ int opendir_fd = dup(dir_fd); ++ if (opendir_fd < 0) ++ { ++ perror_msg("dup(dir_fd)"); ++ return -EBADFD; ++ } ++ ++ lseek(opendir_fd, SEEK_SET, 0); ++ *d = fdopendir(opendir_fd); ++ if (!*d) ++ { ++ int ret = -errno; ++ close(opendir_fd); ++ perror_msg("fdopendir(dir_fd)"); ++ return ret; ++ } ++ ++ /* 'opendir_fd' will be closed with 'd' */ ++ return 0; ++} ++ ++/* A macro for going through the entries of a directory referenced as a file ++ * descriptor. ++ * ++ * Usage: ++ * ++ * FOREACH_REGULAR_FILE_AS_FD_AT_BEGIN(dir_fd) ++ * { ++ * printf("Short name '%s'", dent->d_name); ++ * printf("File descriptor %d", fd); ++ * } ++ * FOREACH_REGULAR_FILE_AS_FD_AT_END ++ */ ++ ++#define FOREACH_REGULAR_FILE_AS_FD_AT_BEGIN(dir_fd) \ ++ DIR *d; \ ++ struct dirent *dent; \ ++ if (fdreopen(dir_fd, &d) < 0) return -1; \ ++ while ((dent = readdir(d)) != NULL) \ ++ { \ ++ if (dot_or_dotdot(dent->d_name)) continue; \ ++ int fd = secure_openat_read(dirfd(d), dent->d_name); \ ++ if (fd >= 0) ++ ++#define FOREACH_REGULAR_FILE_AS_FD_AT_END \ ++ close(fd); \ ++ } \ ++ closedir(d); ++ ++ ++/* Sets attributes of the meta-data directory and its contents to the same ++ * attributes of the parent dump directory. ++ */ ++static int dd_sanitize_mode_meta_data(struct dump_dir *dd) ++{ ++ if (!dd->locked) ++ error_msg_and_die("%s: dump_dir is not opened", __func__); /* bug */ ++ ++ int dd_md_fd = dd_get_meta_data_dir_fd(dd, /*no create*/0); ++ if (dd_md_fd < 0) ++ return 0; ++ ++ int res = fchmod(dd_md_fd, DD_MODE_TO_DIR_MODE(dd->mode)); ++ if (res < 0) ++ { ++ perror_msg("Failed to chmod meta-data sub-dir"); ++ return res; ++ } ++ ++ FOREACH_REGULAR_FILE_AS_FD_AT_BEGIN(dd_md_fd) ++ { ++ log_debug("chmoding %s", dent->d_name); ++ ++ res = fchmod(fd, dd->mode); ++ if (res) ++ { ++ perror_msg("fchmod('%s')", dent->d_name); ++ break; ++ } ++ } ++ FOREACH_REGULAR_FILE_AS_FD_AT_END ++ ++ return 0; ++} ++ ++/* Sets owner and group of the meta-data directory and its contents to the same ++ * attributes of the parent dump directory. ++ */ ++ ++static int dd_chown_meta_data(struct dump_dir *dd, uid_t uid, gid_t gid) ++{ ++ if (!dd->locked) ++ error_msg_and_die("%s: dump_dir is not opened", __func__); /* bug */ ++ ++ int dd_md_fd = dd_get_meta_data_dir_fd(dd, /*no create*/0); ++ if (dd_md_fd < 0) ++ return 0; ++ ++ int res = fchown(dd_md_fd, uid, gid); ++ if (res < 0) ++ { ++ perror_msg("Failed to chown meta-data sub-dir"); ++ return res; ++ } ++ ++ FOREACH_REGULAR_FILE_AS_FD_AT_BEGIN(dd_md_fd) ++ { ++ log_debug("%s: chowning %s", __func__, dent->d_name); ++ ++ res = fchown(fd, uid, gid); ++ if (res) ++ { ++ perror_msg("fchown('%s')", dent->d_name); ++ break; ++ } ++ } ++ FOREACH_REGULAR_FILE_AS_FD_AT_END ++ ++ return res; ++} ++ + static char* rm_trailing_slashes(const char *dir) + { + unsigned len = strlen(dir); +@@ -455,45 +740,41 @@ static struct dump_dir *dd_do_open(struct dump_dir *dd, const char *dir, int fla + /* & 0666 should remove the executable bit */ + dd->mode = (stat_buf.st_mode & 0666); + +- dd->dd_uid = (uid_t)-1L; +- dd->dd_gid = (gid_t)-1L; ++ /* We want to have dd_uid and dd_gid always initialized. But we have to ++ * initialize it in the way which does not prevent non-privileged user ++ * from saving data in their dump directories. ++ * ++ * Non-privileged users are not allowed to change the group to ++ * 'abrt' so we have to use their GID. ++ * ++ * If the caller is super-user, we have to use dd's fs owner and fs ++ * group, because he can do everything and the data must be readable by ++ * the real owner. ++ * ++ * We always use fs uid, because non-privileged users must own the ++ * directory and super-user must use fs owner. ++ */ ++ dd->dd_uid = stat_buf.st_uid; ++ ++ /* We use fs group only if the caller is super-user, because we want to ++ * make sure non-privileged users can modify elements (libreport call ++ * chown(dd_uid, dd_gid) after modifying an element) and the modified ++ * elements do not have super-user's group. ++ */ ++ dd->dd_gid = getegid(); + if (geteuid() == 0) +- { +- /* In case caller would want to create more files, he'll need uid:gid */ +- if (fstat(dd->dd_fd, &stat_buf) != 0) +- { +- error_msg("Can't stat '%s'", dd->dd_dirname); +- dd_close(dd); +- return NULL; +- } +- dd->dd_uid = stat_buf.st_uid; + dd->dd_gid = stat_buf.st_gid; +- } + + if ((flags & DD_OPEN_FD_ONLY)) ++ { ++ dd->dd_md_fd = dd_open_meta_data_dir(dd); + return dd; ++ } + } + + errno = 0; + if (dd_lock(dd, WAIT_FOR_OTHER_PROCESS_USLEEP, flags) < 0) + { +- if ((flags & DD_OPEN_READONLY) && errno == EACCES) +- { +- /* Directory is not writable. If it seems to be readable, +- * return "read only" dd, not NULL +- * +- * Does the directory have 'r' flag? +- */ +- if (faccessat(dd->dd_fd, ".", R_OK, AT_SYMLINK_NOFOLLOW) == 0) +- { +- if(dd_check(dd) != NULL) +- { +- dd_close(dd); +- dd = NULL; +- } +- return dd; +- } +- } + if (errno == EISDIR) + { + /* EISDIR: dd_lock can lock the dir, but it sees no time file there, +@@ -504,26 +785,56 @@ static struct dump_dir *dd_do_open(struct dump_dir *dd, const char *dir, int fla + * defaults to "."! + */ + error_msg("'%s' is not a problem directory", dd->dd_dirname); ++ goto fail_with_close; + } +- else ++ ++ if (!(flags & DD_OPEN_READONLY)) + { +- cant_access: +- if (errno == ENOENT || errno == ENOTDIR) +- { +- if (!(flags & DD_FAIL_QUIETLY_ENOENT)) +- error_msg("'%s' does not exist", dd->dd_dirname); +- } +- else +- { +- if (!(flags & DD_FAIL_QUIETLY_EACCES)) +- perror_msg("Can't access '%s'", dd->dd_dirname); +- } ++ log_debug("'%s' can't be opened for writing", dd->dd_dirname); ++ goto fail_with_close; + } +- dd_close(dd); +- return NULL; ++ ++ if (errno != EACCES) ++ { ++ VERB3 perror_msg("failed to lock dump directory '%s'", dd->dd_dirname); ++ goto fail_with_close; ++ } ++ ++ /* Directory is not writable. If it seems to be readable, ++ * return "read only" dd, not NULL ++ * ++ * Does the directory have 'r' flag? ++ */ ++ if (faccessat(dd->dd_fd, ".", R_OK, AT_SYMLINK_NOFOLLOW) != 0) ++ { ++ VERB3 perror_msg("failed to lock dump directory '%s'", dd->dd_dirname); ++ goto fail_with_close; ++ } ++ ++ /* dd_check prints out good log messages */ ++ if(dd_check(dd) != NULL) ++ goto fail_with_close; ++ ++ /* The dd is opened in READONLY moded, continue.*/ + } + + return dd; ++ ++cant_access: ++ if (errno == ENOENT || errno == ENOTDIR) ++ { ++ if (!(flags & DD_FAIL_QUIETLY_ENOENT)) ++ error_msg("'%s' does not exist", dd->dd_dirname); ++ } ++ else ++ { ++ if (!(flags & DD_FAIL_QUIETLY_EACCES)) ++ perror_msg("Can't access '%s'", dd->dd_dirname); ++ } ++ ++fail_with_close: ++ dd_close(dd); ++ return NULL; + } + + struct dump_dir *dd_fdopendir(struct dump_dir *dd, int flags) +@@ -607,8 +918,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags) + */ + struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int flags) + { +- /* a little trick to copy read bits from file mode to exec bit of dir mode*/ +- mode_t dir_mode = mode | ((mode & 0444) >> 2); ++ mode_t dir_mode = DD_MODE_TO_DIR_MODE(mode); + struct dump_dir *dd = dd_init(); + + dd->mode = mode; +@@ -673,8 +983,25 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + goto fail; + } + +- dd->dd_uid = (uid_t)-1L; +- dd->dd_gid = (gid_t)-1L; ++ /* Initiliaze dd_uid and dd_gid to sane values which reflect the reality. ++ */ ++ dd->dd_uid = stat_sb.st_uid; ++ dd->dd_gid = stat_sb.st_gid; ++ ++ /* Create META-DATA directory with real fs attributes which must be changed ++ * in dd_reset_ownership(), when populating of a new dump directory is ++ * done. ++ * ++ * It allows daemons to create a dump directory, populate the directory as ++ * root and then switch the ownership to the real user. ++ */ ++ dd->dd_md_fd = dd_create_subdir(dd->dd_fd, META_DATA_DIR_NAME, dd->dd_uid, dd->dd_gid, dir_mode); ++ if (dd->dd_md_fd < 0) ++ { ++ error_msg("Can't create meta-data directory"); ++ goto fail; ++ } ++ + if (uid != (uid_t)-1L) + { + dd->dd_uid = 0; +@@ -726,12 +1053,16 @@ int dd_reset_ownership(struct dump_dir *dd) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- const int r = fchown(dd->dd_fd, dd->dd_uid, dd->dd_gid); ++ int r = fchown(dd->dd_fd, dd->dd_uid, dd->dd_gid); + if (r < 0) + { + perror_msg("Can't change '%s' ownership to %lu:%lu", dd->dd_dirname, + (long)dd->dd_uid, (long)dd->dd_gid); + } ++ ++ if (dd_chown_meta_data(dd, dd->dd_uid, dd->dd_gid) != 0) ++ error_msg("Failed to reset ownership of meta-data"); ++ + return r; + } + +@@ -864,27 +1195,27 @@ void dd_sanitize_mode_and_owner(struct dump_dir *dd) + next: + free(short_name); + } ++ ++ /* No need to check return value, the functions print good messages. ++ * There are two approaches for handling errors in libreport: ++ * - print out a warning message and keep status quo ++ * - terminate the process ++ */ ++ dd_sanitize_mode_meta_data(dd); ++ dd_chown_meta_data(dd, dd->dd_uid, dd->dd_gid); + } + + static int delete_file_dir(int dir_fd, bool skip_lock_file) + { +- int opendir_fd = dup(dir_fd); +- if (opendir_fd < 0) ++ DIR *d; ++ int ret = fdreopen(dir_fd, &d); ++ if (ret < 0) + { +- perror_msg("delete_file_dir: dup(dir_fd)"); +- return -1; +- } +- +- lseek(opendir_fd, SEEK_SET, 0); +- DIR *d = fdopendir(opendir_fd); +- if (!d) +- { +- close(opendir_fd); + /* The caller expects us to error out only if the directory + * still exists (not deleted). If directory + * *doesn't exist*, return 0 and clear errno. + */ +- if (errno == ENOENT || errno == ENOTDIR) ++ if (ret == -ENOENT || ret == -ENOTDIR) + { + errno = 0; + return 0; +@@ -946,6 +1277,35 @@ static int delete_file_dir(int dir_fd, bool skip_lock_file) + return 0; + } + ++static int dd_delete_meta_data(struct dump_dir *dd) ++{ ++ if (!dd->locked) ++ { ++ error_msg("Can't remove meta-data of unlocked problem directory %s", dd->dd_dirname); ++ return -1; ++ } ++ ++ int dd_md_fd = dd_get_meta_data_dir_fd(dd, /*no create*/0); ++ if (dd_md_fd < 0) ++ return 0; ++ ++ if (delete_file_dir(dd_md_fd, /*skip_lock_file:*/ true) != 0) ++ { ++ perror_msg("Can't remove meta-data from '"META_DATA_DIR_NAME"'"); ++ return -2; ++ } ++ ++ dd_close_meta_data_dir(dd); ++ ++ if (unlinkat(dd->dd_fd, META_DATA_DIR_NAME, AT_REMOVEDIR)) ++ { ++ perror_msg("Can't remove meta-data directory '"META_DATA_DIR_NAME"'"); ++ return -3; ++ } ++ ++ return 0; ++} ++ + int dd_delete(struct dump_dir *dd) + { + if (!dd->locked) +@@ -954,6 +1314,9 @@ int dd_delete(struct dump_dir *dd) + return -1; + } + ++ if (dd_delete_meta_data(dd) != 0) ++ return -2; ++ + if (delete_file_dir(dd->dd_fd, /*skip_lock_file:*/ true) != 0) + { + perror_msg("Can't remove contents of directory '%s'", dd->dd_dirname); +@@ -1029,7 +1392,10 @@ int dd_chown(struct dump_dir *dd, uid_t new_uid) + + chown_res = fchown(fd, owners_uid, groups_gid); + if (chown_res) ++ { + perror_msg("fchownat('%s')", short_name); ++ break; ++ } + + close(fd); + next: +@@ -1037,6 +1403,15 @@ next: + } + } + ++ if (chown_res == 0) ++ chown_res = dd_chown_meta_data(dd, owners_uid, groups_gid); ++ ++ if (chown_res == 0) ++ { ++ dd->dd_uid = owners_uid; ++ dd->dd_gid = groups_gid; ++ } ++ + return chown_res; + } + +@@ -1175,7 +1550,7 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla + // if (!dd->locked) + // error_msg_and_die("dump_dir is not opened"); /* bug */ + +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + { + error_msg("Cannot load text. '%s' is not a valid file name", name); + if ((flags & DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE)) +@@ -1201,7 +1576,7 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot save text. '%s' is not a valid file name", name); + + save_binary_file_at(dd->dd_fd, name, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode); +@@ -1212,7 +1587,7 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot save binary. '%s' is not a valid file name", name); + + save_binary_file_at(dd->dd_fd, name, data, size, dd->dd_uid, dd->dd_gid, dd->mode); +@@ -1220,7 +1595,7 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns + + long dd_get_item_size(struct dump_dir *dd, const char *name) + { +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot get item size. '%s' is not a valid file name", name); + + long size = -1; +@@ -1245,7 +1620,7 @@ int dd_delete_item(struct dump_dir *dd, const char *name) + if (!dd->locked) + error_msg_and_die("dump_dir is not opened"); /* bug */ + +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot delete item. '%s' is not a valid file name", name); + + int res = unlinkat(dd->dd_fd, name, /*only files*/0); +@@ -1483,7 +1858,7 @@ int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason) + + int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + { +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + + log_debug("copying '%s' to '%s' at '%s'", source_path, name, dd->dd_dirname); +@@ -1502,7 +1877,7 @@ int dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path) + + int dd_copy_file_unpack(struct dump_dir *dd, const char *name, const char *source_path) + { +- if (!str_is_correct_filename(name)) ++ if (!dd_validate_element_name(name)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name); + + log_debug("unpacking '%s' to '%s' at '%s'", source_path, name, dd->dd_dirname); +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index 31c320e..4bf479e 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -158,6 +158,177 @@ int main(int argc, char **argv) + } + ]]) + ++## --------------------- ## ++## dd_create_open_delete ## ++## --------------------- ## ++ ++AT_TESTFUN([dd_create_open_delete], ++[[ ++#include "internal_libreport.h" ++#include ++#include ++ ++int main(int argc, char **argv) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX/dump_dir"; ++ ++ char *last_slash = strrchr(template, '/'); ++ *last_slash = '\0'; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ *last_slash = '/'; ++ ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(strcmp(dd->dd_dirname, template) == 0); ++ assert(dd->dd_fd >= 0); ++ assert(dd->dd_md_fd >= 0); ++ ++ struct stat dd_st; ++ assert(fstat(dd->dd_fd, &dd_st) == 0); ++ ++ struct stat md_st; ++ assert(fstat(dd->dd_md_fd, &md_st) == 0); ++ ++ assert(dd_st.st_uid == md_st.st_uid); ++ assert(dd_st.st_gid == md_st.st_gid); ++ assert((dd_st.st_mode & 0666) == (md_st.st_mode & 0666)); ++ ++ struct stat path_md_st; ++ assert(fstatat(dd->dd_fd, ".libreport", &path_md_st, 0) == 0); ++ assert(md_st.st_ino = path_md_st.st_ino); ++ ++ dd_create_basic_files(dd, (uid_t)-1, NULL); ++ dd_save_text(dd, FILENAME_TYPE, "attest"); ++ ++ dd_close(dd); ++ dd = NULL; ++ ++ dd = dd_opendir(template, 0); ++ assert(dd != NULL); ++ assert(strcmp(dd->dd_dirname, template) == 0); ++ assert(dd->dd_fd >= 0); ++ assert(dd->dd_md_fd < 0); ++ ++ dd_delete(dd); ++ ++ assert(stat(template, &dd_st) != 0); ++ ++ *last_slash = '\0'; ++ assert(rmdir(template) == 0); ++ return EXIT_SUCCESS; ++} ++]]) ++ ++## -------------------------- ## ++## dd_sanitize_mode_and_owner ## ++## -------------------------- ## ++ ++AT_TESTFUN([dd_sanitize_mode_and_owner], ++[[ ++#include "internal_libreport.h" ++#include ++#include ++ ++int main(int argc, char **argv) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX/dump_dir"; ++ ++ char *last_slash = strrchr(template, '/'); ++ *last_slash = '\0'; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ *last_slash = '/'; ++ ++ /* Prepare a directory for chmod test, use mode 0600 and chmod it to 0640 */ ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0600); ++ assert(dd != NULL); ++ ++ { ++ struct stat path_md_st; ++ assert((fstatat(dd->dd_fd, ".libreport", &path_md_st, 0) == 0) || !"Failed initialize meta-data"); ++ assert((path_md_st.st_mode & 0077) == 0); ++ } ++ ++ dd_create_basic_files(dd, (uid_t)-1, NULL); ++ dd_save_text(dd, FILENAME_TYPE, "attest"); ++ ++ dd_close(dd); ++ ++ /* initialize meta-data */ ++ dd = dd_opendir(template, DD_OPEN_FD_ONLY); ++ /* reopen for writing */ ++ dd = dd_fdopendir(dd, /*for writing*/0); ++ assert(dd != NULL); ++ ++ assert(fchmod(dd->dd_fd, 0750) == 0); ++ dd->mode = 0640; ++ ++ fprintf(stderr, "Going to sanitize\n"); ++ dd_sanitize_mode_and_owner(dd); ++ fprintf(stderr, "Sanitized\n"); ++ ++ { ++ DIR *d = opendir(template); ++ struct dirent *dent; ++ while ((dent = readdir(d)) != NULL) ++ { ++ if ( strcmp(".", dent->d_name) == 0 ++ || strcmp("..", dent->d_name) == 0 ++ || strcmp(".lock", dent->d_name) == 0 ++ || strcmp(".libreport", dent->d_name) == 0 ++ ) ++ continue; ++ ++ struct stat sb; ++ printf("Testing element: %s\n", dent->d_name); ++ assert(fstatat(dd->dd_fd, dent->d_name, &sb, 0) == 0 || !"Cannot stat a regular element"); ++ assert((sb.st_mode & 0777) == 0640 || !"Failed to chmod a regular element"); ++ } ++ closedir(d); ++ } ++ ++ { ++ struct stat path_md_st; ++ assert(fstatat(dd->dd_fd, ".libreport", &path_md_st, 0) == 0 || !"Cannot stat meta-data directory"); ++ assert((path_md_st.st_mode & 0777) == 0750 || !"Failed chmod meta-data"); ++ } ++ ++ int md_dir_fd = openat(dd->dd_fd, ".libreport", O_DIRECTORY); ++ assert(md_dir_fd >= 0 || !"Cannot open meta-data directory"); ++ DIR *d = fdopendir(md_dir_fd); ++ struct dirent *dent; ++ while ((dent = readdir(d)) != NULL) ++ { ++ if (strcmp(".", dent->d_name) == 0 || strcmp("..", dent->d_name) == 0) ++ continue; ++ ++ struct stat sb; ++ printf("Testing meta-data: %s\n", dent->d_name); ++ assert(fstatat(md_dir_fd, dent->d_name, &sb, 0) == 0 || !"Cannot stat meta-data file"); ++ assert((sb.st_mode & 0777) == 0640 || !"Failed to chmod a meta-data file"); ++ } ++ ++ closedir(d); ++ dd_delete(dd); ++ ++ *last_slash = '\0'; ++ assert(rmdir(template) == 0); ++ return EXIT_SUCCESS; ++} ++]]) ++ + ## -------------- ## + ## recursive_lock ## + ## -------------- ## +-- +2.1.0 + diff --git a/0080-dd-add-a-function-parsing-number-from-fd.patch b/0080-dd-add-a-function-parsing-number-from-fd.patch new file mode 100644 index 0000000..c8dd8d1 --- /dev/null +++ b/0080-dd-add-a-function-parsing-number-from-fd.patch @@ -0,0 +1,197 @@ +From 7bdff7f55c801d09834f0487b377c8408fd162b9 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 29 May 2015 15:08:56 +0200 +Subject: [PATCH] dd: add a function parsing number from fd + +We often need to parse a number from a plain text file. + +Related: #354 + +Signed-off-by: Jakub Filak +--- + src/lib/dump_dir.c | 120 ++++++++++++++++++++++++++++++++--------------------- + 1 file changed, 72 insertions(+), 48 deletions(-) + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index aed2de0..9ba2352 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -151,15 +151,14 @@ int secure_openat_read(int dir_fd, const char *filename) + if (strchr(filename, '/')) + { + error_msg("Path must be file name without directory: '%s'", filename); +- errno = EFAULT; +- return -1; ++ return -EFAULT; + } + + static char reopen_buf[sizeof("/proc/self/fd/") + 3*sizeof(int) + 1]; + + int path_fd = openat(dir_fd, filename, O_PATH | O_NOFOLLOW); + if (path_fd < 0) +- return -1; ++ return -errno; + + struct stat path_sb; + int r = fstat(path_fd, &path_sb); +@@ -167,15 +166,14 @@ int secure_openat_read(int dir_fd, const char *filename) + { + perror_msg("stat"); + close(path_fd); +- return -1; ++ return -EINVAL; + } + + if (!S_ISREG(path_sb.st_mode) || path_sb.st_nlink > 1) + { + log_notice("Path isn't a regular file or has more links (%lu)", (unsigned long)path_sb.st_nlink); +- errno = EINVAL; + close(path_fd); +- return -1; ++ return -EINVAL; + } + + if (snprintf(reopen_buf, sizeof(reopen_buf), "/proc/self/fd/%d", path_fd) >= sizeof(reopen_buf)) { +@@ -189,71 +187,97 @@ int secure_openat_read(int dir_fd, const char *filename) + return fd; + } + +-/* Returns value less than 0 if the file is not readable or +- * if the file doesn't contain valid unixt time stamp. +- * +- * Any possible failure will be logged. +- */ +-static time_t parse_time_file_at(int dir_fd, const char *filename) ++static int read_number_from_file_at(int dir_fd, const char *filename, const char *typename, ++ size_t typesz, long long max, long long min, long long *value) + { +- /* Open input file, and parse it. */ +- int fd = secure_openat_read(dir_fd, filename); ++ const int fd = secure_openat_read(dir_fd, filename); + if (fd < 0) + { +- VERB2 pwarn_msg("Can't open '%s'", filename); +- return -1; ++ log_warning("Can't open '%s'", filename); ++ return fd; + } + +- /* ~ maximal number of digits for positive time stamp string */ +- char time_buf[sizeof(time_t) * 3 + 1]; +- ssize_t rdsz = read(fd, time_buf, sizeof(time_buf)); +- ++ int ret = 0; ++ /* - xmalloc_read() does not count '\0' Byte ++ * - count on sign ++ * - count on '\n' ++ */ ++ const size_t max_size = typesz * 3 + 2; ++ /* allow to read one extra Byte to be able to identify longer text */ ++ size_t total_read = max_size + 1; ++ char *const value_buf = xmalloc_read(fd, &total_read); + /* Just reading, so no need to check the returned value. */ +- close(fd); + +- if (rdsz == -1) ++ if (value_buf == NULL) + { +- VERB2 pwarn_msg("Can't read from '%s'", filename); +- return -1; ++ log_warning("Can't read from '%s'", filename); ++ ret = -EBADFD; ++ goto finito; + } +- /* approximate maximal number of digits in timestamp is sizeof(time_t)*3 */ +- /* buffer has this size + 1 byte for trailing '\0' */ +- /* if whole size of buffer was read then file is bigger */ +- /* than string representing maximal time stamp */ +- if (rdsz == sizeof(time_buf)) ++ ++ if (total_read >= max_size) + { +- VERB2 warn_msg("File '%s' is too long to be valid unix " +- "time stamp (max size %u)", filename, (int)sizeof(time_buf)); +- return -1; ++ log_warning("File '%s' is too long to be valid %s " ++ "(max size %u)", filename, typename, (int)sizeof(value_buf)); ++ ret = -EMSGSIZE; ++ goto finito; + } + +- /* Our tools don't put trailing newline into time file, ++ /* Our tools don't put trailing newline into one line files, + * but we allow such format too: + */ +- if (rdsz > 0 && time_buf[rdsz - 1] == '\n') +- rdsz--; +- time_buf[rdsz] = '\0'; +- +- /* Note that on some architectures (x32) time_t is "long long" */ ++ if (value_buf[total_read - 1] == '\n') ++ --total_read; ++ value_buf[total_read] = '\0'; + + errno = 0; /* To distinguish success/failure after call */ + char *endptr; +- long long val = strtoll(time_buf, &endptr, /* base */ 10); +- const long long MAX_TIME_T = (1ULL << (sizeof(time_t)*8 - 1)) - 1; ++ long long res = strtoll(value_buf, &endptr, /* base */ 10); + + /* Check for various possible errors */ +- if (errno +- || (*endptr != '\0') +- || val >= MAX_TIME_T +- || !isdigit_str(time_buf) /* this filters out "-num", " num", "" */ ++ if ( (errno == ERANGE && (res == LONG_LONG_MAX || res == LONG_LONG_MIN)) ++ || (errno != 0 && res == 0) ++ || (*endptr != '\0') ++ || endptr == value_buf + ) { +- VERB2 pwarn_msg("File '%s' doesn't contain valid unix " +- "time stamp ('%s')", filename, time_buf); +- return -1; ++ log_warning("File '%s' doesn't contain valid %s" ++ "('%s')", filename, typename, value_buf); ++ ret = -EINVAL; ++ goto finito; + } + ++ /* If the stored number is long long, this condition falsely indetify ++ * LONG_LONG_(MAX|MIN) as an value which is out of range. ++ */ ++ if (res <= min || res >= max) ++ { ++ log_warning("File '%s' contains a number out-of-range of %s" ++ "('%s')", filename, typename, value_buf); ++ ret = -ERANGE; ++ goto finito; ++ } ++ ++ *value = res; ++ ++finito: ++ close(fd); ++ free(value_buf); + /* If we got here, strtoll() successfully parsed a number */ +- return val; ++ return ret; ++} ++ ++/* Returns value less than 0 if the file is not readable or ++ * if the file doesn't contain valid unixt time stamp. ++ * ++ * Any possible failure will be logged. ++ */ ++static time_t parse_time_file_at(int dir_fd, const char *filename) ++{ ++ /* Note that on some architectures (x32) time_t is "long long" */ ++ const long long MAX_TIME_T = (1ULL << (sizeof(time_t)*8 - 1)) - 1; ++ long long value = -1; ++ read_number_from_file_at(dir_fd, filename, "unix time stamp", sizeof(time_t), MAX_TIME_T, 0, &value); ++ return (time_t)value; + } + + /* Return values: +-- +2.1.0 + diff --git a/0081-dd-add-owner-to-meta-data.patch b/0081-dd-add-owner-to-meta-data.patch new file mode 100644 index 0000000..2cecbaa --- /dev/null +++ b/0081-dd-add-owner-to-meta-data.patch @@ -0,0 +1,465 @@ +From ad27acbb2104d4172a33dd82ef9d94c8f5dba617 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 28 May 2015 11:47:39 +0200 +Subject: [PATCH] dd: add owner to meta-data + +Use 'nobody' user to mark dump directories which have no owner and are +publicly readable and chownable. The 'nobody' should not be used +directly but through the function set_no_owner() which is a substitution +for adding S_IROTH (man 2 open) to dump dir mode. + +Use meta-data to get the owner only if the dump directory is owned by +super-user, because it is the only state when we can trust that value. + +Related: #354 + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 21 ++++++ + src/lib/dump_dir.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++- + tests/dump_dir.at | 110 ++++++++++++++++++++++++++++ + 3 files changed, 322 insertions(+), 4 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index f33eb99..b37b262 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -141,6 +141,26 @@ int dd_rename(struct dump_dir *dd, const char *new_path); + */ + int dd_chown(struct dump_dir *dd, uid_t new_uid); + ++/* Sets a new owner (does NOT chown the directory) ++ * ++ * Does not validate the passed uid. ++ * The given dump_dir must be opened for writing. ++ */ ++int dd_set_owner(struct dump_dir *dd, uid_t owner); ++ ++/* Makes the dump directory owned by nobody. ++ * ++ * The directory will be accessible for all users. ++ * The given dump_dir must be opened for writing. ++ */ ++int dd_set_no_owner(struct dump_dir *dd); ++ ++/* Gets the owner ++ * ++ * If meta-data misses owner, returns fs owner. ++ * Can be used with DD_OPEN_FD_ONLY. ++ */ ++uid_t dd_get_owner(struct dump_dir *dd); + + /* reported_to handling */ + #define add_reported_to_data libreport_add_reported_to_data +@@ -187,6 +207,7 @@ int dd_accessible_by_uid(struct dump_dir *dd, uid_t uid); + enum { + DD_STAT_ACCESSIBLE_BY_UID = 1, + DD_STAT_OWNED_BY_UID = DD_STAT_ACCESSIBLE_BY_UID << 1, ++ DD_STAT_NO_OWNER = DD_STAT_OWNED_BY_UID << 1, + }; + + /* Gets information about a dump directory for particular uid. +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 9ba2352..2cd14bb 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -88,6 +88,7 @@ + // parent dump directory. It is not a fatal error, if the meta-data directory + // does not exist (backward compatibility). + #define META_DATA_DIR_NAME ".libreport" ++#define META_DATA_FILE_OWNER "owner" + + enum { + /* Try to create meta-data dir if it does not exist */ +@@ -106,6 +107,8 @@ static char *load_text_file(const char *path, unsigned flags); + static char *load_text_file_at(int dir_fd, const char *name, unsigned flags); + static void copy_file_from_chroot(struct dump_dir* dd, const char *name, + const char *chroot_dir, const char *file_path); ++static bool save_binary_file_at(int dir_fd, const char *name, const char* data, ++ unsigned size, uid_t uid, gid_t gid, mode_t mode); + + static bool isdigit_str(const char *str) + { +@@ -136,6 +139,22 @@ static bool exist_file_dir_at(int dir_fd, const char *name) + #define dd_validate_element_name(name) \ + (str_is_correct_filename(name) && (strcmp(META_DATA_DIR_NAME, name) != 0)) + ++/* nobody user should not own any file */ ++static int get_no_owner_uid(uid_t *uid) ++{ ++ struct passwd *pw = getpwnam("nobody"); ++ if (pw == NULL) ++ { ++ perror_msg("can't get nobody's uid"); ++ if (errno == 0) ++ return -ENOENT; ++ return -errno; ++ } ++ ++ *uid = pw->pw_uid; ++ return 0; ++} ++ + /* Opens the file in the three following steps: + * 1. open the file with O_PATH (get a file descriptor for operations with + * inode) and O_NOFOLLOW (do not dereference symbolick links) +@@ -611,6 +630,113 @@ static int dd_get_meta_data_dir_fd(struct dump_dir *dd, int flags) + return dd->dd_md_fd; + } + ++/* Tries to safely overwrite the existing file. ++ * ++ * The functions writes the new value to a temporary file and if the temporary ++ * file is successfully created, then moves the tmp file to the old file name. ++ * ++ * If the meta-data directory does not exist, the function will try to create ++ * it. ++ */ ++static int dd_meta_data_save_text(struct dump_dir *dd, const char *name, const char *data) ++{ ++ if (!dd->locked) ++ error_msg_and_die("dump_dir is not opened"); /* bug */ ++ ++ if (!str_is_correct_filename(name)) ++ error_msg_and_die("Cannot save meta-data. '%s' is not a valid file name", name); ++ ++ int dd_md_fd = dd_get_meta_data_dir_fd(dd, DD_MD_GET_CREATE); ++ if (dd_md_fd < 0) ++ { ++ error_msg("Can't save meta-data: '%s'", name); ++ return dd_md_fd; ++ } ++ ++ char *tmp_name = xasprintf("~%s.tmp", name); ++ ++ int ret = -1; ++ if (!save_binary_file_at(dd_md_fd, tmp_name, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode)) ++ goto finito; ++ ++ /* man 2 rename ++ * ++ * If newpath already exists it will be atomically replaced (subject to a ++ * few conditions; see ERRORS below), so that there is no point at which ++ * another process attempting to access newpath will find it missing. ++ */ ++ if (renameat(dd_md_fd, tmp_name, dd_md_fd, name)) ++ { ++ ret = -errno; ++ perror_msg("Failed to move temporary file '%s' to '%s'", tmp_name, name); ++ goto finito; ++ } ++ ++ ret = 0; ++ ++finito: ++ free(tmp_name); ++ return ret; ++} ++ ++int dd_set_owner(struct dump_dir *dd, uid_t owner) ++{ ++ /* I was tempted to use the keyword static, but we should have reentracy ++ * always on mind. Who knows! */ ++ char long_str[sizeof(long) * 3 + 2]; ++ ++ if (owner == (uid_t)-1) ++ { ++ owner = getuid(); ++ if (owner < 0) ++ perror_msg_and_die("%s: getuid", __func__); ++ } ++ ++ snprintf(long_str, sizeof(long_str), "%li", (long)owner); ++ const int ret = dd_meta_data_save_text(dd, META_DATA_FILE_OWNER, long_str); ++ if (ret < 0) ++ error_msg("The dump dir owner wasn't set to '%s'", long_str); ++ return ret; ++} ++ ++int dd_set_no_owner(struct dump_dir *dd) ++{ ++ uid_t no_owner_uid = (uid_t)-1; ++ int ret = get_no_owner_uid(&no_owner_uid); ++ if (ret < 0) ++ return ret; ++ ++ return dd_set_owner(dd, no_owner_uid); ++} ++ ++uid_t dd_get_owner(struct dump_dir *dd) ++{ ++ static const long long MAX_UID_T = (1ULL << (sizeof(uid_t)*8 - 1)) - 1; ++ ++ int dd_md_fd = dd_get_meta_data_dir_fd(dd, /*no create*/0); ++ if (dd_md_fd < 0) ++ { ++ log_info("No meta-data, using fs owner."); ++ return dd->dd_uid; ++ } ++ ++ long long owner = -1; ++ ++ int ret = read_number_from_file_at(dd_md_fd, META_DATA_FILE_OWNER, "UID", ++ sizeof(uid_t), MAX_UID_T, -1, &owner); ++ ++ if (ret < 0) ++ { ++ if (ret != -ENOENT) ++ return ret; ++ ++ log_info("No meta-data 'owner', using fs owner."); ++ return dd->dd_uid; ++ } ++ ++ return (uid_t)owner; ++} ++ + /* A helper function useful for traversing directories. + * + * DIR* d opendir(dir_fd); ... closedir(d); closes also dir_fd but we want to +@@ -1026,6 +1152,12 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + goto fail; + } + ++ if (dd_set_owner(dd, dd->dd_uid) < 0) ++ { ++ log_debug("Failed to initialized 'owner'"); ++ goto fail; ++ } ++ + if (uid != (uid_t)-1L) + { + dd->dd_uid = 0; +@@ -1087,6 +1219,11 @@ int dd_reset_ownership(struct dump_dir *dd) + if (dd_chown_meta_data(dd, dd->dd_uid, dd->dd_gid) != 0) + error_msg("Failed to reset ownership of meta-data"); + ++ /* We ignore failures above, so we will ignore failures here too. ++ * The meta-data owner already exist (created by dd_create_skeleton). ++ */ ++ dd_set_owner(dd, dd->dd_uid); ++ + return r; + } + +@@ -1122,8 +1259,14 @@ void dd_create_basic_files(struct dump_dir *dd, uid_t uid, const char *chroot_di + free(time_str); + + /* it doesn't make sense to create the uid file if uid == -1 */ ++ /* and 'owner' is set since dd_create_skeleton */ + if (uid != (uid_t)-1L) + { ++ /* Failure is not a problem here, because we still have the fs ++ * attributes and there is only a little chance that the old value ++ * gets lost. */ ++ dd_set_owner(dd, uid); ++ + snprintf(long_str, sizeof(long_str), "%li", (long)uid); + dd_save_text(dd, FILENAME_UID, long_str); + } +@@ -1436,6 +1579,9 @@ next: + dd->dd_gid = groups_gid; + } + ++ if (chown_res == 0) ++ chown_res = dd_set_owner(dd, (long)dd->dd_uid); ++ + return chown_res; + } + +@@ -1812,7 +1958,45 @@ static bool uid_in_group(uid_t uid, gid_t gid) + int dd_stat_for_uid(struct dump_dir *dd, uid_t uid) + { + int ddstat = 0; +- if (uid == 0 || (dd->mode & S_IROTH)) ++ ++ if (uid == 0) ++ { ++ log_debug("directory accessible by super-user"); ++ ddstat |= DD_STAT_ACCESSIBLE_BY_UID; ++ } ++ ++#define DD_OWNER_FLAGS (DD_STAT_ACCESSIBLE_BY_UID | DD_STAT_OWNED_BY_UID) ++ if (dd->dd_uid == 0) ++ { ++ log_debug("directory owned by super-user: checking meta-data"); ++ ++ const uid_t owner = dd_get_owner(dd); ++ ++ if (owner < 0) ++ goto fsattributes; ++ ++ if (owner == uid) ++ { ++ log_debug("meta-data: %ld uid owns directory", (long)uid); ++ ddstat |= DD_OWNER_FLAGS; ++ goto finito; ++ } ++ ++ uid_t no_owner_uid = (uid_t)-1; ++ int ret = get_no_owner_uid(&no_owner_uid); ++ if ( ret >= 0 ++ && owner == no_owner_uid) ++ { ++ log_debug("meta-data: directory is accessible by %ld uid", (long)uid); ++ ddstat |= DD_STAT_ACCESSIBLE_BY_UID; ++ ddstat |= DD_STAT_NO_OWNER; ++ } ++ ++ goto finito; ++ } ++ ++fsattributes: ++ if (dd->mode & S_IROTH) + { + log_debug("directory is accessible by %ld uid", (long)uid); + ddstat |= DD_STAT_ACCESSIBLE_BY_UID; +@@ -1824,11 +2008,14 @@ int dd_stat_for_uid(struct dump_dir *dd, uid_t uid) + if (uid_in_group(uid, dd->dd_gid)) + #endif + { +- log_debug("%ld uid owns directory", (long)uid); +- ddstat |= DD_STAT_ACCESSIBLE_BY_UID; +- ddstat |= DD_STAT_OWNED_BY_UID; ++ log_debug("fs attributes: %ld uid owns directory", (long)uid); ++ ddstat |= DD_OWNER_FLAGS; + } + ++#undef DD_OWNER_FLAGS ++ ++finito: ++ log_debug("UID=%d, %s: %o", uid, dd->dd_dirname, ddstat); + return ddstat; + } + +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index 4bf479e..cc0148c 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -203,6 +203,11 @@ int main(int argc, char **argv) + assert(fstatat(dd->dd_fd, ".libreport", &path_md_st, 0) == 0); + assert(md_st.st_ino = path_md_st.st_ino); + ++ struct stat owner_md_st; ++ assert(fstatat(dd->dd_md_fd, "owner", &owner_md_st, 0) == 0); ++ assert((dd_st.st_mode & 0666) == (owner_md_st.st_mode & 0666)); ++ assert(geteuid() == dd_get_owner(dd)); ++ + dd_create_basic_files(dd, (uid_t)-1, NULL); + dd_save_text(dd, FILENAME_TYPE, "attest"); + +@@ -329,6 +334,111 @@ int main(int argc, char **argv) + } + ]]) + ++## -------- ## ++## dd_owner ## ++## -------- ## ++ ++AT_TESTFUN([dd_owner], ++[[ ++#include "internal_libreport.h" ++#include ++#include ++ ++int main(int argc, char **argv) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX/dump_dir"; ++ ++ char *last_slash = strrchr(template, '/'); ++ *last_slash = '\0'; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ *last_slash = '/'; ++ ++ printf("Dump dir path: %s\n", template); ++ ++ { ++ fprintf(stderr, "TEST === default owner\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ assert(geteuid() == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ ++ { ++ fprintf(stderr, "TEST === create basic files w/o UID\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ dd_create_basic_files(dd, (uid_t)-1, NULL); ++ assert(geteuid() == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ ++ { ++ fprintf(stderr, "TEST === create basic files with UID\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ dd_create_basic_files(dd, (geteuid() + 1), NULL); ++ assert((geteuid() + 1) == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ ++ { ++ fprintf(stderr, "TEST === set no owner\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ dd_set_no_owner(dd); ++ ++ struct passwd *nobody_pw= getpwnam("nobody"); ++ assert(nobody_pw != NULL); ++ assert(nobody_pw->pw_uid == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ ++ { ++ fprintf(stderr, "TEST === set artibrary owner\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ dd_set_owner(dd, (geteuid() + 2)); ++ assert((geteuid() + 2) == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ ++ { ++ fprintf(stderr, "TEST === chown no owner\n"); ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL); ++ ++ dd_set_no_owner(dd); ++ assert(geteuid() != dd_get_owner(dd)); ++ ++ dd_chown(dd, geteuid()); ++ assert(geteuid() == dd_get_owner(dd)); ++ ++ assert(dd_delete(dd) == 0); ++ } ++ *last_slash = '\0'; ++ assert(rmdir(template) == 0); ++ ++ return EXIT_SUCCESS; ++} ++]]) ++ + ## -------------- ## + ## recursive_lock ## + ## -------------- ## +-- +2.1.0 + diff --git a/0082-lib-add-utility-functions-parsing-numbers.patch b/0082-lib-add-utility-functions-parsing-numbers.patch new file mode 100644 index 0000000..dc23e98 --- /dev/null +++ b/0082-lib-add-utility-functions-parsing-numbers.patch @@ -0,0 +1,287 @@ +From e82a1473dac1c7068f5d30f34bc07360ab70881d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 16:53:18 +0200 +Subject: [PATCH] lib: add utility functions parsing numbers + +The x* variants dies on errors but we do not want to do that in some +cases. + +Signed-off-by: Jakub Filak +--- + src/include/internal_libreport.h | 6 +++ + src/lib/xatonum.c | 95 ++++++++++++++++++++++++++------- + tests/xfuncs.at | 110 +++++++++++++++++++++++++++++++++++++++ + 3 files changed, 192 insertions(+), 19 deletions(-) + +diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h +index 25af681..4f8ea26 100644 +--- a/src/include/internal_libreport.h ++++ b/src/include/internal_libreport.h +@@ -241,8 +241,12 @@ const uint8_t *str_to_sha1(uint8_t result[SHA1_RESULT_LEN], const char *str); + const char *str_to_sha1str(char result[SHA1_RESULT_LEN*2 + 1], const char *str); + + ++#define try_atou libreport_try_atou ++int try_atou(const char *numstr, unsigned *value); + #define xatou libreport_xatou + unsigned xatou(const char *numstr); ++#define try_atoi libreport_try_atoi ++int try_atoi(const char *numstr, int *value); + #define xatoi libreport_xatoi + int xatoi(const char *numstr); + /* Using xatoi() instead of naive atoi() is not always convenient - +@@ -252,6 +256,8 @@ int xatoi(const char *numstr); + * It should really be named xatoi_nonnegative (since it allows 0), + * but that would be too long. + */ ++#define try_atoi_positive libreport_try_atoi_positive ++int try_atoi_positive(const char *numstr, int *value); + #define xatoi_positive libreport_xatoi_positive + int xatoi_positive(const char *numstr); + +diff --git a/src/lib/xatonum.c b/src/lib/xatonum.c +index 71b0247..510d766 100644 +--- a/src/lib/xatonum.c ++++ b/src/lib/xatonum.c +@@ -22,44 +22,101 @@ + */ + #include "internal_libreport.h" + +-unsigned xatou(const char *numstr) ++int try_atou(const char *numstr, unsigned *value) + { ++ int ret = 0; + unsigned long r; + int old_errno; + char *e; + ++ old_errno = errno; + if (*numstr < '0' || *numstr > '9') +- goto inval; ++ { ++ ret = -EINVAL; ++ goto finito; ++ } + +- old_errno = errno; + errno = 0; + r = strtoul(numstr, &e, 10); +- if (errno || numstr == e || *e != '\0' || r > UINT_MAX) +- goto inval; /* error / no digits / illegal trailing chars */ ++ if (errno || numstr == e || *e != '\0') ++ { ++ ret = errno != 0 ? -errno : -EINVAL; /* error / no digits / illegal trailing chars */ ++ goto finito; ++ } ++ ++ /* check range */ ++ if (r > UINT_MAX) ++ { ++ /* In this case errno is probably 0, because UINT_MAX < ULONG_MAX, thus ++ * strtoul should not return an error */ ++ ret = -ERANGE; ++ goto finito; ++ } ++ ++ *value = r; ++ ++finito: + errno = old_errno; /* Ok. So restore errno. */ +- return r; ++ return ret; ++} ++ ++unsigned xatou(const char *numstr) ++{ ++ unsigned value = (unsigned)-1; ++ ++ if (try_atou(numstr, &value) != 0) ++ error_msg_and_die("expected number in range <0, %d>: '%s'", UINT_MAX, numstr); ++ ++ return value; ++} + +-inval: +- error_msg_and_die("invalid number '%s'", numstr); ++int try_atoi_positive(const char *numstr, int *value) ++{ ++ unsigned tmp; ++ int r = try_atou(numstr, &tmp); ++ if (r != 0) ++ return r; ++ ++ if (tmp > (unsigned)INT_MAX) ++ return -ERANGE; ++ ++ *value = (int)tmp; ++ return 0; + } + + int xatoi_positive(const char *numstr) + { +- unsigned r = xatou(numstr); +- if (r > (unsigned)INT_MAX) +- error_msg_and_die("invalid number '%s'", numstr); +- return r; ++ int value = INT_MIN; ++ ++ if (try_atoi_positive(numstr, &value) != 0) ++ error_msg_and_die("expected number in range <0, %d>: '%s'", INT_MAX, numstr); ++ ++ return value; ++} ++ ++int try_atoi(const char *numstr, int *value) ++{ ++ if (*numstr != '-') ++ return try_atoi_positive(numstr, value); ++ ++ unsigned tmp; ++ int r = try_atou(numstr + 1, &tmp); ++ if (r < 0) ++ return r; ++ ++ if (tmp > (unsigned)INT_MAX + 1) ++ return -ERANGE; ++ ++ *value = - (int)tmp; ++ return 0; + } + + int xatoi(const char *numstr) + { +- unsigned r; ++ int value = INT_MIN; + +- if (*numstr != '-') +- return xatoi_positive(numstr); ++ if (try_atoi(numstr, &value)) ++ error_msg_and_die("expected number in range <%d, %d>: '%s'", INT_MIN, INT_MAX, numstr); + +- r = xatou(numstr + 1); +- if (r > (unsigned)INT_MAX + 1) +- error_msg_and_die("invalid number '%s'", numstr); +- return - (int)r; ++ return (int)value; + } +diff --git a/tests/xfuncs.at b/tests/xfuncs.at +index d7e947a..dbcc602 100644 +--- a/tests/xfuncs.at ++++ b/tests/xfuncs.at +@@ -54,3 +54,113 @@ int main(void) + } + + ]]) ++ ++## ------------- ## ++## parse_numbers ## ++## ------------- ## ++ ++AT_TESTFUN([parse_numbers], ++[[#include "internal_libreport.h" ++#include ++#include ++#include ++ ++int main(void) ++{ ++ g_verbose=3; ++ ++ /* 1. not a number - ERROR ++ * 2. a number with an alpha suffix - ERROR ++ * 3. out of range number - ERROR ++ * a. max + 1 - ERROR ++ * b. min - 1 - ERROR ++ * c. max - OK ++ * d. min - OK ++ */ ++ ++ { ++ unsigned uint_value = 12345; ++ assert(try_atou("foo", &uint_value) != 0); ++ assert(uint_value == 12345); ++ ++ assert(try_atou("foo54321", &uint_value) != 0); ++ assert(uint_value == 12345); ++ ++ char buf[sizeof(unsigned long) * 3 + 1]; ++ ++ snprintf(buf, sizeof(buf), "%lu", 1LU + UINT_MAX); ++ assert(try_atou(buf, &uint_value) != 0 || !"Above UINT_MAX"); ++ assert(uint_value == 12345); ++ ++ assert(try_atou("-1", &uint_value) != 0); ++ assert(uint_value == 12345); ++ ++ snprintf(buf, sizeof(buf), "%lu", (long unsigned)UINT_MAX); ++ assert(try_atou(buf, &uint_value) == 0); ++ assert(uint_value == UINT_MAX); ++ assert(xatou(buf) == UINT_MAX); ++ ++ assert(try_atou("0", &uint_value) == 0); ++ assert(uint_value == 0); ++ assert(xatou("0") == 0); ++ } ++ ++ { ++ int int_value = 12345; ++ assert(try_atoi("foo", &int_value) != 0); ++ assert(int_value == 12345); ++ ++ assert(try_atoi("foo54321", &int_value) != 0); ++ assert(int_value == 12345); ++ ++ char buf[sizeof(long) * 3 + 1]; ++ ++ snprintf(buf, sizeof(buf), "%ld", 1L + INT_MAX); ++ assert(try_atoi(buf, &int_value) != 0 || !"Parse INT_MAX+1"); ++ assert(int_value == 12345 || !"Above INT_MAX"); ++ ++ snprintf(buf, sizeof(buf), "%ld", -1L + INT_MIN); ++ assert(try_atoi(buf, &int_value) != 0 || !"Parse INT_MIN-1"); ++ assert(int_value == 12345 || !"Belove INT_MIN"); ++ ++ snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MAX); ++ assert(try_atoi(buf, &int_value) == 0 || !"Parse INT_MAX"); ++ assert(int_value == INT_MAX); ++ assert(xatoi(buf) == INT_MAX); ++ ++ snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MIN); ++ assert(try_atoi(buf, &int_value) == 0 || !"Parse INT_MIN"); ++ assert(int_value == INT_MIN); ++ assert(xatoi(buf) == INT_MIN); ++ } ++ ++ { ++ int positive_value = 12345; ++ assert(try_atoi_positive("foo", &positive_value) != 0); ++ assert(positive_value == 12345); ++ ++ assert(try_atoi_positive("foo54321", &positive_value) != 0); ++ assert(positive_value == 12345); ++ ++ char buf[sizeof(long) * 3 + 1]; ++ ++ snprintf(buf, sizeof(buf), "%ld", 1L + INT_MAX); ++ assert(try_atoi_positive(buf, &positive_value) != 0); ++ assert(positive_value == 12345 || !"Above INT_MAX"); ++ ++ assert(try_atoi_positive("-1", &positive_value) != 0); ++ assert(positive_value == 12345 || !"After -1"); ++ ++ snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MAX); ++ assert(try_atoi_positive(buf, &positive_value) == 0 || !"Parse INT_MAX"); ++ assert(positive_value == INT_MAX); ++ assert(xatoi_positive(buf) == INT_MAX); ++ ++ assert(try_atoi_positive("0", &positive_value) == 0); ++ assert(positive_value == 0); ++ assert(xatoi_positive("0") == 0); ++ } ++ ++ return 0; ++} ++]]) +-- +2.1.0 + diff --git a/0083-dd-make-super-user-UID-and-FS-group-configurable.patch b/0083-dd-make-super-user-UID-and-FS-group-configurable.patch new file mode 100644 index 0000000..16845ce --- /dev/null +++ b/0083-dd-make-super-user-UID-and-FS-group-configurable.patch @@ -0,0 +1,117 @@ +From 39bcae063c959687458acdd9304732612bedf097 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 18:35:43 +0200 +Subject: [PATCH] dd: make super-user UID and FS group configurable + +The main goal of this patch is to enable running the unit tests for +non-root users, because they cannot add a new user nor create files +owned by 0 or having group 'abrt'. + +The GID variable might be used by other projects than ABRT. + +The super-user variable might be used by ABRT, if security people decide +that abrtd must run under non-root user. + +Signed-off-by: Jakub Filak +--- + src/include/dump_dir.h | 26 ++++++++++++++++++++++++++ + src/lib/dump_dir.c | 25 ++++++++++++++++++------- + 2 files changed, 44 insertions(+), 7 deletions(-) + +diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h +index b37b262..7643d86 100644 +--- a/src/include/dump_dir.h ++++ b/src/include/dump_dir.h +@@ -41,6 +41,32 @@ int create_symlink_lockfile_at(int dir_fd, const char *filename, const char *pid + */ + int secure_openat_read(int dir_fd, const char *filename); + ++/******************************************************************************/ ++/* Global variables */ ++/******************************************************************************/ ++ ++/* UID of super-user (default 0) ++ * ++ * This variable is used by the dd* functions when they access security ++ * sensitive elements. The functions will ONLY TRUST the contents of those ++ * elements that ARE OWNED by super-user. ++ */ ++extern uid_t dd_g_super_user_uid; ++ ++/* GID of a dump diretory created via dd_create() with uid != -1 ++ * ++ * The default value is -1 which means that the dd* functions must ignore this ++ * variable. ++ * ++ * Initialize this variable only if you don't want to use the default group ++ * ('abrt'). ++ */ ++extern gid_t dd_g_fs_group_gid; ++ ++/******************************************************************************/ ++/* Dump Directory */ ++/******************************************************************************/ ++ + enum { + DD_FAIL_QUIETLY_ENOENT = (1 << 0), + DD_FAIL_QUIETLY_EACCES = (1 << 1), +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 2cd14bb..1e3fc6a 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -102,6 +102,12 @@ enum { + // bits + #define DD_MODE_TO_DIR_MODE(mode) ((mode) | (((mode) & 0444) >> 2)) + ++/* Owner of trusted elements */ ++uid_t dd_g_super_user_uid = 0; ++ ++/* Group of new dump directories */ ++gid_t dd_g_fs_group_gid = (gid_t)-1; ++ + + static char *load_text_file(const char *path, unsigned flags); + static char *load_text_file_at(int dir_fd, const char *name, unsigned flags); +@@ -1171,12 +1177,17 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int + else + error_msg("User %lu does not exist, using uid 0", (long)uid); + +- /* Get ABRT's group gid */ +- struct group *gr = getgrnam("abrt"); +- if (gr) +- dd->dd_gid = gr->gr_gid; ++ if (dd_g_fs_group_gid == (uid_t)-1) ++ { ++ /* Get ABRT's group gid */ ++ struct group *gr = getgrnam("abrt"); ++ if (gr) ++ dd->dd_gid = gr->gr_gid; ++ else ++ error_msg("Group 'abrt' does not exist, using gid 0"); ++ } + else +- error_msg("Group 'abrt' does not exist, using gid 0"); ++ dd->dd_gid = dd_g_fs_group_gid; + #else + /* Get ABRT's user uid */ + struct passwd *pw = getpwnam("abrt"); +@@ -1959,14 +1970,14 @@ int dd_stat_for_uid(struct dump_dir *dd, uid_t uid) + { + int ddstat = 0; + +- if (uid == 0) ++ if (uid == dd_g_super_user_uid) + { + log_debug("directory accessible by super-user"); + ddstat |= DD_STAT_ACCESSIBLE_BY_UID; + } + + #define DD_OWNER_FLAGS (DD_STAT_ACCESSIBLE_BY_UID | DD_STAT_OWNED_BY_UID) +- if (dd->dd_uid == 0) ++ if (dd->dd_uid == dd_g_super_user_uid) + { + log_debug("directory owned by super-user: checking meta-data"); + +-- +2.1.0 + diff --git a/0084-dd-set-owner-to-UID-when-creating-dd-from-problem-da.patch b/0084-dd-set-owner-to-UID-when-creating-dd-from-problem-da.patch new file mode 100644 index 0000000..355de51 --- /dev/null +++ b/0084-dd-set-owner-to-UID-when-creating-dd-from-problem-da.patch @@ -0,0 +1,186 @@ +From 9513fab99fa25f665adedf2fac6f5809a4987f2d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 18:41:58 +0200 +Subject: [PATCH] dd: set owner to UID when creating dd from problem data + +That is how the creating of a new dump directory from a problem data was +working before the patches adding the meta-data owner and the patches +dividing the dump directory owner to the FS owner of the dump directory +and the owner of the problem. + +The created dump directory must be accessible to the uid from +FILENAME_UID but must have the requested FS owner. + +Signed-off-by: Jakub Filak +--- + src/lib/create_dump_dir.c | 55 ++++++++++++++++++++++++++++------- + tests/dump_dir.at | 73 +++++++++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 117 insertions(+), 11 deletions(-) + +diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c +index dec75fa..3c9e01d 100644 +--- a/src/lib/create_dump_dir.c ++++ b/src/lib/create_dump_dir.c +@@ -22,6 +22,18 @@ + + #define NEW_PD_SUFFIX ".new" + ++static uid_t parse_uid(const char *uid_str) ++{ ++ assert(sizeof(uid_t) == sizeof(unsigned)); ++ ++ uid_t uid = (uid_t)-1; ++ ++ if (try_atou(uid_str, &uid) != 0) ++ error_msg(_("uid value is not valid: '%s'"), uid_str); ++ ++ return uid; ++} ++ + static struct dump_dir *try_dd_create(const char *base_dir_name, const char *dir_name, uid_t uid) + { + char *path = concat_path_file(base_dir_name, dir_name); +@@ -85,9 +97,38 @@ struct dump_dir *create_dump_dir(const char *base_dir_name, const char *type, ui + * reporting from anaconda where we can't read /etc/{system,redhat}-release + * and os_release is taken from anaconda + */ +- const uid_t crashed_uid = dd_exist(dd, FILENAME_UID) ? /*uid already saved*/-1 : uid; ++ char *uid_str = dd_load_text_ext(dd, FILENAME_UID, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); ++ const uid_t crashed_uid = uid_str != NULL ? /*uid already saved*/-1 : uid; + dd_create_basic_files(dd, crashed_uid, NULL); + ++ /* If crashed uid is (uid_t)-1, then dd_create_basic_files() didn't set the ++ * dd owner and the dd owner remained on fs owner (the default owner used ++ * when creating a new dump directory). ++ * ++ * Our callers expect, that the dd owner is set to value of UID (it used to ++ * be the case before the dd owner was introduced), so we have to try to ++ * get UID from the dump directory, parse it and use the parse value. ++ * Errors are not critical, because the dump directory is already owned by ++ * the fs owner. ++ */ ++ if (crashed_uid == (uid_t)-1 && uid_str != NULL) ++ { ++ uid_t owner_uid = parse_uid(uid_str); ++ if (owner_uid != (uid_t)-1) ++ { ++ log_notice("Changing owner of the new problem to: %s", uid_str); ++ /* Ignore errors, the old value is preseverd or fs uid will be used ++ * instead. The function prints out good error messges.*/ ++ dd_set_owner(dd, owner_uid); ++ } ++ else ++ log_notice("Failed to parse UID, keeping the default owner."); ++ } ++ else ++ log_notice("No UID provided, keeping the default owner."); ++ ++ free(uid_str); ++ + problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0'; + char* new_path = concat_path_file(base_dir_name, problem_id); + log_info("Renaming from '%s' to '%s'", dd->dd_dirname, new_path); +@@ -156,17 +197,9 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, + + if (uid_str) + { +- char *endptr; +- errno = 0; +- long val = strtol(uid_str, &endptr, 10); +- +- if (errno != 0 || endptr == uid_str || *endptr != '\0' || INT_MAX < val) +- { +- error_msg(_("uid value is not valid: '%s'"), uid_str); ++ uid = parse_uid(uid_str); ++ if (uid == (uid_t)-1) + return NULL; +- } +- +- uid = (uid_t)val; + } + + return create_dump_dir_from_problem_data_ext(problem_data, base_dir_name, uid); +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index cc0148c..0e491ce 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -532,3 +532,76 @@ int main(void) + return 0; + } + ]]) ++ ++## --------------- ## ++## create_dump_dir ## ++## --------------- ## ++ ++AT_TESTFUN([create_dump_dir], ++[[ ++#include "internal_libreport.h" ++#include ++# ++int main(void) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX"; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ printf("Base dump dir path: %s\n", template); ++ ++ dd_g_fs_group_gid = getegid(); ++ ++ problem_data_t *pd = problem_data_new(); ++ ++ problem_data_add_text_editable(pd, FILENAME_TYPE, "attest"); ++ ++ { ++ fprintf(stderr, "=== NO UID - geteuid() ===\n"); ++ struct dump_dir *no_uid_dd_geteuid = create_dump_dir_from_problem_data_ext(pd, template, geteuid()); ++ assert(no_uid_dd_geteuid != NULL); ++ assert(dd_get_owner(no_uid_dd_geteuid) == geteuid()); ++ assert(dd_delete(no_uid_dd_geteuid) == 0); ++ fprintf(stderr, "=== NO UID - geteuid() ===\n"); ++ } ++ ++ { ++ fprintf(stderr, "=== NO UID - NO UID ===\n"); ++ struct dump_dir *no_uid_dd_nouid = create_dump_dir_from_problem_data_ext(pd, template, (uid_t)-1L); ++ assert(no_uid_dd_nouid != NULL); ++ assert(dd_get_owner(no_uid_dd_nouid) == geteuid()); ++ assert(dd_delete(no_uid_dd_nouid) == 0); ++ fprintf(stderr, "=== NO UID - NO UID ===\n"); ++ } ++ ++ char buf[sizeof(long)*3 + 2]; ++ snprintf(buf, sizeof(buf), "%ld", (long)(geteuid() + 1)); ++ problem_data_add_text_editable(pd, FILENAME_UID, buf); ++ ++ { ++ fprintf(stderr, "=== UID - geteuid() ===\n"); ++ struct dump_dir *uid_dd_geteuid = create_dump_dir_from_problem_data_ext(pd, template, geteuid()); ++ assert(uid_dd_geteuid != NULL); ++ assert(dd_get_owner(uid_dd_geteuid) == (geteuid() + 1)); ++ assert(dd_delete(uid_dd_geteuid) == 0); ++ fprintf(stderr, "=== UID - geteuid() ===\n"); ++ } ++ ++ { ++ fprintf(stderr, "=== UID - NO UID ===\n"); ++ struct dump_dir *uid_dd_nouid = create_dump_dir_from_problem_data_ext(pd, template, (uid_t)-1L); ++ assert(uid_dd_nouid != NULL); ++ assert(dd_get_owner(uid_dd_nouid) == (geteuid() + 1)); ++ assert(dd_delete(uid_dd_nouid) == 0); ++ fprintf(stderr, "=== UID - NO UID ===\n"); ++ } ++ ++ assert(rmdir(template) == 0); ++ return EXIT_SUCCESS; ++} ++]]) +-- +2.1.0 + diff --git a/0085-dd-don-t-use-eUID-for-owner-use-fs-uid-instead.patch b/0085-dd-don-t-use-eUID-for-owner-use-fs-uid-instead.patch new file mode 100644 index 0000000..4fd8683 --- /dev/null +++ b/0085-dd-don-t-use-eUID-for-owner-use-fs-uid-instead.patch @@ -0,0 +1,32 @@ +From d2ec7ee0b2c6ce230b9ef1b02e00b0a98cd26697 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 19:43:00 +0200 +Subject: [PATCH] dd: don't use eUID for owner, use fs uid instead + +Set the owner to the safest value and avoid confusions. + +Signed-off-by: Jakub Filak +--- + src/lib/dump_dir.c | 6 +----- + 1 file changed, 1 insertion(+), 5 deletions(-) + +diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c +index 1e3fc6a..fda85a0 100644 +--- a/src/lib/dump_dir.c ++++ b/src/lib/dump_dir.c +@@ -692,11 +692,7 @@ int dd_set_owner(struct dump_dir *dd, uid_t owner) + char long_str[sizeof(long) * 3 + 2]; + + if (owner == (uid_t)-1) +- { +- owner = getuid(); +- if (owner < 0) +- perror_msg_and_die("%s: getuid", __func__); +- } ++ owner = dd->dd_uid; + + snprintf(long_str, sizeof(long_str), "%li", (long)owner); + const int ret = dd_meta_data_save_text(dd, META_DATA_FILE_OWNER, long_str); +-- +2.1.0 + diff --git a/0086-testsuite-add-test-for-dd_stat_for_uid.patch b/0086-testsuite-add-test-for-dd_stat_for_uid.patch new file mode 100644 index 0000000..a11f99e --- /dev/null +++ b/0086-testsuite-add-test-for-dd_stat_for_uid.patch @@ -0,0 +1,115 @@ +From 4a9b4b72b404c2c5ccdaa1b1cbeb9abd34629b5c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 20:09:08 +0200 +Subject: [PATCH] testsuite: add test for dd_stat_for_uid() + +Signed-off-by: Jakub Filak +--- + tests/dump_dir.at | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 91 insertions(+) + +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index 0e491ce..6a6e395 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -439,6 +439,97 @@ int main(int argc, char **argv) + } + ]]) + ++## ------- ## ++## dd_stat ## ++## ------- ## ++ ++AT_TESTFUN([dd_stat], ++[[ ++#include "internal_libreport.h" ++#include ++#include ++ ++int main(int argc, char **argv) ++{ ++ g_verbose = 3; ++ ++ char template[] = "/tmp/XXXXXX/dump_dir"; ++ ++ char *last_slash = strrchr(template, '/'); ++ *last_slash = '\0'; ++ ++ if (mkdtemp(template) == NULL) { ++ perror("mkdtemp()"); ++ return EXIT_FAILURE; ++ } ++ ++ *last_slash = '/'; ++ ++ printf("Dump dir path: %s\n", template); ++ ++ dd_g_super_user_uid = geteuid(); ++ ++ struct dump_dir *dd = dd_create(template, (uid_t)-1, 0640); ++ assert(dd != NULL || !"create new"); ++ dd_create_basic_files(dd, geteuid(), NULL); ++ dd_save_text(dd, "type", "custom"); ++ dd_close(dd); ++ dd = NULL; ++ ++ dd = dd_opendir(template, DD_OPEN_FD_ONLY); ++ assert(dd != NULL || !"open fds"); ++ { ++ const int fst_stat = dd_stat_for_uid(dd, geteuid()); ++ ++ assert(fst_stat & DD_STAT_OWNED_BY_UID); ++ assert(fst_stat & DD_STAT_ACCESSIBLE_BY_UID); ++ assert(!(fst_stat & DD_STAT_NO_OWNER)); ++ } ++ ++ dd = dd_fdopendir(dd, /*for writing*/0); ++ assert(dd != NULL || !"reopen for writing"); ++ ++ assert(dd_set_owner(dd, geteuid() + 1) == 0); ++ assert(dd_get_owner(dd) == geteuid() + 1); ++ ++ { ++ const int scn_stat = dd_stat_for_uid(dd, geteuid() + 2); ++ assert(scn_stat == 0); ++ } ++ ++ { ++ const int thr_stat = dd_stat_for_uid(dd, dd_g_super_user_uid); ++ assert(!(thr_stat & DD_STAT_OWNED_BY_UID)); ++ assert(thr_stat & DD_STAT_ACCESSIBLE_BY_UID); ++ assert(!(thr_stat & DD_STAT_NO_OWNER)); ++ } ++ ++ assert(dd_set_no_owner(dd) == 0); ++ assert(dd_get_owner(dd) != geteuid() + 3); ++ ++ { ++ const int frt_stat = dd_stat_for_uid(dd, geteuid() + 3); ++ assert(!(frt_stat & DD_STAT_OWNED_BY_UID)); ++ assert(frt_stat & DD_STAT_ACCESSIBLE_BY_UID); ++ assert(frt_stat & DD_STAT_NO_OWNER); ++ } ++ ++ { ++ const int fft_stat = dd_stat_for_uid(dd, dd_g_super_user_uid); ++ assert(!(fft_stat & DD_STAT_OWNED_BY_UID)); ++ assert(fft_stat & DD_STAT_ACCESSIBLE_BY_UID); ++ assert(fft_stat & DD_STAT_NO_OWNER); ++ } ++ ++ assert(dd_delete(dd) == 0); ++ ++ *last_slash = '\0'; ++ assert(rmdir(template) == 0); ++ ++ return EXIT_SUCCESS; ++} ++]]) ++ + ## -------------- ## + ## recursive_lock ## + ## -------------- ## +-- +2.1.0 + diff --git a/0087-report-wrap-more-dump-dir-functions.patch b/0087-report-wrap-more-dump-dir-functions.patch new file mode 100644 index 0000000..c1a091d --- /dev/null +++ b/0087-report-wrap-more-dump-dir-functions.patch @@ -0,0 +1,99 @@ +From 3870f5af1b1b2fa4c2be931351f771a2c59ac76c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 8 Jun 2015 19:56:41 +0200 +Subject: [PATCH] report: wrap more dump dir functions + +Signed-off-by: Jakub Filak +--- + src/report-python/dump_dir.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 57 insertions(+) + +diff --git a/src/report-python/dump_dir.c b/src/report-python/dump_dir.c +index f9d96cf..bd8a2a4 100644 +--- a/src/report-python/dump_dir.c ++++ b/src/report-python/dump_dir.c +@@ -77,6 +77,42 @@ static PyObject *p_dd_delete(PyObject *pself, PyObject *args) + Py_RETURN_NONE; + } + ++/* void dd_rename(struct dump_dir *dd, const char *new_name); */ ++static PyObject *p_dd_rename(PyObject *pself, PyObject *args) ++{ ++ p_dump_dir *self = (p_dump_dir*)pself; ++ if (!self->dd) ++ { ++ PyErr_SetString(ReportError, "dump dir is not open"); ++ return NULL; ++ } ++ const char *new_name; ++ if (!PyArg_ParseTuple(args, "s", &new_name)) ++ { ++ return NULL; ++ } ++ return Py_BuildValue("i", dd_rename(self->dd, new_name)); ++} ++ ++/* void dd_create_basic_files(struct dump_dir *dd, uid_t uid, const char *chroot_dir); */ ++static PyObject *p_dd_create_basic_files(PyObject *pself, PyObject *args) ++{ ++ p_dump_dir *self = (p_dump_dir*)pself; ++ if (!self->dd) ++ { ++ PyErr_SetString(ReportError, "dump dir is not open"); ++ return NULL; ++ } ++ uid_t uid; ++ const char *chroot_dir = NULL; ++ if (!PyArg_ParseTuple(args, "i|z", &uid, &chroot_dir)) ++ { ++ return NULL; ++ } ++ dd_create_basic_files(self->dd, uid, chroot_dir); ++ Py_RETURN_NONE; ++} ++ + /* int dd_exist(struct dump_dir *dd, const char *path); */ + static PyObject *p_dd_exist(PyObject *pself, PyObject *args) + { +@@ -160,6 +196,24 @@ static PyObject *p_dd_save_binary(PyObject *pself, PyObject *args) + Py_RETURN_NONE; + } + ++/* void dd_copy_file(struct dump_dir *dd, const char *name, const char *source_path); */ ++static PyObject *p_dd_copy_file(PyObject *pself, PyObject *args) ++{ ++ p_dump_dir *self = (p_dump_dir*)pself; ++ if (!self->dd) ++ { ++ PyErr_SetString(ReportError, "dump dir is not open"); ++ return NULL; ++ } ++ const char *name; ++ const char *source_path; ++ if (!PyArg_ParseTuple(args, "ss", &name, &source_path)) ++ { ++ return NULL; ++ } ++ return Py_BuildValue("i", dd_copy_file(self->dd, name, source_path)); ++} ++ + /* int dd_delete_item(struct dump_dir *dd, const char *name); */ + static PyObject *p_dd_delete_item(PyObject *pself, PyObject *args) + { +@@ -200,10 +254,13 @@ static PyMethodDef p_dump_dir_methods[] = { + /* method_name, func, flags, doc_string */ + { "close" , p_dd_close, METH_NOARGS, NULL }, + { "delete" , p_dd_delete, METH_NOARGS, NULL }, ++ { "rename" , p_dd_rename, METH_VARARGS, NULL }, ++ { "create_basic_files", p_dd_create_basic_files, METH_VARARGS, NULL }, + { "exist" , p_dd_exist, METH_VARARGS, NULL }, + { "load_text" , p_dd_load_text, METH_VARARGS, NULL }, + { "save_text" , p_dd_save_text, METH_VARARGS, NULL }, + { "save_binary", p_dd_save_binary, METH_VARARGS, NULL }, ++ { "copy_file" , p_dd_copy_file, METH_VARARGS, NULL }, + { "delete_item", p_dd_delete_item, METH_VARARGS, NULL }, + { NULL } + }; +-- +2.1.0 + diff --git a/0088-cli-fix-gcc-5-warning.patch b/0088-cli-fix-gcc-5-warning.patch new file mode 100644 index 0000000..18de72f --- /dev/null +++ b/0088-cli-fix-gcc-5-warning.patch @@ -0,0 +1,26 @@ +From 9e9a395e5121e4cdc4f42954951d89f0bfbf416c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 12 Feb 2015 17:24:09 +0100 +Subject: [PATCH] cli: fix gcc-5 warning + +Signed-off-by: Jakub Filak +--- + src/cli/cli.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/cli/cli.c b/src/cli/cli.c +index a467bf6..ff5b29f 100644 +--- a/src/cli/cli.c ++++ b/src/cli/cli.c +@@ -116,7 +116,7 @@ int main(int argc, char** argv) + * FALSE == FALSE (need arg but havent). + * OPT_list_events is an exception, it can be used in both cases. + */ +- ((!(opts & OPTMASK_need_arg) == argc) && (op != OPT_list_events)) ++ (((!(opts & OPTMASK_need_arg)) == argc) && (op != OPT_list_events)) + ) { + show_usage_and_die(program_usage_string, program_options); + } +-- +2.1.0 + diff --git a/0089-testsuite-add-analyzer-files-to-dump-dirs.patch b/0089-testsuite-add-analyzer-files-to-dump-dirs.patch new file mode 100644 index 0000000..0df0b9a --- /dev/null +++ b/0089-testsuite-add-analyzer-files-to-dump-dirs.patch @@ -0,0 +1,75 @@ +From eafc1adcfa59c53240143064fbe85a9cd0374372 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 11 Jun 2015 15:29:35 +0200 +Subject: [PATCH] testsuite: add 'analyzer' files to dump dirs + +Signed-off-by: Jakub Filak +--- + tests/dump_dir.at | 9 +++++++++ + 1 file changed, 9 insertions(+) + +diff --git a/tests/dump_dir.at b/tests/dump_dir.at +index 6a6e395..8e6e0d5 100644 +--- a/tests/dump_dir.at ++++ b/tests/dump_dir.at +@@ -39,6 +39,9 @@ void validate_dump_dir_contents(struct dump_dir *dd) + assert(dd_exist(dd, FILENAME_TYPE)); + ++items; + ++ assert(dd_exist(dd, FILENAME_ANALYZER)); ++ ++items; ++ + assert(dd_exist(dd, FILENAME_LAST_OCCURRENCE)); + ++items; + +@@ -113,6 +116,7 @@ int main(int argc, char **argv) + + dd_create_basic_files(dd, geteuid(), NULL); + dd_save_text(dd, FILENAME_TYPE, "attest"); ++ dd_save_text(dd, FILENAME_ANALYZER, "attest"); + + dd_save_text(dd, "at_test_text", "foo"); + assert(dd_exist(dd, "at_test_text")); +@@ -210,6 +214,7 @@ int main(int argc, char **argv) + + dd_create_basic_files(dd, (uid_t)-1, NULL); + dd_save_text(dd, FILENAME_TYPE, "attest"); ++ dd_save_text(dd, FILENAME_ANALYZER, "attest"); + + dd_close(dd); + dd = NULL; +@@ -268,6 +273,7 @@ int main(int argc, char **argv) + + dd_create_basic_files(dd, (uid_t)-1, NULL); + dd_save_text(dd, FILENAME_TYPE, "attest"); ++ dd_save_text(dd, FILENAME_ANALYZER, "attest"); + + dd_close(dd); + +@@ -473,6 +479,7 @@ int main(int argc, char **argv) + assert(dd != NULL || !"create new"); + dd_create_basic_files(dd, geteuid(), NULL); + dd_save_text(dd, "type", "custom"); ++ dd_save_text(dd, "analyzer", "custom"); + dd_close(dd); + dd = NULL; + +@@ -556,6 +563,7 @@ int main(int argc, char **argv) + + dd_create_basic_files(dd, -1L, "/"); + dd_save_text(dd, "type", "custom"); ++ dd_save_text(dd, "analyzer", "custom"); + + struct dump_dir *dd2 = dd_opendir(path, DD_OPEN_READONLY); + assert(dd2->owns_lock == 0); +@@ -651,6 +659,7 @@ int main(void) + problem_data_t *pd = problem_data_new(); + + problem_data_add_text_editable(pd, FILENAME_TYPE, "attest"); ++ problem_data_add_text_editable(pd, FILENAME_ANALYZER, "attest"); + + { + fprintf(stderr, "=== NO UID - geteuid() ===\n"); +-- +2.1.0 + diff --git a/0090-testsuite-fix-parse_numbers-on-i686.patch b/0090-testsuite-fix-parse_numbers-on-i686.patch new file mode 100644 index 0000000..34cfde1 --- /dev/null +++ b/0090-testsuite-fix-parse_numbers-on-i686.patch @@ -0,0 +1,94 @@ +From 17c7912c49c0135e232fe669e561808bc5373db2 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 10 Jun 2015 08:11:29 +0200 +Subject: [PATCH] testsuite: fix parse_numbers on i686 + +Signed-off-by: Jakub Filak +--- + tests/xfuncs.at | 26 +++++++++++++------------- + 1 file changed, 13 insertions(+), 13 deletions(-) + +diff --git a/tests/xfuncs.at b/tests/xfuncs.at +index dbcc602..222824c 100644 +--- a/tests/xfuncs.at ++++ b/tests/xfuncs.at +@@ -86,23 +86,23 @@ int main(void) + assert(try_atou("foo54321", &uint_value) != 0); + assert(uint_value == 12345); + +- char buf[sizeof(unsigned long) * 3 + 1]; ++ char buf[sizeof(unsigned long long) * 3 + 1]; + +- snprintf(buf, sizeof(buf), "%lu", 1LU + UINT_MAX); ++ snprintf(buf, sizeof(buf), "%llu", 1LLU + UINT_MAX); + assert(try_atou(buf, &uint_value) != 0 || !"Above UINT_MAX"); + assert(uint_value == 12345); + + assert(try_atou("-1", &uint_value) != 0); + assert(uint_value == 12345); + +- snprintf(buf, sizeof(buf), "%lu", (long unsigned)UINT_MAX); ++ snprintf(buf, sizeof(buf), "%llu", (unsigned long long)UINT_MAX); + assert(try_atou(buf, &uint_value) == 0); + assert(uint_value == UINT_MAX); +- assert(xatou(buf) == UINT_MAX); ++ assert(xatou(buf) == UINT_MAX); + + assert(try_atou("0", &uint_value) == 0); + assert(uint_value == 0); +- assert(xatou("0") == 0); ++ assert(xatou("0") == 0); + } + + { +@@ -113,22 +113,22 @@ int main(void) + assert(try_atoi("foo54321", &int_value) != 0); + assert(int_value == 12345); + +- char buf[sizeof(long) * 3 + 1]; ++ char buf[sizeof(long long) * 3 + 1]; + +- snprintf(buf, sizeof(buf), "%ld", 1L + INT_MAX); ++ snprintf(buf, sizeof(buf), "%lld", 1LL + INT_MAX); + assert(try_atoi(buf, &int_value) != 0 || !"Parse INT_MAX+1"); + assert(int_value == 12345 || !"Above INT_MAX"); + +- snprintf(buf, sizeof(buf), "%ld", -1L + INT_MIN); ++ snprintf(buf, sizeof(buf), "%lld", -1LL + INT_MIN); + assert(try_atoi(buf, &int_value) != 0 || !"Parse INT_MIN-1"); + assert(int_value == 12345 || !"Belove INT_MIN"); + +- snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MAX); ++ snprintf(buf, sizeof(buf), "%lld", (unsigned long long)INT_MAX); + assert(try_atoi(buf, &int_value) == 0 || !"Parse INT_MAX"); + assert(int_value == INT_MAX); + assert(xatoi(buf) == INT_MAX); + +- snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MIN); ++ snprintf(buf, sizeof(buf), "%lld", (unsigned long long)INT_MIN); + assert(try_atoi(buf, &int_value) == 0 || !"Parse INT_MIN"); + assert(int_value == INT_MIN); + assert(xatoi(buf) == INT_MIN); +@@ -142,16 +142,16 @@ int main(void) + assert(try_atoi_positive("foo54321", &positive_value) != 0); + assert(positive_value == 12345); + +- char buf[sizeof(long) * 3 + 1]; ++ char buf[sizeof(long long) * 3 + 1]; + +- snprintf(buf, sizeof(buf), "%ld", 1L + INT_MAX); ++ snprintf(buf, sizeof(buf), "%lld", 1LL + INT_MAX); + assert(try_atoi_positive(buf, &positive_value) != 0); + assert(positive_value == 12345 || !"Above INT_MAX"); + + assert(try_atoi_positive("-1", &positive_value) != 0); + assert(positive_value == 12345 || !"After -1"); + +- snprintf(buf, sizeof(buf), "%ld", (long unsigned)INT_MAX); ++ snprintf(buf, sizeof(buf), "%lld", (unsigned long long)INT_MAX); + assert(try_atoi_positive(buf, &positive_value) == 0 || !"Parse INT_MAX"); + assert(positive_value == INT_MAX); + assert(xatoi_positive(buf) == INT_MAX); +-- +2.1.0 + diff --git a/libreport.spec b/libreport.spec index aed4d75..c5f860e 100644 --- a/libreport.spec +++ b/libreport.spec @@ -7,7 +7,7 @@ Summary: Generic library for reporting various problems Name: libreport Version: 2.3.0 -Release: 6%{?dist} +Release: 7%{?dist} License: GPLv2+ Group: System Environment/Libraries URL: https://github.com/abrt/abrt/wiki/ABRT-Project @@ -62,6 +62,50 @@ Patch0043: 0043-dump_dir-allow-semi-recursive-locking.patch Patch0044: 0044-ignored-words-passed-is-not-a-dirty-word.patch Patch0045: 0045-ignored-words-fix-a-typo-in-SHELL-sbin-nologin.patch Patch0046: 0046-ignored-words-add-a-few-key-and-access-words.patch +Patch0047: 0047-testsuite-make-report_python-os-release-test-more-ve.patch +Patch0048: 0048-report-python-fix-getVersion_fromOSRELEASE.patch +Patch0049: 0049-testsuite-report_python-dump-etc-os-relase-to-stdout.patch +Patch0050: 0050-lib-add-proc-pid-utils.patch +Patch0051: 0051-dump_dir-add-function-for-creating-a-dump-dir-in-dum.patch +Patch0052: 0052-Whitespace-fix.patch +Patch0053: 0053-dump_dir-introduce-dd_copy_file.patch +Patch0054: 0054-dump_dir-introduce-dd_copy_file_unpack.patch +#Patch0055: 0055-spec-add-xz-devel-to-the-build-requirements.patch +Patch0056: 0056-rewrite-dump_fd_info.patch +Patch0057: 0057-disable-gdk-deprecation-warnings.patch +Patch0058: 0058-fix-several-memory-leaks.patch +Patch0059: 0059-utils-make-arguments-of-a-list-func-const.patch +Patch0060: 0060-includes-move-stdbool.h-to-libreport_types.h.patch +Patch0061: 0061-ignored-words-ignore-lxqt-openssh-askpass.patch +Patch0062: 0062-problem_data-add-a-new-function-problem_item_get_siz.patch +Patch0063: 0063-problem_data-cache-problem_item-size.patch +Patch0064: 0064-report-client-provide-cpio-log-when-unpacking-fails.patch +Patch0065: 0065-report-client-fix-close-an-unclosed-file.patch +Patch0066: 0066-report-client-check-owner-of-var-cache-abrt-di-when-.patch +Patch0067: 0067-dumpdir-fix-initialization-of-dd_gid.patch +Patch0068: 0068-lib-make-the-dump-proc-data-functions-more-robust.patch +Patch0069: 0069-lib-introduce-a-new-function-copy_file_ext.patch +Patch0070: 0070-dump_dir-allow-creating-of-a-new-dir-w-o-chowning-it.patch +Patch0071: 0071-dump_dir-allow-hooks-to-create-dump-directory-withou.patch +Patch0072: 0072-lib-add-a-function-checking-file-names.patch +Patch0073: 0073-dd-harden-functions-against-directory-traversal-issu.patch +Patch0074: 0074-lib-allow-creating-root-owned-problem-directories-fr.patch +Patch0075: 0075-lib-fix-races-in-dump-directory-handling-code.patch +Patch0076: 0076-lib-add-alternative-dd-functions-accepting-fds.patch +Patch0077: 0077-build-switch-the-default-dump-dir-mode-to-0640.patch +Patch0078: 0078-lib-parse-fsgid-and-don-t-fall-back-to-0.patch +Patch0079: 0079-dd-add-support-for-meta-data.patch +Patch0080: 0080-dd-add-a-function-parsing-number-from-fd.patch +Patch0081: 0081-dd-add-owner-to-meta-data.patch +Patch0082: 0082-lib-add-utility-functions-parsing-numbers.patch +Patch0083: 0083-dd-make-super-user-UID-and-FS-group-configurable.patch +Patch0084: 0084-dd-set-owner-to-UID-when-creating-dd-from-problem-da.patch +Patch0085: 0085-dd-don-t-use-eUID-for-owner-use-fs-uid-instead.patch +Patch0086: 0086-testsuite-add-test-for-dd_stat_for_uid.patch +Patch0087: 0087-report-wrap-more-dump-dir-functions.patch +Patch0088: 0088-cli-fix-gcc-5-warning.patch +Patch0089: 0089-testsuite-add-analyzer-files-to-dump-dirs.patch +Patch0090: 0090-testsuite-fix-parse_numbers-on-i686.patch # git is need for '%%autosetup -S git' which automatically applies all the # patches above. Please, be aware that the patches must be generated @@ -92,6 +136,7 @@ BuildRequires: doxygen BuildRequires: systemd-devel BuildRequires: augeas-devel BuildRequires: augeas +BuildRequires: xz-devel Requires: libreport-filesystem = %{version}-%{release} # required for update from old report library, otherwise we obsolete report-gtk # and all it's plugins, but don't provide the python bindings and the sealert @@ -724,13 +769,24 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %changelog +* Tue Jun 16 2015 Matej Habrnal 2.3.0-7 +- harden the code against directory traversal, symbolic and hard link attacks +- fix a bug causing that the first value of AlwaysExcludedElements was ignored +- fix missing icon for the "Stop" button icon name +- switch the default dump dir mode to 0640 +- fix races in dump directory handling code +- improve development documentation +- translations updates +- Resolves #1213485, #1169774 + + * Tue Feb 24 2015 Matej Habrnal 2.3.0-6 - ignore (a|A)ccesib(ility|le) words - try to reduce false positive sensitive words - ureport: correct variable initializations - allow (semi)recursive locking - ignored words: add a few 'key' and 'access' words -- Resolves: #1175720 +- Resolves: #1175720, #1180135 * Fri Nov 28 2014 Jakub Filak 2.3.0-5 - anaconda: filter out rootpw lines