[Devel] [PATCH RHEL7 COMMIT] fuse kio: Hold pcs_rpc counter till sio may be freed

Konstantin Khorenko khorenko at virtuozzo.com
Tue Oct 16 18:09:45 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-862.14.4.vz7.72.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.14.4.vz7.72.9
------>
commit 455a8eaf76f3c0b844e24d50d5ab3111dd7b0cd9
Author: Kirill Tkhai <ktkhai at virtuozzo.com>
Date:   Tue Oct 16 18:09:43 2018 +0300

    fuse kio: Hold pcs_rpc counter till sio may be freed
    
    Socket callbacks dereference sio->parent, so pcs_rps
    mustn't be destroyed before RCU grace period end.
    Thus, the patch makes the counter be put
    in sio_destroy_rcu().
    
    Also, rework waiting for all pcs_rps are destroyed in
    pcs_rpc_engine_fini() (wait for ep->eng->nrpcs == 0).
    Note, that flush_delayed_work(&ep->eng->stat_work)
    is removed from pcs_rpc_destroy() to use nrpcs as
    such the identifier (now it's not allowed to touch
    ep->eng after nrpcs == 0 since pcs_rpc_engine_fini()
    waits only this, and umount code may already free eng).
    We don't want introduce counter for eng instead of this,
    since this will be overkill.
    
    Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
    Reviewed-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
    
    =====================
    Patchset description:
    
    Order pcs_rpc and pcs_sockio destruction and close leaked socket
    
    https://pmc.acronis.com/browse/VSTOR-15305
    
    Ploop can asynchronously unmap regions by sending IOCB_CMD_UNMAP_ITER, but this
    command isn't quite correctly interpreted in Fuse. Moreover, in Fast-path mode,
    fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_ZERO_RANGE) falls to fuse user daemon
    and it can lead to data corruption.
    
    Let's fix it.
    
    Kirill Tkhai (9):
          fuse kio: Use __maybe_unused
          fuse kio: Use sio eof instead of parent to determ abort
          fuse kio: Reorder callback assignment
          fuse kio: Add pcs_cleanup_wq
          fuse kio: Destroy rpc in work func
          fuse kio: Introduce pcs_sk_kick_queue()
          fuse kio: Dereference sk_user_data under rcu
          fuse kio: Fix rpc socket leak on rpc_abort()
          fuse kio: Hold pcs_rpc counter till sio may be freed
---
 fs/fuse/kio/pcs/pcs_rpc.c     | 29 ++++++++++++++---------------
 fs/fuse/kio/pcs/pcs_sock_io.c |  2 ++
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_rpc.c b/fs/fuse/kio/pcs/pcs_rpc.c
index 7571523fe271..7ef69a842ce8 100644
--- a/fs/fuse/kio/pcs/pcs_rpc.c
+++ b/fs/fuse/kio/pcs/pcs_rpc.c
@@ -38,6 +38,8 @@ static unsigned long rpc_cpu_time_slice = PCS_RPC_CPU_SLICE;
 module_param(rpc_cpu_time_slice, ulong, 0644);
 MODULE_PARM_DESC(rpc_cpu_time_slice, "Time slice for RPC rebinding");
 
+DECLARE_WAIT_QUEUE_HEAD(pcs_waitq);
+
 static void timer_work(struct work_struct *w);
 static int rpc_gc_classify(struct pcs_rpc * ep);
 
@@ -300,7 +302,7 @@ void pcs_rpc_attach_new_ep(struct pcs_rpc * ep, struct pcs_rpc_engine * eng)
 
 static void pcs_rpc_destroy(struct pcs_rpc *ep)
 {
-	bool flush;
+	bool last_ep;
 	BUG_ON(ep->state != PCS_RPC_DESTROY);
 	BUG_ON(ep->flags & PCS_RPC_F_HASHED);
 	BUG_ON(!(ep->flags & PCS_RPC_F_DEAD));
@@ -309,6 +311,8 @@ static void pcs_rpc_destroy(struct pcs_rpc *ep)
 	BUG_ON(!list_empty(&ep->pending_queue));
 	BUG_ON(timer_pending(&ep->timer_work.timer));
 
+	cancel_delayed_work_sync(&ep->calendar_work);
+
 	/* pcs_free(ep->sun); */
 	/* ep->sun = NULL; */
 	if (ep->gc)
@@ -319,12 +323,11 @@ static void pcs_rpc_destroy(struct pcs_rpc *ep)
 	 */
 	spin_lock(&ep->eng->lock);
 	hlist_del(&ep->link);
-	flush = !(--ep->eng->nrpcs);
+	last_ep = (!--ep->eng->nrpcs);
 	spin_unlock(&ep->eng->lock);
 
-	cancel_delayed_work_sync(&ep->calendar_work);
-	if (flush)
-		flush_delayed_work(&ep->eng->stat_work);
+	if (last_ep)
+		wake_up_all(&pcs_waitq);
 
 	memset(ep, 0xFF, sizeof(*ep));
 	kfree(ep);
@@ -1308,18 +1311,13 @@ void pcs_rpc_engine_init(struct pcs_rpc_engine * eng, u8 role)
 static void pcs_rpc_fini_verify(struct pcs_rpc_engine *eng, struct hlist_head *rpc_list)
 {
 	spin_lock(&eng->lock);
-	while (!hlist_empty(rpc_list)) {
+	if (!hlist_empty(rpc_list)) {
 		struct pcs_rpc * ep =
 			hlist_entry(rpc_list->first, struct pcs_rpc, link);
 
-		pr_warn("rpc connection isn't closed ep: %p (flags: %u, "
+		WARN(1, "rpc connection isn't closed ep: %p (flags: %u, "
 			"state: %u, refcnt: %u)\n", ep, ep->flags, ep->state,
 			atomic_read(&ep->refcnt));
-		WARN_ON(!(ep->flags & PCS_RPC_F_DEAD));
-		WARN_ON(atomic_read(&ep->refcnt) <= 0);
-		spin_unlock(&eng->lock);
-		schedule_timeout(5 * HZ);
-		spin_lock(&eng->lock);
 	}
 	spin_unlock(&eng->lock);
 }
@@ -1328,8 +1326,9 @@ void pcs_rpc_engine_fini(struct pcs_rpc_engine * eng)
 {
 	unsigned int i;
 
-	if (cancel_delayed_work_sync(&eng->stat_work))
-		pr_err("fuse kio: stat_work was actually canceled\n");
+	wait_event(pcs_waitq, (eng->nrpcs == 0));
+
+	cancel_delayed_work_sync(&eng->stat_work);
 
 	for (i = 0; i < PCS_RPC_HASH_SIZE; i++)
 		pcs_rpc_fini_verify(eng, &eng->ht[i]);
@@ -1419,7 +1418,7 @@ void rpc_connect_done(struct pcs_rpc *ep, struct socket *sock)
 		BUG();
 
 	ep->conn = &sio->ioconn;
-	sio->parent = ep;
+	sio->parent = pcs_rpc_get(ep);
 	sio->get_msg = rpc_get_hdr;
 	sio->eof = rpc_eof_cb;
 	//pcs_ioconn_register(ep->conn);
diff --git a/fs/fuse/kio/pcs/pcs_sock_io.c b/fs/fuse/kio/pcs/pcs_sock_io.c
index 68829daaa00e..04b63b58dad6 100644
--- a/fs/fuse/kio/pcs/pcs_sock_io.c
+++ b/fs/fuse/kio/pcs/pcs_sock_io.c
@@ -485,7 +485,9 @@ static void pcs_restore_sockets(struct pcs_ioconn *ioconn)
 static void sio_destroy_rcu(struct rcu_head *head)
 {
 	struct pcs_sockio *sio = container_of(head, struct pcs_sockio, rcu);
+	struct pcs_rpc *ep = sio->parent;
 
+	pcs_rpc_put(ep);
 	memset(sio, 0xFF, sizeof(*sio));
 	kfree(sio);
 }



More information about the Devel mailing list