diff options
author | Denys Vlasenko <dvlasenk@redhat.com> | 2010-10-15 18:31:35 +0200 |
---|---|---|
committer | Denys Vlasenko <dvlasenk@redhat.com> | 2010-10-15 18:31:35 +0200 |
commit | b9c5c7c7e29b33eac351e801ce92a9adadc00907 (patch) | |
tree | 55854aadcf56488078751f7e906a2afa86247cf1 /lib/utils | |
parent | 4a13c99ba80ef3082a4a3bd1025dd6778265ec8d (diff) | |
download | abrt-b9c5c7c7e29b33eac351e801ce92a9adadc00907.tar.gz abrt-b9c5c7c7e29b33eac351e801ce92a9adadc00907.tar.xz abrt-b9c5c7c7e29b33eac351e801ce92a9adadc00907.zip |
Before this patch, dd_opendir(".") was creating
lock file "..lock", which is obviously wrong.
This patch makes dd_opendir use realpath(dir_name).
This makes dd_opendir(".") and similar cases correctly
determine what is its parent directory and therefore
use correct lock file name.
dd_create can't use realpath, since it will always return NULL
on not-yet existing directory. But this is not a problem, since
dd_create(".") isn't a sane operation. So we continue to use
old code. I only added a sanity check to refuse names ending
in . and .. - just in case.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Diffstat (limited to 'lib/utils')
-rw-r--r-- | lib/utils/dump_dir.c | 53 |
1 files changed, 40 insertions, 13 deletions
diff --git a/lib/utils/dump_dir.c b/lib/utils/dump_dir.c index ba917589..11442d9b 100644 --- a/lib/utils/dump_dir.c +++ b/lib/utils/dump_dir.c @@ -28,10 +28,6 @@ // Perhaps dd_opendir should do some sanity checking like // "if there is no "uid" file in the directory, it's not a crash dump", // and fail. -// -// Locking is broken wrt "funny" directory names. -// dd_opendir(dd, ".") will create "..lock" istead of proper "../DIRNAME.lock" -// Similarly for dd_opendir(dd, "DIRNAME/."), dd_opendir(dd, "..") etc. static char *load_text_file(const char *path); @@ -45,14 +41,6 @@ static bool isdigit_str(const char *str) return true; } -static char* rm_trailing_slashes(const char *dir) -{ - unsigned len = strlen(dir); - while (len != 0 && dir[len-1] == '/') - len--; - return xstrndup(dir, len); -} - static bool exist_file_dir(const char *path) { struct stat buf; @@ -171,12 +159,33 @@ void dd_close(struct dump_dir *dd) free(dd); } +static char* rm_trailing_slashes(const char *dir) +{ + unsigned len = strlen(dir); + while (len != 0 && dir[len-1] == '/') + len--; + return xstrndup(dir, len); +} + int dd_opendir(struct dump_dir *dd, const char *dir, int flags) { if (dd->locked) error_msg_and_die("dump_dir is already opened"); /* bug */ - dd->dd_dir = rm_trailing_slashes(dir); + /* Used to use rm_trailing_slashes(dir) here, but with dir = "." + * or "..", or if the last component is a symlink, + * then lock file is created in the wrong place. + * IOW: this breaks locking. + */ + dd->dd_dir = realpath(dir, NULL); + if (!dd->dd_dir) + { + if (!(flags & DD_FAIL_QUIETLY)) + error_msg("'%s' does not exist", dir); + if (flags & DD_CLOSE_ON_OPEN_ERR) + dd_close(dd); + return 0; + } dd_lock(dd); @@ -222,8 +231,26 @@ int dd_create(struct dump_dir *dd, const char *dir, uid_t uid) if (dd->locked) error_msg_and_die("dump_dir is already opened"); /* bug */ + /* Unlike dd_opendir, can't use realpath: the directory doesn't exist yet, + * realpath will always return NULL. We don't really have to: + * dd_opendir(".") makes sense, dd_create(".") does not. + */ dd->dd_dir = rm_trailing_slashes(dir); + char *last_component = strrchr(dd->dd_dir, '/'); + if (last_component) + last_component++; + else + last_component = dd->dd_dir; + if (dot_or_dotdot(last_component)) + { + /* dd_create("."), dd_create(".."), dd_create("dir/."), + * dd_create("dir/..") and similar are madness, refuse them. + */ + error_msg("Bad dir name '%s'", dd->dd_dir); + return 0; + } + dd_lock(dd); /* Was creating it with mode 0700 and user as the owner, but this allows |