aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Hrozek <jakub.hrozek@posteo.se>2016-01-15 12:55:51 +0100
committerMichael Adam <obnox@samba.org>2016-01-18 11:06:29 +0100
commitc91b2033bebe69ff5bab67c8db960ac5ec021268 (patch)
treedbd00ec864f245cc92154e41495b00ac9a280a07
parentd3cfb7af2a8f5ce844c46ce8c5fade8df1e2e429 (diff)
downloadpam_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.c107
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.