[PATCH] lockd: unregister notifier blocks if the service fails to come up completely

[PATCH] lockd: unregister notifier blocks if the service fails to come up completely

[Scott Mayhew]

If the lockd service fails to start up then we need to be sure that the
notifier blocks are not registered, otherwise a subsequent start of the
service could cause the same notifier to be registered twice, leading to
soft lockups.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 fs/lockd/svc.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 154a107..7d8217a 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -335,12 +335,17 @@ static struct notifier_block lockd_inet6addr_notifier = {
 };
 #endif
 
-static void lockd_svc_exit_thread(void)
+static void lockd_unregister_notifiers(void)
 {
 	unregister_inetaddr_notifier(&lockd_inetaddr_notifier);
 #if IS_ENABLED(CONFIG_IPV6)
 	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
 #endif
+}
+
+static void lockd_svc_exit_thread(void)
+{
+	lockd_unregister_notifiers();
 	svc_exit_thread(nlmsvc_rqst);
 }
 
@@ -450,12 +455,16 @@ int lockd_up(struct net *net)
 	}
 
 	error = lockd_up_net(serv, net);
-	if (error < 0)
+	if (error < 0) {
+		lockd_unregister_notifiers();
 		goto err_net;
+	}
 
 	error = lockd_start_svc(serv);
-	if (error < 0)
+	if (error < 0) {
+		lockd_unregister_notifiers();
 		goto err_start;
+	}
 
 	nlmsvc_users++;
 	/*
-- 
2.4.11

Re: [PATCH] lockd: unregister notifier blocks if the service fails to come up completely

[J. Bruce Fields]

On Thu, Jun 30, 2016 at 10:39:32AM -0400, Scott Mayhew wrote:
> If the lockd service fails to start up then we need to be sure that the
> notifier blocks are not registered, otherwise a subsequent start of the
> service could cause the same notifier to be registered twice, leading to
> soft lockups.

Good catch, thanks!

I think the following (untested) should do the same job slightly more
concisely?

--b.

commit d2f017465698
Author: Scott Mayhew <smayhew@redhat.com>
Date:   Thu Jun 30 10:39:32 2016 -0400

    lockd: unregister notifier blocks if the service fails to come up completely
    
    If the lockd service fails to start up then we need to be sure that the
    notifier blocks are not registered, otherwise a subsequent start of the
    service could cause the same notifier to be registered twice, leading to
    soft lockups.
    
    Signed-off-by: Scott Mayhew <smayhew@redhat.com>
    Cc: stable@vger.kernel.org
    Fixes: 0751ddf77b6a "lockd: Register callbacks on the inetaddr_chain..."
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 154a107cd376..fc4084ef4736 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -335,12 +335,17 @@ static struct notifier_block lockd_inet6addr_notifier = {
 };
 #endif
 
-static void lockd_svc_exit_thread(void)
+static void lockd_unregister_notifiers(void)
 {
 	unregister_inetaddr_notifier(&lockd_inetaddr_notifier);
 #if IS_ENABLED(CONFIG_IPV6)
 	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
 #endif
+}
+
+static void lockd_svc_exit_thread(void)
+{
+	lockd_unregister_notifiers();
 	svc_exit_thread(nlmsvc_rqst);
 }
 
@@ -462,7 +467,7 @@ int lockd_up(struct net *net)
 	 * Note: svc_serv structures have an initial use count of 1,
 	 * so we exit through here on both success and failure.
 	 */
-err_net:
+err_put:
 	svc_destroy(serv);
 err_create:
 	mutex_unlock(&nlmsvc_mutex);
@@ -470,7 +475,9 @@ err_create:
 
 err_start:
 	lockd_down_net(serv, net);
-	goto err_net;
+err_net:
+	lockd_unregister_notifiers();
+	goto err_put;
 }
 EXPORT_SYMBOL_GPL(lockd_up);
 

Re: [PATCH] lockd: unregister notifier blocks if the service fails to come up completely

[Scott Mayhew]

On Thu, 30 Jun 2016, J. Bruce Fields wrote:

> On Thu, Jun 30, 2016 at 10:39:32AM -0400, Scott Mayhew wrote:
> > If the lockd service fails to start up then we need to be sure that the
> > notifier blocks are not registered, otherwise a subsequent start of the
> > service could cause the same notifier to be registered twice, leading to
> > soft lockups.
> 
> Good catch, thanks!
> 
> I think the following (untested) should do the same job slightly more
> concisely?

Yes, that works too.  Thanks.

-Scott
> 
> --b.
> 
> commit d2f017465698
> Author: Scott Mayhew <smayhew@redhat.com>
> Date:   Thu Jun 30 10:39:32 2016 -0400
> 
>     lockd: unregister notifier blocks if the service fails to come up completely
>     
>     If the lockd service fails to start up then we need to be sure that the
>     notifier blocks are not registered, otherwise a subsequent start of the
>     service could cause the same notifier to be registered twice, leading to
>     soft lockups.
>     
>     Signed-off-by: Scott Mayhew <smayhew@redhat.com>
>     Cc: stable@vger.kernel.org
>     Fixes: 0751ddf77b6a "lockd: Register callbacks on the inetaddr_chain..."
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index 154a107cd376..fc4084ef4736 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -335,12 +335,17 @@ static struct notifier_block lockd_inet6addr_notifier = {
>  };
>  #endif
>  
> -static void lockd_svc_exit_thread(void)
> +static void lockd_unregister_notifiers(void)
>  {
>  	unregister_inetaddr_notifier(&lockd_inetaddr_notifier);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
>  #endif
> +}
> +
> +static void lockd_svc_exit_thread(void)
> +{
> +	lockd_unregister_notifiers();
>  	svc_exit_thread(nlmsvc_rqst);
>  }
>  
> @@ -462,7 +467,7 @@ int lockd_up(struct net *net)
>  	 * Note: svc_serv structures have an initial use count of 1,
>  	 * so we exit through here on both success and failure.
>  	 */
> -err_net:
> +err_put:
>  	svc_destroy(serv);
>  err_create:
>  	mutex_unlock(&nlmsvc_mutex);
> @@ -470,7 +475,9 @@ err_create:
>  
>  err_start:
>  	lockd_down_net(serv, net);
> -	goto err_net;
> +err_net:
> +	lockd_unregister_notifiers();
> +	goto err_put;
>  }
>  EXPORT_SYMBOL_GPL(lockd_up);
>