Return-Path: Received: from mail.kernel.org ([198.145.29.99]:51508 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753245AbdJTRDl (ORCPT ); Fri, 20 Oct 2017 13:03:41 -0400 Message-ID: <1508519017.5572.46.camel@kernel.org> Subject: Re: [PATCH v2] lockd: double unregister of inetaddr notifiers From: Jeff Layton To: Vasily Averin , linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Trond Myklebust , Anna Schumaker , "J. Bruce Fields" , Scott Mayhew , stable@vger.kernel.org Date: Fri, 20 Oct 2017 13:03:37 -0400 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Fri, 2017-10-20 at 17:33 +0300, Vasily Averin wrote: > v2: reported to stable@ because it fixes backported patch. > > lockd_up() can call lockd_unregister_notifiers twice: > inside lockd_start_svc() when it calls lockd_svc_exit_thread() > and then in error path of lockd_up() > > Patch forces lockd_start_svc() to unregister notifiers in all error cases > and removes extra unregister in error path of lockd_up(). > > Fixes: cb7d224f82e4 "lockd: unregister notifier blocks if the service ..." > Signed-off-by: Vasily Averin > --- > fs/lockd/svc.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c > index b995bdc..f04ecfc 100644 > --- a/fs/lockd/svc.c > +++ b/fs/lockd/svc.c > @@ -369,6 +369,7 @@ static int lockd_start_svc(struct svc_serv *serv) > printk(KERN_WARNING > "lockd_up: svc_rqst allocation failed, error=%d\n", > error); > + lockd_unregister_notifiers(); > goto out_rqst; > } > > @@ -459,13 +460,16 @@ int lockd_up(struct net *net) > } > > error = lockd_up_net(serv, net); > - if (error < 0) > - goto err_net; > + if (error < 0) { > + lockd_unregister_notifiers(); > + goto err_put; > + } > > error = lockd_start_svc(serv); > - if (error < 0) > - goto err_start; > - > + if (error < 0) { > + lockd_down_net(serv, net); > + goto err_put; > + } > nlmsvc_users++; > /* > * Note: svc_serv structures have an initial use count of 1, > @@ -476,12 +480,6 @@ int lockd_up(struct net *net) > err_create: > mutex_unlock(&nlmsvc_mutex); > return error; > - > -err_start: > - lockd_down_net(serv, net); > -err_net: > - lockd_unregister_notifiers(); > - goto err_put; > } > EXPORT_SYMBOL_GPL(lockd_up); > I think this looks right. I really do hate the way that the notifier handling is sprinkled all over the place in this code (not a comment on your patch so much as the lockd code in general). I don't see a better way to do it right offhand though. Reviewed-by: Jeff Layton