[Devel] [PATCH RHEL7 COMMIT] fuse kio: Fix potential use after free

[Konstantin Khorenko]

The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.50.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-693.21.1.vz7.50.4
------>
commit f374a9cb41c9d7c11f233a890844ed3604fe92fd
Author: Kirill Tkhai <ktkhai at virtuozzo.com>
Date:   Mon Jun 4 23:24:14 2018 +0300

    fuse kio: Fix potential use after free
    
    pcs_map_notify_addr_change() must take rcu_read_lock()
    while dereferencing of cs_list->map. Otherwise, it can
    be freed in parallel.
    
    This patch adds rcu_read_lock() protection and __rcu
    midifier.
    
    Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
    
    =====================
    Patchset description:
    
    Fix deadlock during change of CS address
    
    This is not a complete patchset, but I meet the situation
    when it's necessary to change original logic in small way,
    so this is a request for comments.
    
    [1-5/7] are mostly preparations and fixes, so my question
    is about [6-7/7].
    
    1) Patch 6 changes order of actions: pcs_map_notify_addr_change()
    is called after assigning of rpc addr. Can we do that? As I
    understand this results in new maps are created with new address,
    while in pcs_map_notify_addr_change() we invalidate old ones.
    So, for me it seems there is no a problem.
    
    This is needed for possibility to unlock cs->lock in pcs_map_notify_addr_change().
    Theoretically, two pcs_cs_find_create() may happen in parallel,
    so we want they assign addr in the order they happen. Otherwise,
    the first one with the old addr_serno may overwrite the addr.
    
    2) Patch 7 uses the preparations from previous patches and
    makes pcs_map_notify_addr_change() to unlock cs->lock for a while.
    New elements are added to head of cs->map_list, so we skip
    them on iterations. But it seems, they must be correct because
    we already updated rpc addr in pcs_cs_find_create(). Is there
    a reason we can't do this?
    
    Kirill Tkhai (7):
          fuse kio: Introduce pcs_cs_list_of_cs_link()
          fuse kio: Fix potential use after free
          fuse kio: Fix possible use after free in cslist_destroy()
          fuse kio: Introduce pcs_cs::use_count instead of ::is_probing
          fuse kio: Wait till cs is unused in pcs_csset_fini()
          fuse kio: Change order around pcs_map_notify_addr_change()
          fuse kio: Fix fix deadlock during change CS address
---
 fs/fuse/kio/pcs/pcs_map.c | 13 ++++++++-----
 fs/fuse/kio/pcs/pcs_map.h |  3 ++-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
index 56b660849a80..ef4efa6cc13e 100644
--- a/fs/fuse/kio/pcs/pcs_map.c
+++ b/fs/fuse/kio/pcs/pcs_map.c
@@ -97,7 +97,7 @@ static void map_drop_cslist(struct pcs_map_entry * m)
 	if (m->cs_list == NULL)
 		return;
 
-	m->cs_list->map = NULL;
+	rcu_assign_pointer(m->cs_list->map, NULL);
 	/* Barrier here is only for sanity checks in cslist_destroy() */
 	smp_mb__before_atomic_dec();
 	cslist_put(m->cs_list);
@@ -616,6 +616,7 @@ static int urgent_whitelist(struct pcs_cs * cs)
 
 		cs_list = cs_link_to_cs_list(csl);
 
+		/* FIXME: do we need rcu here? */
 		if (cs_list->map == NULL)
 			continue;
 
@@ -697,6 +698,7 @@ void pcs_map_notify_addr_change(struct pcs_cs * cs)
 
 	cs_whitelist(cs, "addr update");
 
+	rcu_read_lock();
 	list_for_each_entry(csl, &cs->map_list, link) {
 		struct pcs_cs_list *cs_list;
 		struct pcs_map_entry *m;
@@ -704,7 +706,8 @@ void pcs_map_notify_addr_change(struct pcs_cs * cs)
 		if (csl->addr_serno == cs->addr_serno)
 			continue;
 		cs_list = cs_link_to_cs_list(csl);
-		if ((m = cs_list->map) == NULL)
+		m = rcu_dereference(cs_list->map);
+		if (!m)
 			continue;
 
 		spin_lock(&m->lock);
@@ -718,10 +721,10 @@ void pcs_map_notify_addr_change(struct pcs_cs * cs)
 		      MAP_ARGS(m), NODE_ARGS(cs->id));
 
 		map_remote_error_nolock(m, PCS_ERR_CSD_STALE_MAP, cs->id.val);
-	unlock:
+unlock:
 		spin_unlock(&m->lock);
-
 	}
+	rcu_read_unlock();
 }
 
 noinline static void pcs_ireq_queue_fail(struct list_head *queue, int error)
@@ -1045,7 +1048,7 @@ void pcs_map_complete(struct pcs_map_entry *m, struct pcs_ioc_getmap *omap)
 			transfer_sync_data(cs_list, m->cs_list);
 			map_drop_cslist(m);
 		}
-		cs_list->map = m;
+		rcu_assign_pointer(cs_list->map, m);
 		cs_list->version = m->version;
 		m->cs_list = cs_list;
 		cs_list = NULL;
diff --git a/fs/fuse/kio/pcs/pcs_map.h b/fs/fuse/kio/pcs/pcs_map.h
index c60e72b365d3..49a05ff625f4 100644
--- a/fs/fuse/kio/pcs/pcs_map.h
+++ b/fs/fuse/kio/pcs/pcs_map.h
@@ -81,7 +81,8 @@ struct pcs_cs_record
 
 struct pcs_cs_list
 {
-	struct pcs_map_entry	*map;
+	struct pcs_map_entry __rcu *map;		/* Currently modified under
+							   ::map->lock */
 	atomic_t		refcnt;
 	atomic_t		seq_read_in_flight;
 	int			read_index;		/* volatile read hint */