From 9513fab99fa25f665adedf2fac6f5809a4987f2d Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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 <assert.h>
+#
+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