diff mbox

nfsd: Fix nfsd leaks sunrpc module references

Message ID 5689E3C9.1010708@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kinglong Mee Jan. 4, 2016, 3:15 a.m. UTC
Stefan Hajnoczi reports,
nfsd leaks 3 references to the sunrpc module here:

  # echo -n "asdf 1234" >/proc/fs/nfsd/portlist
  bash: echo: write error: Protocol not supported

Now stop nfsd and try unloading the kernel modules:

  # systemctl stop nfs-server
  # systemctl stop nfs
  # systemctl stop proc-fs-nfsd.mount
  # systemctl stop var-lib-nfs-rpc_pipefs.mount
  # rmmod nfsd
  # rmmod nfs_acl
  # rmmod lockd
  # rmmod auth_rpcgss
  # rmmod sunrpc
  rmmod: ERROR: Module sunrpc is in use
  # lsmod | grep rpc
  sunrpc                315392  3

It is caused by nfsd don't cleanup rpcb program for nfsd
when destroying svc service after creating xprt fail.

Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
 fs/nfsd/nfssvc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Stefan Hajnoczi Jan. 4, 2016, 9:03 a.m. UTC | #1
On Mon, Jan 04, 2016 at 11:15:21AM +0800, Kinglong Mee wrote:
> Stefan Hajnoczi reports,
> nfsd leaks 3 references to the sunrpc module here:
> 
>   # echo -n "asdf 1234" >/proc/fs/nfsd/portlist
>   bash: echo: write error: Protocol not supported
> 
> Now stop nfsd and try unloading the kernel modules:
> 
>   # systemctl stop nfs-server
>   # systemctl stop nfs
>   # systemctl stop proc-fs-nfsd.mount
>   # systemctl stop var-lib-nfs-rpc_pipefs.mount
>   # rmmod nfsd
>   # rmmod nfs_acl
>   # rmmod lockd
>   # rmmod auth_rpcgss
>   # rmmod sunrpc
>   rmmod: ERROR: Module sunrpc is in use
>   # lsmod | grep rpc
>   sunrpc                315392  3
> 
> It is caused by nfsd don't cleanup rpcb program for nfsd
> when destroying svc service after creating xprt fail.
> 
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>

Thanks for looking into this!

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
J. Bruce Fields Jan. 4, 2016, 10:39 p.m. UTC | #2
On Mon, Jan 04, 2016 at 11:15:21AM +0800, Kinglong Mee wrote:
> Stefan Hajnoczi reports,
> nfsd leaks 3 references to the sunrpc module here:
> 
>   # echo -n "asdf 1234" >/proc/fs/nfsd/portlist
>   bash: echo: write error: Protocol not supported
> 
> Now stop nfsd and try unloading the kernel modules:
> 
>   # systemctl stop nfs-server
>   # systemctl stop nfs
>   # systemctl stop proc-fs-nfsd.mount
>   # systemctl stop var-lib-nfs-rpc_pipefs.mount
>   # rmmod nfsd
>   # rmmod nfs_acl
>   # rmmod lockd
>   # rmmod auth_rpcgss
>   # rmmod sunrpc
>   rmmod: ERROR: Module sunrpc is in use
>   # lsmod | grep rpc
>   sunrpc                315392  3
> 
> It is caused by nfsd don't cleanup rpcb program for nfsd
> when destroying svc service after creating xprt fail.
> 
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
>  fs/nfsd/nfssvc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index ad4e237..543de5f 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -314,14 +314,13 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
>  	 * write_ports can create the server without actually starting
>  	 * any threads--if we get shut down before any threads are
>  	 * started, then nfsd_last_thread will be run before any of this
> -	 * other initialization has been done.
> +	 * other initialization has been done except the rpcb information.
>  	 */
> +	svc_rpcb_cleanup(serv, net);

Are you sure this is right?

For example, nfsd_create_serv calls svc_destroy on svc_bind failure, is
it OK to call svc_rpcb_cleanup in that case?

--b.

>  	if (!nn->nfsd_net_up)
>  		return;
> -	nfsd_shutdown_net(net);
> -
> -	svc_rpcb_cleanup(serv, net);
>  
> +	nfsd_shutdown_net(net);
>  	printk(KERN_WARNING "nfsd: last server has exited, flushing export "
>  			    "cache\n");
>  	nfsd_export_flush(net);
> -- 
> 2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Jan. 5, 2016, 12:49 a.m. UTC | #3
On 1/5/2016 06:39, J. Bruce Fields wrote:
> On Mon, Jan 04, 2016 at 11:15:21AM +0800, Kinglong Mee wrote:
>> Stefan Hajnoczi reports,
>> nfsd leaks 3 references to the sunrpc module here:
>>
>>   # echo -n "asdf 1234" >/proc/fs/nfsd/portlist
>>   bash: echo: write error: Protocol not supported
>>
>> Now stop nfsd and try unloading the kernel modules:
>>
>>   # systemctl stop nfs-server
>>   # systemctl stop nfs
>>   # systemctl stop proc-fs-nfsd.mount
>>   # systemctl stop var-lib-nfs-rpc_pipefs.mount
>>   # rmmod nfsd
>>   # rmmod nfs_acl
>>   # rmmod lockd
>>   # rmmod auth_rpcgss
>>   # rmmod sunrpc
>>   rmmod: ERROR: Module sunrpc is in use
>>   # lsmod | grep rpc
>>   sunrpc                315392  3
>>
>> It is caused by nfsd don't cleanup rpcb program for nfsd
>> when destroying svc service after creating xprt fail.
>>
>> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>>  fs/nfsd/nfssvc.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index ad4e237..543de5f 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -314,14 +314,13 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
>>  	 * write_ports can create the server without actually starting
>>  	 * any threads--if we get shut down before any threads are
>>  	 * started, then nfsd_last_thread will be run before any of this
>> -	 * other initialization has been done.
>> +	 * other initialization has been done except the rpcb information.
>>  	 */
>> +	svc_rpcb_cleanup(serv, net);
> 
> Are you sure this is right?
> 
> For example, nfsd_create_serv calls svc_destroy on svc_bind failure, is
> it OK to call svc_rpcb_cleanup in that case?

The svc_rpcb_cleanup in nfsd_last_thread is called in svc_shutdown_net,
svc_destroy never call it.

svc_shutdown_net is called in nfsd_destroy only nfsd_serv->sv_nrthreads == 1.

So that, it's ok calling svc_rpcb_cleanup in nfsd_last_thread always.

Is there anything I missing?

thanks,
Kinglong Mee

> 
> --b.
> 
>>  	if (!nn->nfsd_net_up)
>>  		return;
>> -	nfsd_shutdown_net(net);
>> -
>> -	svc_rpcb_cleanup(serv, net);
>>  
>> +	nfsd_shutdown_net(net);
>>  	printk(KERN_WARNING "nfsd: last server has exited, flushing export "
>>  			    "cache\n");
>>  	nfsd_export_flush(net);
>> -- 
>> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Jan. 5, 2016, 10:44 p.m. UTC | #4
On Tue, Jan 05, 2016 at 08:49:06AM +0800, Kinglong Mee wrote:
> On 1/5/2016 06:39, J. Bruce Fields wrote:
> > On Mon, Jan 04, 2016 at 11:15:21AM +0800, Kinglong Mee wrote:
> >> Stefan Hajnoczi reports,
> >> nfsd leaks 3 references to the sunrpc module here:
> >>
> >>   # echo -n "asdf 1234" >/proc/fs/nfsd/portlist
> >>   bash: echo: write error: Protocol not supported
> >>
> >> Now stop nfsd and try unloading the kernel modules:
> >>
> >>   # systemctl stop nfs-server
> >>   # systemctl stop nfs
> >>   # systemctl stop proc-fs-nfsd.mount
> >>   # systemctl stop var-lib-nfs-rpc_pipefs.mount
> >>   # rmmod nfsd
> >>   # rmmod nfs_acl
> >>   # rmmod lockd
> >>   # rmmod auth_rpcgss
> >>   # rmmod sunrpc
> >>   rmmod: ERROR: Module sunrpc is in use
> >>   # lsmod | grep rpc
> >>   sunrpc                315392  3
> >>
> >> It is caused by nfsd don't cleanup rpcb program for nfsd
> >> when destroying svc service after creating xprt fail.
> >>
> >> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> >> ---
> >>  fs/nfsd/nfssvc.c | 7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> >> index ad4e237..543de5f 100644
> >> --- a/fs/nfsd/nfssvc.c
> >> +++ b/fs/nfsd/nfssvc.c
> >> @@ -314,14 +314,13 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
> >>  	 * write_ports can create the server without actually starting
> >>  	 * any threads--if we get shut down before any threads are
> >>  	 * started, then nfsd_last_thread will be run before any of this
> >> -	 * other initialization has been done.
> >> +	 * other initialization has been done except the rpcb information.
> >>  	 */
> >> +	svc_rpcb_cleanup(serv, net);
> > 
> > Are you sure this is right?
> > 
> > For example, nfsd_create_serv calls svc_destroy on svc_bind failure, is
> > it OK to call svc_rpcb_cleanup in that case?
> 
> The svc_rpcb_cleanup in nfsd_last_thread is called in svc_shutdown_net,
> svc_destroy never call it.
> 
> svc_shutdown_net is called in nfsd_destroy only nfsd_serv->sv_nrthreads == 1.
> 
> So that, it's ok calling svc_rpcb_cleanup in nfsd_last_thread always.
> 
> Is there anything I missing?

Oops, somehow I got nfsd_destroy and svc_destroy confused.

Applying.  Thanks for your patience!

--b.

> 
> thanks,
> Kinglong Mee
> 
> > 
> > --b.
> > 
> >>  	if (!nn->nfsd_net_up)
> >>  		return;
> >> -	nfsd_shutdown_net(net);
> >> -
> >> -	svc_rpcb_cleanup(serv, net);
> >>  
> >> +	nfsd_shutdown_net(net);
> >>  	printk(KERN_WARNING "nfsd: last server has exited, flushing export "
> >>  			    "cache\n");
> >>  	nfsd_export_flush(net);
> >> -- 
> >> 2.5.0
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index ad4e237..543de5f 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -314,14 +314,13 @@  static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
 	 * write_ports can create the server without actually starting
 	 * any threads--if we get shut down before any threads are
 	 * started, then nfsd_last_thread will be run before any of this
-	 * other initialization has been done.
+	 * other initialization has been done except the rpcb information.
 	 */
+	svc_rpcb_cleanup(serv, net);
 	if (!nn->nfsd_net_up)
 		return;
-	nfsd_shutdown_net(net);
-
-	svc_rpcb_cleanup(serv, net);
 
+	nfsd_shutdown_net(net);
 	printk(KERN_WARNING "nfsd: last server has exited, flushing export "
 			    "cache\n");
 	nfsd_export_flush(net);