[Devel] [PATCH RHEL7 COMMIT] ms/lockd: fix "list_add double add" caused by legacy signal interface

Konstantin Khorenko khorenko at virtuozzo.com
Wed Jul 4 18:44:16 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-862.3.2.vz7.61.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.3.2.vz7.61.12
------>
commit 257a1a1b664ce20ed8a36e14ae09f9174e82de48
Author: Vasily Averin <vvs at virtuozzo.com>
Date:   Mon Nov 13 07:25:40 2017 +0300

    ms/lockd: fix "list_add double add" caused by legacy signal interface
    
    restart_grace() uses hardcoded init_net.
    It can cause to "list_add double add" in following scenario:
    
    1) nfsd and lockd was started in several net namespaces
    2) nfsd in init_net was stopped (lockd was not stopped because
     it have users from another net namespaces)
    3) lockd got signal, called restart_grace() -> set_grace_period()
     and enabled lock_manager in hardcoded init_net.
    4) nfsd in init_net is started again,
     its lockd_up() calls set_grace_period() and tries to add
     lock_manager into init_net 2nd time.
    
    Jeff Layton suggest:
    "Make it safe to call locks_start_grace multiple times on the same
    lock_manager. If it's already on the global grace_list, then don't try
    to add it again.  (But we don't intentionally add twice, so for now we
    WARN about that case.)
    
    With this change, we also need to ensure that the nfsd4 lock manager
    initializes the list before we call locks_start_grace. While we're at
    it, move the rest of the nfsd_net initialization into
    nfs4_state_create_net. I see no reason to have it spread over two
    functions like it is today."
    
    Suggested patch was updated to generate warning in described situation.
    
    Suggested-by: Jeff Layton <jlayton at redhat.com>
    Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
    Signed-off-by: J. Bruce Fields <bfields at redhat.com>
    
    This patch helps to detect problems like we had in
    https://jira.sw.ru/browse/PSBM-86390
    
    (cherry picked from commit 81833de1a46edce9ca20cfe079872ac1c20ef359)
    Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
---
 fs/nfs_common/grace.c | 6 +++++-
 fs/nfsd/nfs4state.c   | 7 ++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/nfs_common/grace.c b/fs/nfs_common/grace.c
index fd8c9a5bcac4..da03f591eaa7 100644
--- a/fs/nfs_common/grace.c
+++ b/fs/nfs_common/grace.c
@@ -30,7 +30,11 @@ locks_start_grace(struct net *net, struct lock_manager *lm)
 	struct list_head *grace_list = net_generic(net, grace_net_id);
 
 	spin_lock(&grace_lock);
-	list_add(&lm->list, grace_list);
+	if (list_empty(&lm->list))
+		list_add(&lm->list, grace_list);
+	else
+		WARN(1, "double list_add attempt detected in net %x %s\n",
+		     net->ns.inum, (net == &init_net) ? "(init_net)" : "");
 	spin_unlock(&grace_lock);
 }
 EXPORT_SYMBOL_GPL(locks_start_grace);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 72608d95b071..21dfa1b3b818 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6927,6 +6927,10 @@ static int nfs4_state_create_net(struct net *net)
 		INIT_LIST_HEAD(&nn->sessionid_hashtbl[i]);
 	nn->conf_name_tree = RB_ROOT;
 	nn->unconf_name_tree = RB_ROOT;
+	nn->boot_time = get_seconds();
+	nn->grace_ended = false;
+	nn->nfsd4_manager.block_opens = true;
+	INIT_LIST_HEAD(&nn->nfsd4_manager.list);
 	INIT_LIST_HEAD(&nn->client_lru);
 	INIT_LIST_HEAD(&nn->close_lru);
 	INIT_LIST_HEAD(&nn->del_recall_lru);
@@ -6984,9 +6988,6 @@ nfs4_state_start_net(struct net *net)
 	ret = nfs4_state_create_net(net);
 	if (ret)
 		return ret;
-	nn->boot_time = get_seconds();
-	nn->grace_ended = false;
-	nn->nfsd4_manager.block_opens = true;
 	locks_start_grace(net, &nn->nfsd4_manager);
 	nfsd4_client_tracking_init(net);
 	printk(KERN_INFO "NFSD: starting %ld-second grace period (net %p)\n",


More information about the Devel mailing list