From 9d29e96c3bb82db6d45928a81a39f70b3f30c669 Mon Sep 17 00:00:00 2001
From: Kartik Agaram <ak@akkartik.com>
Date: Tue, 17 Apr 2018 21:37:49 -0700
Subject: [PATCH] Fix open(2) allowing opening directories invalidly and check
 O_TRUNC errors.

Among other things, redirecting to a directory will now display an error
as it should.

Also fix a bug when opening /dev/pts: O_WRITE on a directory is a POSIX
violation.
---
 kernel/descriptor.cpp | 28 ++++++++++++++++++++--------
 kernel/kernel.cpp     |  2 +-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/kernel/descriptor.cpp b/kernel/descriptor.cpp
index cb7a3f64..e9a354ca 100644
--- a/kernel/descriptor.cpp
+++ b/kernel/descriptor.cpp
@@ -645,7 +645,11 @@ static bool IsSaneFlagModeCombination(int flags, mode_t /*mode*/)
 	// opening a directory, attempting to truncate it, and then aborting with an
 	// error because a directory was opened.
 	if ( (flags & (O_CREATE | O_TRUNC)) && (flags & (O_DIRECTORY)) )
-		return errno = EINVAL, false;
+		return false;
+	// POSIX: "The result of using O_TRUNC without either O_RDWR or
+	// O_WRONLY is undefined."
+	if ( (flags & O_TRUNC) && !(flags & O_WRITE) )
+		return false;
 	return true;
 }
 
@@ -770,6 +774,21 @@ Ref<Descriptor> Descriptor::open(ioctx_t* ctx, const char* filename, int flags,
 
 	delete[] filename_mine;
 
+	// Abort if the user tries to write to an existing directory.
+	if ( S_ISDIR(desc->type) )
+	{
+		if ( flags & (O_CREATE | O_TRUNC | O_WRITE) )
+			return errno = EISDIR, Ref<Descriptor>();
+	}
+
+	// Truncate the file if requested.
+	if ( (flags & O_TRUNC) && S_ISREG(desc->type) )
+	{
+		assert(flags & O_WRITE); // IsSaneFlagModeCombination
+		if ( desc->truncate(ctx, 0) < 0 )
+			return Ref<Descriptor>();
+	}
+
 	// Abort the open if the user wanted a directory but this wasn't.
 	if ( flags & O_DIRECTORY && !S_ISDIR(desc->type) )
 		return errno = ENOTDIR, Ref<Descriptor>();
@@ -802,13 +821,6 @@ Ref<Descriptor> Descriptor::open_elem(ioctx_t* ctx, const char* filename,
 	if ( !ret )
 		return Ref<Descriptor>();
 
-	// Truncate the file if requested.
-	// TODO: This is a bit dodgy, should this be moved to the inode open method
-	//       or something? And how should error handling be done here?
-	if ( (flags & O_TRUNC) && S_ISREG(ret->type) )
-		if ( flags & O_WRITE )
-			ret->truncate(ctx, 0);
-
 	return ret;
 }
 
diff --git a/kernel/kernel.cpp b/kernel/kernel.cpp
index 670b09c5..652c6f1d 100644
--- a/kernel/kernel.cpp
+++ b/kernel/kernel.cpp
@@ -520,7 +520,7 @@ static void BootThread(void* /*user*/)
 	if ( slashdev->mkdir(&ctx, "pts", 0755) < 0 )
 		Panic("Could not mkdir /dev/pts");
 	Ref<Descriptor> ptsdir =
-		slashdev->open(&ctx, "pts", O_DIRECTORY | O_READ | O_WRITE);
+		slashdev->open(&ctx, "pts", O_DIRECTORY | O_READ);
 	if ( !ptsdir )
 		Panic("Could not open /dev/pts");
 	if ( !mtable->AddMount(ptsdir->ino, ptsdir->dev, pts, true) )