summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--util/nvmutil/include/common.h13
-rw-r--r--util/nvmutil/lib/file.c213
-rw-r--r--util/nvmutil/lib/io.c3
-rw-r--r--util/nvmutil/lib/state.c3
4 files changed, 98 insertions, 134 deletions
diff --git a/util/nvmutil/include/common.h b/util/nvmutil/include/common.h
index b47e5f4b..e856664f 100644
--- a/util/nvmutil/include/common.h
+++ b/util/nvmutil/include/common.h
@@ -478,8 +478,7 @@ const char *getnvmprogname(void);
/* libc hardening
*/
-char *new_tmpfile(int *fd);
-char *new_tmplate(int *fd);
+int new_tmpfile(int *fd, char **path);
static int mkhtemp_try_create(int dirfd,
struct stat *st_dir_initial,
char *fname_copy,
@@ -497,9 +496,13 @@ int same_dir(const char *a, const char *b);
int tmpdir_policy(const char *path,
int *allow_noworld_unsticky);
char *env_tmpdir(int always_sticky);
-int secure_file(int *fd, struct stat *st,
- int bad_flags, int check_seek,
- int do_lock, mode_t mode);
+int secure_file(int *fd,
+ struct stat *st,
+ struct stat *expected,
+ int bad_flags,
+ int check_seek,
+ int do_lock,
+ mode_t mode);
int close_on_eintr(int fd);
int fsync_on_eintr(int fd);
int fs_rename_at(int olddirfd, const char *old,
diff --git a/util/nvmutil/lib/file.c b/util/nvmutil/lib/file.c
index ee849b73..50b48065 100644
--- a/util/nvmutil/lib/file.c
+++ b/util/nvmutil/lib/file.c
@@ -30,6 +30,9 @@
/* check that a file changed
*/
+#define MKHTEMP_RETRY_MAX 512
+#define MKHTEMP_SPIN_THRESHOLD 32
+
int
same_file(int fd, struct stat *st_old,
int check_size)
@@ -234,8 +237,10 @@ err_fsync_dir:
/* hardened tmpfile creation
*/
-char *
-new_tmpfile(int *fd)
+/* returns opened fd, sets fd and *path at pointers *fd and *path */
+/* also sets external stat */
+int
+new_tmpfile(int *fd, char **path)
{
/* TODO:
* directory support (currently only files)
@@ -253,7 +258,6 @@ new_tmpfile(int *fd)
char *tmpdir = NULL;
size_t dirlen;
- size_t pathlen;
size_t destlen;
size_t baselen;
char *dest = NULL; /* final path */
@@ -261,43 +265,26 @@ new_tmpfile(int *fd)
int dirfd = -1;
const char *fname = NULL;
- /* open the directory early,
- * check via fd throughout,
- * for directory replacement
- * attack mitigation
- */
struct stat st_dir_initial;
char *dir = NULL;
char *base = NULL;
- if (fd == NULL) {
-
+ if (path == NULL || fd == NULL) {
+ errno = EFAULT;
+ goto err_new_tmpfile;
+ } else if (*path == NULL) {
errno = EFAULT;
goto err_new_tmpfile;
}
- /* block operating on
- * someone elses file
- */
- if (*fd >= 0) {
-
+ if (*fd >= 0) { /* file already opend */
errno = EEXIST;
goto err_new_tmpfile;
-
- /* they might
- * want it later!
- */
}
- /* but now it's *mine*:
- */
*fd = -1;
- /* base dir e.g. /tmp
- */
-
#if defined(PERMIT_NON_STICKY_ALWAYS) && \
((PERMIT_NON_STICKY_ALWAYS) > 0)
-
tmpdir = env_tmpdir(PERMIT_NON_STICKY_ALWAYS);
#else
tmpdir = env_tmpdir(0);
@@ -305,22 +292,13 @@ new_tmpfile(int *fd)
if (tmpdir == NULL)
goto err_new_tmpfile;
-
if (slen(tmpdir, maxlen, &dirlen) < 0)
goto err_new_tmpfile;
- pathlen = 0;
-
- /* now we want the base dir,
- * with the file appended,
- * and the XXXXXXXXXX suffix
- */
-
/* using sizeof (not slen) adds an extra byte,
* useful because we either want '.' or '/'
*/
- destlen = dirlen + pathlen + sizeof(suffix);
-
+ destlen = dirlen + sizeof(suffix);
if (destlen > maxlen - 1) { /* -1 for NULL */
errno = EOVERFLOW;
@@ -334,34 +312,22 @@ new_tmpfile(int *fd)
goto err_new_tmpfile;
}
- /* As you see above, we only allow
- * either a base tmpdir and suffix,
- * or a user-supplied file and we
- * suffix that.
- */
- if (dirlen > 0 && pathlen > 0) {
-
- errno = EINVAL;
- goto err_new_tmpfile; /* pre-emptive fix */
- }
-
- *(dest + dirlen) = '/';
memcpy(dest, tmpdir, dirlen);
- memcpy(dest + dirlen + 1, suffix,
- sizeof(suffix) - 1);
+ *(dest + dirlen) = '/';
+ memcpy(dest + dirlen + 1, suffix, sizeof(suffix) - 1);
+ *(dest + destlen) = '\0';
+
+ /* new tmpfile name */
+ fname = dest + dirlen + 1;
dirfd = fs_open(tmpdir,
O_RDONLY | O_DIRECTORY);
if (dirfd < 0)
goto err_new_tmpfile;
- fname = dest + dirlen + 1;
-
if (fstat(dirfd, &st_dir_initial) < 0)
goto err_new_tmpfile;
- *(dest + destlen) = '\0';
-
*fd = mkhtemp(fd, &st, dest, dirfd, fname, &st_dir_initial);
if (*fd < 0)
goto err_new_tmpfile;
@@ -370,14 +336,16 @@ new_tmpfile(int *fd)
(void) close_on_eintr(dirfd);
dirfd = -1;
}
-
if (dir != NULL) {
free(dir);
dir = NULL;
}
errno = saved_errno;
- return dest;
+
+ *path = dest;
+
+ return 0;
err_new_tmpfile:
@@ -410,7 +378,7 @@ err_new_tmpfile:
errno = saved_errno;
- return NULL;
+ return -1;
}
/* hardened TMPDIR parsing
@@ -712,7 +680,6 @@ mkhtemp(int *fd,
size_t xc = 0;
size_t fname_len = 0;
- char *template_copy = NULL;
char *fname_copy = NULL;
char *p;
@@ -726,6 +693,7 @@ mkhtemp(int *fd,
#else
size_t max_len = 4096;
#endif
+ int r;
if (fd == NULL ||
template == NULL ||
@@ -793,29 +761,21 @@ mkhtemp(int *fd,
return -1;
}
- template_copy = malloc(len + 1);
- if (template_copy == NULL) {
- errno = ENOMEM;
- goto err;
- }
-
fname_copy = malloc(fname_len + 1);
if (fname_copy == NULL) {
errno = ENOMEM;
goto err;
}
- memcpy(template_copy, template, len + 1);
-
+ /* fname_copy = suffix region only; p points to trailing XXXXXX */
memcpy(fname_copy,
- template_copy + len - fname_len,
+ template + len - fname_len,
fname_len + 1);
-
p = fname_copy + fname_len - xc;
- for (retries = 0; retries < 300; retries++) {
+ for (retries = 0; retries < MKHTEMP_RETRY_MAX; retries++) {
- int r = mkhtemp_try_create(
+ r = mkhtemp_try_create(
dirfd,
st_dir_initial,
fname_copy,
@@ -824,17 +784,16 @@ mkhtemp(int *fd,
fd,
st);
- if (r == 1) {
- /* success: copy final name back */
- memcpy(template + len - fname_len,
- fname_copy, fname_len);
+ /* DoS mitigation; add jitter delay (0–1023 µs) */
+ if (r != 1 && retries >= MKHTEMP_SPIN_THRESHOLD)
+ usleep((useconds_t)rlong() & 0x3FF);
- errno = saved_errno;
- goto success;
- }
+ /* success: copy final name back */
+ memcpy(template + len - fname_len,
+ fname_copy, fname_len);
- if (r == -1)
- goto err;
+ errno = saved_errno;
+ goto success;
}
errno = EEXIST;
@@ -846,8 +805,6 @@ err:
}
success:
- if (template_copy != NULL)
- free(template_copy);
if (fname_copy != NULL)
free(fname_copy);
@@ -865,6 +822,9 @@ mkhtemp_try_create(int dirfd,
struct stat *st)
{
struct stat st_dir_now;
+ struct stat st_open;
+ int saved_errno = errno;
+ int close_errno;
if (mkhtemp_fill_random(p, xc) < 0)
return -1;
@@ -885,15 +845,35 @@ mkhtemp_try_create(int dirfd,
if (*fd >= 0) {
- if (secure_file(fd, st, O_APPEND, 1, 1, 0600) < 0)
+ if (fstat(*fd, &st_open) < 0)
+ return -1;
+
+ if (fstat(dirfd, &st_dir_now) < 0)
+ return -1;
+
+ if (st_dir_now.st_dev != st_dir_initial->st_dev ||
+ st_dir_now.st_ino != st_dir_initial->st_ino) {
+
+ close_errno = errno;
+ (void) close_on_eintr(*fd);
+ errno = close_errno;
+
+ errno = ESTALE;
return -1;
+ }
+ if (secure_file(fd, st, &st_open, O_APPEND, 1, 1, 0600) < 0)
+ return -1;
+
+ errno = saved_errno;
return 1; /* success */
}
if (errno == EEXIST ||
errno == EINTR ||
- errno == EAGAIN)
+ errno == EAGAIN ||
+ errno == EWOULDBLOCK ||
+ errno == ETXTBSY)
return 0; /* retry */
return -1; /* fatal */
@@ -964,31 +944,16 @@ err_mkhtemp_fill_random:
return -1;
}
-/* why doesn't literally
- every libc have this?
-
- TODO: consider set_flags,
- complementing bad_flags,
- for setting new flags;
- with another option to
- set it before the bad_flags
- check, or after it (because
- some callers may be setting
- flags given to them with
- theirs OR'd in, yet still
- want to filter bad flags,
- whereas others may not want
- to modify flags on a file
- that already contains
- specific flags)
- */
-int
-secure_file(int *fd,
- struct stat *st, int bad_flags,
- int check_seek, int do_lock,
+int secure_file(int *fd,
+ struct stat *st,
+ struct stat *expected,
+ int bad_flags,
+ int check_seek,
+ int do_lock,
mode_t mode)
{
int flags;
+ struct stat st_now;
int saved_errno = errno;
if (fd == NULL) {
@@ -1012,16 +977,8 @@ secure_file(int *fd,
if (bad_flags > 0) {
- /*
- * For example:
- * O_APPEND would permit offsets
- * to be ignored, which breaks
- * positional read/write
- *
- * You might provide that as
- * the mask to this function,
- * before doing positional i/o
- */
+ /* e.g. O_APPEND breaks pwrite/pread
+ * by allowing offsets to be ignored */
if (flags & bad_flags) {
errno = EPERM;
goto err_secure_file;
@@ -1031,12 +988,20 @@ secure_file(int *fd,
if (fstat(*fd, st) == -1)
goto err_secure_file;
- /*
- * Extremely defensive
- * likely pointless checks
- */
+ /* verify inode/device stability */
+ if (expected != NULL) {
+
+ if (st_now.st_dev != expected->st_dev ||
+ st_now.st_ino != expected->st_ino) {
+
+ errno = ESTALE;
+ goto err_secure_file;
+ }
+ }
+
+ /* now copy to caller */
+ *st = st_now;
- /* check if it's a file */
if (!S_ISREG(st->st_mode)) {
errno = EBADF;
goto err_secure_file;
@@ -1044,23 +1009,17 @@ secure_file(int *fd,
if (check_seek) {
- /* check if it's seekable
- */
+ /* check if it's seekable */
if (lseek(*fd, 0, SEEK_CUR) == (off_t)-1)
goto err_secure_file;
}
- /* tmpfile re-linked, or
- unlinked, while opened
- */
+ /* tmpfile re/un linked while open */
if (st->st_nlink != 1) {
errno = ELOOP;
goto err_secure_file;
}
- /* block if we don't own the file
- * (exception made for root)
- */
if (st->st_uid != geteuid() && /* someone else's file */
geteuid() != 0) { /* override for root */
diff --git a/util/nvmutil/lib/io.c b/util/nvmutil/lib/io.c
index e95be73c..94bde87e 100644
--- a/util/nvmutil/lib/io.c
+++ b/util/nvmutil/lib/io.c
@@ -473,7 +473,8 @@ gbe_mv(void)
/* create replacement temp in target directory
*/
- dest_tmp = new_tmpfile(&dest_fd);
+ if (new_tmpfile(&dest_fd, &f->fname) < 1)
+ goto ret_gbe_mv;
if (dest_tmp == NULL)
goto ret_gbe_mv;
diff --git a/util/nvmutil/lib/state.c b/util/nvmutil/lib/state.c
index 836c6bc7..19d5cd8c 100644
--- a/util/nvmutil/lib/state.c
+++ b/util/nvmutil/lib/state.c
@@ -112,7 +112,8 @@ xstart(int argc, char *argv[])
us.argv0 = argv[0];
us.f.fname = argv[1];
- us.f.tname = new_tmpfile(&us.f.tmp_fd);
+ if (new_tmpfile(&us.f.tmp_fd, &us.f.tname) < 0)
+ err_no_cleanup(errno, "xstart: cannot create tmpfile");
/* parse user command */
/* TODO: CHECK ACCESSES VIA xstatus() */