From a4794b39efc62c9ba92b38b419de3babbbcd8cfb Mon Sep 17 00:00:00 2001 From: Jakub Filak Date: Wed, 15 Apr 2015 15:27:09 +0200 Subject: [ABRT PATCH] ccpp: postpone changing ownership of new dump directories Florian Weimer : Currently, dd_create changes ownership of the directory immediately, when it is still empty. This means that any operations within the directory (which happen as the root user) can race with changes to the directory contents by the user. If you delay changing directory ownership until all the files have created and written, this is no longer a problem. Related: #1211835 Signed-off-by: Jakub Filak --- src/hooks/abrt-hook-ccpp.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c index ece1ece..7e05aa6 100644 --- a/src/hooks/abrt-hook-ccpp.c +++ b/src/hooks/abrt-hook-ccpp.c @@ -672,8 +672,12 @@ int main(int argc, char** argv) /* use fsuid instead of uid, so we don't expose any sensitive * information of suided app in /var/tmp/abrt + * + * dd_create_skeleton() creates a new directory and leaves ownership to + * the current user, hence, we have to call dd_reset_ownership() after the + * directory is populated. */ - dd = dd_create(path, fsuid, DEFAULT_DUMP_DIR_MODE); + dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE); if (dd) { char *rootdir = get_rootdir(pid); @@ -831,6 +835,9 @@ int main(int argc, char** argv) } #endif + /* And finally set the right uid and gid */ + dd_reset_ownership(dd); + /* We close dumpdir before we start catering for crash storm case. * Otherwise, delete_dump_dir's from other concurrent * CCpp's won't be able to delete our dump (their delete_dump_dir -- 1.8.3.1