On Mon, 2017-11-13 at 07:25 +0300, Vasily Averin wrote: > 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. > > 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@xxxxxxxxxx> > Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx> > --- > 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 bd3e2d3..5be08f0 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); > } I'm not sure that warning really means much. It's not _really_ a bug to request that a new grace period start while it's already in one. In general, it's ok to request a new grace period while it's currently enforcing one. That should just have the effect of extending the existing grace period. > EXPORT_SYMBOL_GPL(locks_start_grace); > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 7345143..b29b5a1 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -7103,6 +7103,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); > @@ -7160,9 +7164,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 %x)\n", -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
