diff options
author | Dan McDonald <danmcd@joyent.com> | 2021-11-04 21:55:12 -0400 |
---|---|---|
committer | Dan McDonald <danmcd@joyent.com> | 2022-01-18 13:32:03 -0500 |
commit | f05d1bde0cc10f872d8760237cdabb67bb35d771 (patch) | |
tree | 83d5b28b4ea4494f6a09ce5fe4210dafafded9d8 | |
parent | d3983e7f69d3e2990945f3c0b3777178ad3a331a (diff) | |
download | illumos-joyent-release-20220113.tar.gz |
14424 tmpfs can be induced to deadlockrelease-20220113
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>
-rw-r--r-- | usr/src/uts/common/fs/tmpfs/tmp_dir.c | 66 |
1 files changed, 64 insertions, 2 deletions
diff --git a/usr/src/uts/common/fs/tmpfs/tmp_dir.c b/usr/src/uts/common/fs/tmpfs/tmp_dir.c index 1a620642cc..d689c512c1 100644 --- a/usr/src/uts/common/fs/tmpfs/tmp_dir.c +++ b/usr/src/uts/common/fs/tmpfs/tmp_dir.c @@ -54,6 +54,11 @@ 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]; @@ -266,8 +271,65 @@ tdirenter( * 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); |