CVE-2021-43395: Improper Locking (CWE-667)
value: An issue was discovered in illumos before f859e7171bb5db34321e45585839c6c3200ebb90, OmniOS Community Edition r151038, OpenIndiana Hipster 2021.04, and SmartOS 20210923. A local unprivileged user can cause a deadlock and kernel panic via crafted rename and rmdir calls on tmpfs filesystems. Oracle Solaris 10 and 11 is also affected.
CWE description: The product does not properly acquire or release a lock on a resource, leading to unexpected resource state changes and behaviors.
CVSS v3 base score: 5.5
Published: 2022-12-26T06:15Z
Repository: https://github.com/illumos/illumos-gate
Language: C
Files changed: usr/src/uts/common/fs/tmpfs/tmp_dir.c
Commit date: 2022-01-18 12:11:37 -0500
Commit message: 14424 tmpfs can be induced to deadlock
Reviewed by: Robert Mustacchi <rm@fingolfin.org>
Reviewed by: Andy Fiddaman <andy@omnios.org>
Reviewed by: Mike Zeller <mike.zeller@joyent.com>
Approved by: Robert Mustacchi <rm@fingolfin.org>

Diff: diff --git a/usr/src/uts/common/fs/tmpfs/tmp_dir.c b/usr/src/uts/common/fs/tmpfs/tmp_dir.c
index f6621c8097b..06ef8dd7fd8 100644
--- a/usr/src/uts/common/fs/tmpfs/tmp_dir.c
+++ b/usr/src/uts/common/fs/tmpfs/tmp_dir.c
@@ -53,10 +53,15 @@ static int tdiraddentry(struct tmpnode *, struct tmpnode *, char *,
 
 
 #define	T_HASH_SIZE	8192		/* must be power of 2 */
 #define	T_MUTEX_SIZE	64
 
+/* Non-static so compilers won't constant-fold these away. */
+clock_t tmpfs_rename_backoff_delay = 1;
+unsigned int tmpfs_rename_backoff_tries = 0;
+unsigned long tmpfs_rename_loops = 0;
+
 static struct tdirent	*t_hashtable[T_HASH_SIZE];
 static kmutex_t		 t_hashmutex[T_MUTEX_SIZE];
 
 #define	T_HASH_INDEX(a)		((a) & (T_HASH_SIZE-1))
 #define	T_MUTEX_INDEX(a)	((a) & (T_MUTEX_SIZE-1))
@@ -265,12 +270,69 @@ tdirenter(
 	/*
 	 * For link and rename lock the source entry and check the link count
 	 * to see if it has been removed while it was unlocked.
 	 */
 	if (op == DE_LINK || op == DE_RENAME) {
-		if (tp != dir)
-			rw_enter(&tp->tn_rwlock, RW_WRITER);
+		if (tp != dir) {
+			unsigned int tries = 0;
+
+			/*
+			 * If we are acquiring tp->tn_rwlock (for SOURCE)
+			 * inside here, we must consider the following:
+			 *
+			 * - dir->tn_rwlock (TARGET) is already HELD (see
+			 * above ASSERT()).
+			 *
+			 * - It is possible our SOURCE is a parent of our
+			 * TARGET. Yes it's unusual, but it will return an
+			 * error below via tdircheckpath().
+			 *
+			 * - It is also possible that another thread,
+			 * concurrent to this one, is performing
+			 * rmdir(TARGET), which means it will first acquire
+			 * SOURCE's lock, THEN acquire TARGET's lock, which
+			 * could result in this thread holding TARGET and
+			 * trying for SOURCE, but the other thread holding
+			 * SOURCE and trying for TARGET.  This is deadlock,
+			 * and it's inducible.
+			 *
+			 * To prevent this, we borrow some techniques from UFS
+			 * and rw_tryenter(), delaying if we fail, and
+			 * if someone tweaks the number of backoff tries to be
+			 * nonzero, return EBUSY after that number of tries.
+			 */
+			while (!rw_tryenter(&tp->tn_rwlock, RW_WRITER)) {
+				/*
+				 * Sloppy, but this is a diagnostic so atomic
+				 * increment would be overkill.
+				 */
+				tmpfs_rename_loops++;
+
+				if (tmpfs_rename_backoff_tries != 0) {
+					if (tries > tmpfs_rename_backoff_tries)
+						return (EBUSY);
+					tries++;
+				}
+				/*
+				 * NOTE: We're still holding dir->tn_rwlock,
+				 * so drop it over the delay, so any other
+				 * thread can get its business done.
+				 *
+				 * No state change or state inspection happens
+				 * prior to here, so it is not wholly dangerous
+				 * to release-and-reacquire dir->tn_rwlock.
+				 *
+				 * Hold the vnode of dir in case it gets
+				 * released by another thread, though.
+				 */
+				VN_HOLD(TNTOV(dir));
+				rw_exit(&dir->tn_rwlock);
+				delay(tmpfs_rename_backoff_delay);
+				rw_enter(&dir->tn_rwlock, RW_WRITER);
+				VN_RELE(TNTOV(dir));
+			}
+		}
 		mutex_enter(&tp->tn_tlock);
 		if (tp->tn_nlink == 0) {
 			mutex_exit(&tp->tn_tlock);
 			if (tp != dir)
 				rw_exit(&tp->tn_rwlock);

Vulnerable code: 		if (tp != dir)
			rw_enter(&tp->tn_rwlock, RW_WRITER);
Fixed code: /* Non-static so compilers won't constant-fold these away. */
clock_t tmpfs_rename_backoff_delay = 1;
unsigned int tmpfs_rename_backoff_tries = 0;
unsigned long tmpfs_rename_loops = 0;

		if (tp != dir) {
			unsigned int tries = 0;

			/*
			 * If we are acquiring tp->tn_rwlock (for SOURCE)
			 * inside here, we must consider the following:
			 *
			 * - dir->tn_rwlock (TARGET) is already HELD (see
			 * above ASSERT()).
			 *
			 * - It is possible our SOURCE is a parent of our
			 * TARGET. Yes it's unusual, but it will return an
			 * error below via tdircheckpath().
			 *
			 * - It is also possible that another thread,
			 * concurrent to this one, is performing
			 * rmdir(TARGET), which means it will first acquire
			 * SOURCE's lock, THEN acquire TARGET's lock, which
			 * could result in this thread holding TARGET and
			 * trying for SOURCE, but the other thread holding
			 * SOURCE and trying for TARGET.  This is deadlock,
			 * and it's inducible.
			 *
			 * To prevent this, we borrow some techniques from UFS
			 * and rw_tryenter(), delaying if we fail, and
			 * if someone tweaks the number of backoff tries to be
			 * nonzero, return EBUSY after that number of tries.
			 */
			while (!rw_tryenter(&tp->tn_rwlock, RW_WRITER)) {
				/*
				 * Sloppy, but this is a diagnostic so atomic
				 * increment would be overkill.
				 */
				tmpfs_rename_loops++;

				if (tmpfs_rename_backoff_tries != 0) {
					if (tries > tmpfs_rename_backoff_tries)
						return (EBUSY);
					tries++;
				}
				/*
				 * NOTE: We're still holding dir->tn_rwlock,
				 * so drop it over the delay, so any other
				 * thread can get its business done.
				 *
				 * No state change or state inspection happens
				 * prior to here, so it is not wholly dangerous
				 * to release-and-reacquire dir->tn_rwlock.
				 *
				 * Hold the vnode of dir in case it gets
				 * released by another thread, though.
				 */
				VN_HOLD(TNTOV(dir));
				rw_exit(&dir->tn_rwlock);
				delay(tmpfs_rename_backoff_delay);
				rw_enter(&dir->tn_rwlock, RW_WRITER);
				VN_RELE(TNTOV(dir));
			}
		}