diff options
author | Jakub Hrozek <jakub.hrozek@posteo.se> | 2016-01-15 12:55:51 +0100 |
---|---|---|
committer | Michael Adam <obnox@samba.org> | 2016-01-18 11:06:29 +0100 |
commit | c91b2033bebe69ff5bab67c8db960ac5ec021268 (patch) | |
tree | dbd00ec864f245cc92154e41495b00ac9a280a07 | |
parent | d3cfb7af2a8f5ce844c46ce8c5fade8df1e2e429 (diff) | |
download | pam_wrapper-c91b2033bebe69ff5bab67c8db960ac5ec021268.tar.gz pam_wrapper-c91b2033bebe69ff5bab67c8db960ac5ec021268.tar.xz pam_wrapper-c91b2033bebe69ff5bab67c8db960ac5ec021268.zip |
pwrap: Improve p_rmdirs() do avoid timing issues
When calling stat and rmdir, we could run into timing issues that the
stat information is incorrect till we are actually running the rmdir()
command. So we open the directory and have the handle open to avoid
that we work on outdated information. It is unlikely but Coverity
complains and we thought better fix it.
CID: 47519
Signed-off-by: Jakub Hrozek <jakub.hrozek@posteo.se>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
-rw-r--r-- | src/pam_wrapper.c | 107 |
1 files changed, 54 insertions, 53 deletions
diff --git a/src/pam_wrapper.c b/src/pam_wrapper.c index e9d7e8c..6b696cd 100644 --- a/src/pam_wrapper.c +++ b/src/pam_wrapper.c @@ -1533,74 +1533,75 @@ int audit_open(void) * DESTRUCTOR ***************************/ -static int p_rmdirs(const char *path) +static int p_rmdirs_at(const char *path, int parent_fd) { DIR *d; struct dirent *dp; struct stat sb; - char *fname; + int path_fd; + int rc; - if ((d = opendir(path)) != NULL) { - while(stat(path, &sb) == 0) { - /* if we can remove the directory we're done */ - if (rmdir(path) == 0) { - break; - } - switch (errno) { - case ENOTEMPTY: - case EEXIST: - case EBADF: - break; /* continue */ - default: - closedir(d); - return 0; - } + /* If path is absolute, parent_fd is ignored. */ + PWRAP_LOG(PWRAP_LOG_TRACE, + "p_rmdirs_at removing %s at %d\n", path, parent_fd); - while ((dp = readdir(d)) != NULL) { - size_t len; - /* skip '.' and '..' */ - if (dp->d_name[0] == '.' && - (dp->d_name[1] == '\0' || - (dp->d_name[1] == '.' && dp->d_name[2] == '\0'))) { - continue; - } + path_fd = openat(parent_fd, + path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (path_fd == -1) { + return -1; + } - len = strlen(path) + strlen(dp->d_name) + 2; - fname = malloc(len); - if (fname == NULL) { - closedir(d); - return -1; - } - snprintf(fname, len, "%s/%s", path, dp->d_name); - - /* stat the file */ - if (lstat(fname, &sb) != -1) { - if (S_ISDIR(sb.st_mode) && !S_ISLNK(sb.st_mode)) { - if (rmdir(fname) < 0) { /* can't be deleted */ - if (errno == EACCES) { - closedir(d); - SAFE_FREE(fname); - return -1; - } - p_rmdirs(fname); - } - } else { - unlink(fname); - } - } /* lstat */ - SAFE_FREE(fname); - } /* readdir */ + d = fdopendir(path_fd); + if (d == NULL) { + close(path_fd); + return -1; + } - rewinddir(d); + while ((dp = readdir(d)) != NULL) { + /* skip '.' and '..' */ + if (dp->d_name[0] == '.' && + (dp->d_name[1] == '\0' || + (dp->d_name[1] == '.' && dp->d_name[2] == '\0'))) { + continue; } - } else { + + rc = fstatat(path_fd, dp->d_name, + &sb, AT_SYMLINK_NOFOLLOW); + if (rc != 0) { + continue; + } + + if (S_ISDIR(sb.st_mode)) { + rc = p_rmdirs_at(dp->d_name, path_fd); + } else { + rc = unlinkat(path_fd, dp->d_name, 0); + } + if (rc != 0) { + continue; + } + } + closedir(d); + + rc = unlinkat(parent_fd, path, AT_REMOVEDIR); + if (rc != 0) { + rc = errno; + PWRAP_LOG(PWRAP_LOG_TRACE, + "cannot unlink %s error %d\n", path, rc); return -1; } - closedir(d); return 0; } +static int p_rmdirs(const char *path) +{ + /* + * If path is absolute, p_rmdirs_at ignores parent_fd. + * If it's relative, start from cwd. + */ + return p_rmdirs_at(path, AT_FDCWD); +} + /* * This function is called when the library is unloaded and makes sure that * resources are freed. |