bhyveload: simplify cb_open() and eliminate minor TOCTOU

It's not at all clear why I wrote it like this, but we can do better.

I wouldn't think this really has any meaningful security implications
since the hierarchy in question can't really be modified by the guest
scripts, but it would seem to make it a little more robust.

Fixes:	6779d44bd878e3c ("bhyveload: use a dirfd to support -h")
Reviewed by:	bnovkov, markj
Differential Revision:	https://reviews.freebsd.org/D55379

1 files changed, 12 insertions, 14 deletions

diff --git a/usr.sbin/bhyveload/bhyveload.c b/usr.sbin/bhyveload/bhyveload.c
index 2b1bfec9a62d..4cc566f334c3 100644
--- a/usr.sbin/bhyveload/bhyveload.c
+++ b/usr.sbin/bhyveload/bhyveload.c
@@ -169,11 +169,9 @@ cb_open(void *arg __unused, const char *filename, void **hp)
 {
 	struct cb_file *cf;
 	struct stat sb;
-	int fd, flags;
+	int fd;
 
 	cf = NULL;
-	fd = -1;
-	flags = O_RDONLY | O_RESOLVE_BENEATH;
 	if (hostbase_fd == -1)
 		return (ENOENT);
 
@@ -185,20 +183,21 @@ cb_open(void *arg __unused, const char *filename, void **hp)
 	if (filename[0] == '\0')
 		filename = ".";
 
-	if (fstatat(hostbase_fd, filename, &sb, AT_RESOLVE_BENEATH) < 0)
-		return (errno);
-
-	if (!S_ISDIR(sb.st_mode) && !S_ISREG(sb.st_mode))
-		return (EINVAL);
-
-	if (S_ISDIR(sb.st_mode))
-		flags |= O_DIRECTORY;
-
 	/* May be opening the root dir */
-	fd = openat(hostbase_fd, filename, flags);
+	fd = openat(hostbase_fd, filename, O_RDONLY | O_RESOLVE_BENEATH);
 	if (fd < 0)
 		return (errno);
 
+	if (fstat(fd, &sb) < 0) {
+		int serrno = errno;
+
+		close(fd);
+		return (serrno);
+	} else if (!S_ISDIR(sb.st_mode) && !S_ISREG(sb.st_mode)) {
+		close(fd);
+		return (EINVAL);
+	}
+
 	cf = malloc(sizeof(struct cb_file));
 	if (cf == NULL) {
 		close(fd);
@@ -217,7 +216,6 @@ cb_open(void *arg __unused, const char *filename, void **hp)
 			return (ENOMEM);
 		}
 	} else {
-		assert(S_ISREG(cf->cf_stat.st_mode));
 		cf->cf_isdir = 0;
 		cf->cf_u.fd = fd;
 	}