Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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@redhat.com>
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

authored by

Vasily Averin and committed by
J. Bruce Fields
81833de1 9e137ed5

+9 -4
+5 -1
fs/nfs_common/grace.c
··· 30 30 struct list_head *grace_list = net_generic(net, grace_net_id); 31 31 32 32 spin_lock(&grace_lock); 33 - list_add(&lm->list, grace_list); 33 + if (list_empty(&lm->list)) 34 + list_add(&lm->list, grace_list); 35 + else 36 + WARN(1, "double list_add attempt detected in net %x %s\n", 37 + net->ns.inum, (net == &init_net) ? "(init_net)" : ""); 34 38 spin_unlock(&grace_lock); 35 39 } 36 40 EXPORT_SYMBOL_GPL(locks_start_grace);
+4 -3
fs/nfsd/nfs4state.c
··· 7103 7103 INIT_LIST_HEAD(&nn->sessionid_hashtbl[i]); 7104 7104 nn->conf_name_tree = RB_ROOT; 7105 7105 nn->unconf_name_tree = RB_ROOT; 7106 + nn->boot_time = get_seconds(); 7107 + nn->grace_ended = false; 7108 + nn->nfsd4_manager.block_opens = true; 7109 + INIT_LIST_HEAD(&nn->nfsd4_manager.list); 7106 7110 INIT_LIST_HEAD(&nn->client_lru); 7107 7111 INIT_LIST_HEAD(&nn->close_lru); 7108 7112 INIT_LIST_HEAD(&nn->del_recall_lru); ··· 7164 7160 ret = nfs4_state_create_net(net); 7165 7161 if (ret) 7166 7162 return ret; 7167 - nn->boot_time = get_seconds(); 7168 - nn->grace_ended = false; 7169 - nn->nfsd4_manager.block_opens = true; 7170 7163 locks_start_grace(net, &nn->nfsd4_manager); 7171 7164 nfsd4_client_tracking_init(net); 7172 7165 printk(KERN_INFO "NFSD: starting %ld-second grace period (net %x)\n",