From: Michael Orlitzky Date: Wed, 28 Mar 2018 01:03:01 +0000 (-0400) Subject: src/libadacl.c: use asprintf() instead of snprintf() for paths. X-Git-Tag: v0.4.0~8 X-Git-Url: http://gitweb.michael.orlitzky.com/?p=apply-default-acl.git;a=commitdiff_plain;h=d8b55a1ea987e0ac8915bd5b2597d85f2d81c85d src/libadacl.c: use asprintf() instead of snprintf() for paths. When constructing a path, there is an ancient problem: how do you ensure that your path-name buffer is large enough, and what do you do if it isn't? The existing solution was to use the PATH_MAX constant from limits.h, which is often a big number, but need not actually be defined. If a path exceeded PATH_MAX bytes, we would fail. Now the GNU/BSD extension asprintf() is used instead. The path-name buffer is constructed on-the-fly to be as large as necessary, and if allocation fails, an error is returned. This solution is a little cleaner, and is not too much less portable considering that we only work on Linux anyway. --- diff --git a/configure.ac b/configure.ac index e474234..80f678d 100644 --- a/configure.ac +++ b/configure.ac @@ -20,7 +20,6 @@ AC_HEADER_REQUIRE(acl/libacl.h) AC_HEADER_REQUIRE(fcntl.h) AC_HEADER_REQUIRE(getopt.h) AC_HEADER_REQUIRE(libgen.h) -AC_HEADER_REQUIRE(limits.h) AC_HEADER_REQUIRE(linux/xattr.h) AC_HEADER_REQUIRE(sys/acl.h) AC_HEADER_REQUIRE(unistd.h) @@ -44,5 +43,11 @@ AC_CHECK_DECLS([O_PATH], [[#define _GNU_SOURCE #include ]]) +# And check for the GNU/BSD extension asprintf(), which lets us avoid +# praying to the PATH_MAX gods while constructing long paths. +AC_CHECK_FUNC(asprintf, + [], + AC_MSG_ERROR(missing required asprintf function)) + LT_INIT AC_OUTPUT diff --git a/src/libadacl.c b/src/libadacl.c index ce125db..62ab3cc 100644 --- a/src/libadacl.c +++ b/src/libadacl.c @@ -5,16 +5,17 @@ * */ -/* Enables get_current_dir_name() in unistd.h and the O_PATH flag. */ +/* Enables get_current_dir_name() in unistd.h, the O_PATH flag, and + * the asprintf() function. +*/ #define _GNU_SOURCE #include /* readdir(), etc. */ #include /* EINVAL, ELOOP, ENOTDIR, etc. */ #include /* openat() */ #include /* basename(), dirname() */ -#include /* PATH_MAX */ #include /* the "bool" type */ -#include /* perror(), snprintf() */ +#include /* perror(), asprintf() */ #include /* free() */ #include /* strdup() */ #include /* fstat() */ @@ -36,7 +37,7 @@ */ #define CLOSE_ERROR -1 #define OPEN_ERROR -1 -#define SNPRINTF_ERROR -1 +#define ASPRINTF_ERROR -1 #define STAT_ERROR -1 #define XATTR_ERROR -1 @@ -134,11 +135,11 @@ int safe_open(const char* pathname, int flags) { return OPEN_ERROR; } - char abspath[PATH_MAX]; - int snprintf_result = 0; + char* abspath = NULL; + int asprintf_result = 0; if (strchr(pathname, '/') == pathname) { /* pathname is already absolute; just copy it. */ - snprintf_result = snprintf(abspath, PATH_MAX, "%s", pathname); + asprintf_result = asprintf(&abspath, "%s", pathname); } else { /* Concatenate the current working directory and pathname into an @@ -163,14 +164,17 @@ int safe_open(const char* pathname, int flags) { free(cwd); return OPEN_ERROR; } - snprintf_result = snprintf(abspath, PATH_MAX, "%s/%s", abs_cwd, pathname); + asprintf_result = asprintf(&abspath, "%s/%s", abs_cwd, pathname); free(cwd); } - if (snprintf_result == SNPRINTF_ERROR || snprintf_result > PATH_MAX) { - perror("safe_open (snprintf)"); + if (asprintf_result == ASPRINTF_ERROR) { + perror("safe_open (asprintf)"); return OPEN_ERROR; } + /* Beyond here, asprintf() worked, and we need to free abspath. */ + int result = OPEN_ERROR; + bool abspath_is_root = (strcmp(abspath, "/") == 0); int rootflags = flags | O_DIRECTORY; if (!abspath_is_root) { @@ -180,18 +184,24 @@ int safe_open(const char* pathname, int flags) { int rootfd = open("/", rootflags); if (rootfd == OPEN_ERROR) { perror("safe_open (open)"); - return OPEN_ERROR; + result = OPEN_ERROR; + goto cleanup; } if (abspath_is_root) { - return rootfd; + result = rootfd; + goto cleanup; } - int result = safe_open_ex(rootfd, abspath+1, flags); + result = safe_open_ex(rootfd, abspath+1, flags); if (close(rootfd) == CLOSE_ERROR) { perror("safe_open (close)"); - return OPEN_ERROR; + result = OPEN_ERROR; + goto cleanup; } + + cleanup: + free(abspath); return result; }