[PATCH] nfs: set vs_hidden on nfs4_callback_version4

[PATCH] nfs: set vs_hidden on nfs4_callback_version4

[Jeff Layton]

This service should not be registered with or unregistered from rpcbind.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/callback_xdr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index c6c86a7..7c8b800 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -996,4 +996,5 @@ struct svc_version nfs4_callback_version4 = {
 	.vs_proc = nfs4_callback_procedures1,
 	.vs_xdrsize = NFS4_CALLBACK_XDRSIZE,
 	.vs_dispatch = NULL,
+	.vs_hidden = 1,
 };
-- 
1.7.6.2

Re: [PATCH] nfs: set vs_hidden on nfs4_callback_version4

[Chuck Lever]

On Sep 21, 2011, at 3:55 PM, Jeff Layton wrote:

> This service should not be registered with or unregistered from rpcbind.

I've been watching the larger conversation.  One thing that registering does, even if you don't want to advertise your service, is tell you if there is already another service on that port.  Do we want to worry about possible port collisions for listeners like the callback service?

Do we want to ensure that any other service on that port is unregistered?  We would already discover a listener on our port when trying to create the socket, but an old registration may persist even after that old service has gone away.

This is something to think about when revisiting the semantics of .vs_hidden.

> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/callback_xdr.c |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index c6c86a7..7c8b800 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -996,4 +996,5 @@ struct svc_version nfs4_callback_version4 = {
> 	.vs_proc = nfs4_callback_procedures1,
> 	.vs_xdrsize = NFS4_CALLBACK_XDRSIZE,
> 	.vs_dispatch = NULL,
> +	.vs_hidden = 1,
> };
> -- 
> 1.7.6.2
> 
> --
> 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

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

RE: [PATCH] nfs: set vs_hidden on nfs4_callback_version4

[Myklebust, Trond]

> -----Original Message-----
> From: Chuck Lever [mailto:chuck.lever@oracle.com]
> Sent: Wednesday, September 21, 2011 4:06 PM
> To: Jeff Layton
> Cc: Myklebust, Trond; linux-nfs@vger.kernel.org;
skinsbursky@parallels.com
> Subject: Re: [PATCH] nfs: set vs_hidden on nfs4_callback_version4
> 
> 
> On Sep 21, 2011, at 3:55 PM, Jeff Layton wrote:
> 
> > This service should not be registered with or unregistered from
rpcbind.
> 
> I've been watching the larger conversation.  One thing that
registering does,
> even if you don't want to advertise your service, is tell you if there
is already
> another service on that port.  Do we want to worry about possible port
> collisions for listeners like the callback service?

We already have a module parameter for fixing the callback service so
that we can avoid port collisions with known services. That said,
normally we do not want to rely on the portmapper for detecting the
existence of a service: that will in any case only detect RPC services
and, as you point out below, is subject to stale entries.

> Do we want to ensure that any other service on that port is
unregistered?
> We would already discover a listener on our port when trying to create
the
> socket, but an old registration may persist even after that old
service has
> gone away.

If anyone tries to use the service which is advertised by the stale
portmapper entry, then they will get a PROG_UNAVAIL error.

Cheers
  Trond

Re: [PATCH] nfs: set vs_hidden on nfs4_callback_version4

[Jeff Layton]

On Wed, 21 Sep 2011 13:24:48 -0700
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> > -----Original Message-----
> > From: Chuck Lever [mailto:chuck.lever@oracle.com]
> > Sent: Wednesday, September 21, 2011 4:06 PM
> > To: Jeff Layton
> > Cc: Myklebust, Trond; linux-nfs@vger.kernel.org;
> skinsbursky@parallels.com
> > Subject: Re: [PATCH] nfs: set vs_hidden on nfs4_callback_version4
> > 
> > 
> > On Sep 21, 2011, at 3:55 PM, Jeff Layton wrote:
> > 
> > > This service should not be registered with or unregistered from
> rpcbind.
> > 
> > I've been watching the larger conversation.  One thing that
> registering does,
> > even if you don't want to advertise your service, is tell you if there
> is already
> > another service on that port.  Do we want to worry about possible port
> > collisions for listeners like the callback service?
> 
> We already have a module parameter for fixing the callback service so
> that we can avoid port collisions with known services. That said,
> normally we do not want to rely on the portmapper for detecting the
> existence of a service: that will in any case only detect RPC services
> and, as you point out below, is subject to stale entries.
> 

Doing so also seems like a potential DoS vector. Just register every
reserved port you can get your hands on and enjoy the show...

> > Do we want to ensure that any other service on that port is
> unregistered?
> > We would already discover a listener on our port when trying to create
> the
> > socket, but an old registration may persist even after that old
> service has
> > gone away.
> 
> If anyone tries to use the service which is advertised by the stale
> portmapper entry, then they will get a PROG_UNAVAIL error.
> 

We should also point out that other non-RPC based services in the
kernel do not deal with the portmapper at all. I don't see any harm
from unregistering these ports from the portmapper on service creation,
but I'm not sure it really provides much benefit.

In any case though, I didn't think the rpcbind protocol really gave you
the ability to unregister based on the just the port -- you have to do
it by program number. In order to do this, we'd need to poll for the
entire list of registered services and unregister any that have
matching ports. That sounds ugly to implement :-/

-- 
Jeff Layton <jlayton@redhat.com>