[PATCH] lib: Fix lstat check in directory_create_or_exist

Christof Schmitt cs at samba.org
Wed Aug 29 22:41:04 UTC 2018


From 50ba2a67f016eb3eb3e5cb660b362430205054ad Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 29 Aug 2018 12:04:29 -0700
Subject: [PATCH 1/2] lib: Fix lstat check in directory_create_or_exist

The lstat check in directory_create_or_exist did not verify whether an
existing object is actually a directory. Also move the check to only
apply when mkdir returns EEXIST; this fixes CID 241930 Time of check
time of use.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 lib/util/util.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/lib/util/util.c b/lib/util/util.c
index 4291bfa5d57..f52f69c6ef0 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -194,20 +194,8 @@ _PUBLIC_ bool directory_create_or_exist(const char *dname,
 					mode_t dir_perms)
 {
 	int ret;
-	struct stat st;
 	mode_t old_umask;
 
-	ret = lstat(dname, &st);
-	if (ret == 0) {
-		return true;
-	}
-
-	if (errno != ENOENT) {
-		DBG_WARNING("lstat failed on directory %s: %s\n",
-			    dname, strerror(errno));
-		return false;
-	}
-
 	/* Create directory */
 	old_umask = umask(0);
 	ret = mkdir(dname, dir_perms);
@@ -220,11 +208,17 @@ _PUBLIC_ bool directory_create_or_exist(const char *dname,
 	}
 	umask(old_umask);
 
-	ret = lstat(dname, &st);
-	if (ret == -1) {
-		DEBUG(0, ("lstat failed on created directory %s: %s\n",
-			  dname, strerror(errno)));
-		return false;
+	if (ret != 0 && errno == EEXIST) {
+		struct stat sbuf;
+
+		ret = lstat(dname, &sbuf);
+		if (ret != 0) {
+			return false;
+		}
+
+		if (!S_ISDIR(sbuf.st_mode)) {
+			return false;
+		}
 	}
 
 	return true;
-- 
2.17.0


From 1a81643c30894fe55e12ab7e92e9446091aeca62 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 29 Aug 2018 12:07:42 -0700
Subject: [PATCH 2/2] torture: Add test for directory_create_or_exist

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 lib/util/tests/util.c | 60 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/lib/util/tests/util.c b/lib/util/tests/util.c
index ff9873800bf..10d475df62c 100644
--- a/lib/util/tests/util.c
+++ b/lib/util/tests/util.c
@@ -2,6 +2,7 @@
  * Tests for strv_util
  *
  * Copyright Martin Schwenke <martin at meltin.net> 2016
+ * Copyright Christof Schmitt <cs at samba.org> 2018
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -18,6 +19,7 @@
  */
 
 #include <talloc.h>
+#include <fcntl.h>
 
 #include "replace.h"
 
@@ -364,6 +366,61 @@ static bool test_trim_string(struct torture_context *tctx)
 	return true;
 }
 
+static bool test_directory_create_or_exist(struct torture_context *tctx)
+{
+	char *path = NULL, *new_path = NULL, *file_path = NULL;
+	bool ret = true, b = true;
+	int fd;
+	NTSTATUS status;
+	const mode_t perms = 0741;
+
+	status = torture_temp_dir(tctx, "util_dir", &path);;
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"Creating test directory failed.\n");
+
+	b = directory_create_or_exist(path, perms);
+	torture_assert_goto(tctx, b == true, ret, done,
+			    "directory_create_or_exist on "
+			    "existing directory failed.\n");
+
+	new_path = talloc_asprintf(tctx, "%s/%s", path, "dir");
+	torture_assert_goto(tctx, new_path != NULL, ret, done,
+			    "Could not allocate memory for directory path\n");
+
+	b = directory_exist(new_path);
+	torture_assert_goto(tctx, b == false, ret, done,
+			    "Check for non-existing directory failed.\n");
+
+	b = directory_create_or_exist(new_path, perms);
+	torture_assert_goto(tctx, b == true, ret, done,
+			    "directory_create_or_exist for "
+			    "new directory failed.\n");
+
+	b = directory_exist(new_path);
+	torture_assert_goto(tctx, b == true, ret, done,
+			    "Check for existing directory failed.\n");
+
+	b = file_check_permissions(new_path, geteuid(), perms, NULL);
+	torture_assert_goto(tctx, b == true, ret, done,
+			    "Permission check for directory failed.\n");
+
+	file_path = talloc_asprintf(tctx, "%s/%s", path, "file");
+	torture_assert_goto(tctx, file_path != NULL, ret, done,
+			    "Could not allocate memory for file path\n");
+	fd = creat(file_path, perms);
+	torture_assert_goto(tctx, fd != -1, ret, done,
+			    "Creating file failed.\n");
+	close(fd);
+
+	b = directory_create_or_exist(file_path, perms);
+	torture_assert_goto(tctx, b == false, ret, done,
+			    "directory_create_or_exist for "
+			    "existing file failed.\n");
+
+done:
+	return ret;
+}
+
 struct torture_suite *torture_local_util(TALLOC_CTX *mem_ctx)
 {
 	struct torture_suite *suite =
@@ -372,5 +429,8 @@ struct torture_suite *torture_local_util(TALLOC_CTX *mem_ctx)
 	torture_suite_add_simple_test(suite,
 				      "trim_string",
 				      test_trim_string);
+	torture_suite_add_simple_test(suite,
+				      "directory_create_or_exist",
+				      test_directory_create_or_exist);
 	return suite;
 }
-- 
2.17.0



More information about the samba-technical mailing list