Merge pull request #351 from abhinavagarwal07/fix-conntab-null-deref

fix conntab lookup after rename in sshfs_release

Author: h4sh5

sshfs.c

@@ -280,6 +280,11 @@ struct read_chunk {
 	struct sshfs_io sio;
 };
 
+struct conntab_entry {
+	unsigned refcount;
+	struct conn *conn;
+};
+
 struct sshfs_file {
 	struct buffer handle;
 	struct list_head write_reqs;
@@ -289,15 +294,11 @@ struct sshfs_file {
 	off_t next_pos;
 	int is_seq;
 	struct conn *conn;
+	struct conntab_entry *ce;
 	int connver;
 	int modifver;
 };
 
-struct conntab_entry {
-	unsigned refcount;
-	struct conn *conn;
-};
-
 struct sshfs {
 	char *directport;
 	char *ssh_command;
@@ -2768,6 +2769,12 @@ static int sshfs_utimens(const char *path, const struct timespec tv[2],
 	return err;
 }
 
+static gboolean conntab_entry_is(gpointer key, gpointer value, gpointer data)
+{
+	(void) key;
+	return value == data;
+}
+
 static int sshfs_open_common(const char *path, mode_t mode,
                              struct fuse_file_info *fi)
 {
@@ -2828,12 +2835,13 @@ static int sshfs_open_common(const char *path, mode_t mode,
 			g_hash_table_insert(sshfs.conntab, g_strdup(path), ce);
 		}
 		sf->conn = ce->conn;
+		sf->ce = ce;
 		ce->refcount++;
 		sf->conn->file_count++;
 		assert(sf->conn->file_count > 0);
 	} else {
 		sf->conn = &sshfs.conns[0];
-		ce = NULL; // only to silence compiler warning
+		sf->ce = NULL;
 	}
 	sf->connver = sf->conn->connver;
 	pthread_mutex_unlock(&sshfs.lock);
@@ -2873,10 +2881,12 @@ static int sshfs_open_common(const char *path, mode_t mode,
 		if (sshfs.max_conns > 1) {
 			pthread_mutex_lock(&sshfs.lock);
 			sf->conn->file_count--;
-			ce->refcount--;
-			if(ce->refcount == 0) {
-				g_hash_table_remove(sshfs.conntab, path);
-				g_free(ce);
+			sf->ce->refcount--;
+			if (sf->ce->refcount == 0) {
+				g_hash_table_foreach_remove(sshfs.conntab,
+							    conntab_entry_is,
+							    sf->ce);
+				g_free(sf->ce);
 			}
 			pthread_mutex_unlock(&sshfs.lock);
 		}
@@ -2947,20 +2957,20 @@ static int sshfs_release(const char *path, struct fuse_file_info *fi)
 {
 	struct sshfs_file *sf = get_sshfs_file(fi);
 	struct buffer *handle = &sf->handle;
-	struct conntab_entry *ce;
 	if (sshfs_file_is_conn(sf)) {
 		sshfs_flush(path, fi);
 		sftp_request(sf->conn, SSH_FXP_CLOSE, handle, 0, NULL);
 	}
 	buf_free(handle);
 	chunk_put_locked(sf->readahead);
 	if (sshfs.max_conns > 1) {
+		struct conntab_entry *ce = sf->ce;
 		pthread_mutex_lock(&sshfs.lock);
 		sf->conn->file_count--;
-		ce = g_hash_table_lookup(sshfs.conntab, path);
 		ce->refcount--;
-		if(ce->refcount == 0) {
-			g_hash_table_remove(sshfs.conntab, path);
+		if (ce->refcount == 0) {
+			g_hash_table_foreach_remove(sshfs.conntab,
+						    conntab_entry_is, ce);
 			g_free(ce);
 		}
 		pthread_mutex_unlock(&sshfs.lock);

test/test_sshfs.py

@@ -168,6 +168,7 @@ def test_sshfs(
         tst_mkdir_exist(mnt_dir)
         tst_readdir_repeated(mnt_dir)
         tst_rename_sibling(mnt_dir)
+        tst_rename_open_release(mnt_dir)
     except Exception as exc:
         cleanup(mount_process, mnt_dir)
         raise exc
@@ -503,6 +504,23 @@ def tst_rename_sibling(mnt_dir):
     os.unlink(name_c)
 
 
+def tst_rename_open_release(mnt_dir):
+    src = pjoin(mnt_dir, name_generator())
+    dst = pjoin(mnt_dir, name_generator())
+
+    fd = os.open(src, os.O_CREAT | os.O_RDWR)
+    try:
+        os.write(fd, b"data")
+        os.rename(src, dst)
+    finally:
+        os.close(fd)
+
+    assert not os.path.exists(src)
+    with open(dst, "rb") as fh:
+        assert fh.read() == b"data"
+    os.unlink(dst)
+
+
 def tst_link(mnt_dir, cache_timeout):
     name1 = pjoin(mnt_dir, name_generator())
     name2 = pjoin(mnt_dir, name_generator())