2010-06-25 18:21:03

by Jeff Layton

Subject: [PATCH 2/2] nfsd: reject NFSv4 requests over UDP (try #2)

RFC3530 states:

Where an NFS version 4 implementation supports operation over the IP
network protocol, the supported transports between NFS and IP MUST be
among the IETF-approved congestion control transport protocols, which
include TCP and SCTP

This patch adds a wrapper for nfsd_dispatch that checks to see whether
the call came in via UDP, and then rejects it with a PROG_MISMATCH RPC
error. This replaces the earlier one I proposed -- it should mean no
extra overhead for NFSv2/3.

It also has the NFS server skip rpcbind registration for NFSv4 over UDP.

Reported-by: Sachin Prabhu <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4proc.c | 3 ++-
fs/nfsd/nfsd.h | 1 +
fs/nfsd/nfssvc.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 59ec449..afbd03c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1363,8 +1363,9 @@ struct svc_version nfsd_version4 = {
.vs_vers = 4,
.vs_nproc = 2,
.vs_proc = nfsd_procedures4,
- .vs_dispatch = nfsd_dispatch,
+ .vs_dispatch = nfsd4_dispatch,
.vs_xdrsize = NFS4_SVC_XDRSIZE,
+ .vs_hidden_udp = true,
};

/*
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7237776..f36e07c 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -41,6 +41,7 @@ extern const struct seq_operations nfs_exports_op;
*/
int nfsd_svc(unsigned short port, int nrservs);
int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp);
+int nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp);

int nfsd_nrthreads(void);
int nfsd_nrpools(void);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 6b59d32..0a04472 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -595,6 +595,19 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
return 1;
}

+int
+nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp)
+{
+ /* Reject any NFSv4 request over UDP. Don't bother caching it. */
+ if (rqstp->rq_prot == IPPROTO_UDP) {
+ dprintk("%s: rejecting vers 4 request over UDP\n", __func__);
+ *statp = rpc_prog_mismatch;
+ return 1;
+ }
+
+ return nfsd_dispatch(rqstp, statp);
+}
+
int nfsd_pool_stats_open(struct inode *inode, struct file *file)
{
int ret;
--
1.5.5.6

2010-06-25 19:02:12

by Jeff Layton

Subject: Re: [PATCH 2/2] nfsd: reject NFSv4 requests over UDP (try #2)

On Fri, 25 Jun 2010 14:45:09 -0400
Trond Myklebust <[email protected]> wrote:

> On Fri, 2010-06-25 at 14:20 -0400, Jeff Layton wrote:
> > RFC3530 states:
> >
> > Where an NFS version 4 implementation supports operation over the IP
> > network protocol, the supported transports between NFS and IP MUST be
> > among the IETF-approved congestion control transport protocols, which
> > include TCP and SCTP
> >
> > This patch adds a wrapper for nfsd_dispatch that checks to see whether
> > the call came in via UDP, and then rejects it with a PROG_MISMATCH RPC
> > error. This replaces the earlier one I proposed -- it should mean no
> > extra overhead for NFSv2/3.
> >
> > It also has the NFS server skip rpcbind registration for NFSv4 over UDP.
> >
> > Reported-by: Sachin Prabhu <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/nfsd/nfs4proc.c | 3 ++-
> > fs/nfsd/nfsd.h | 1 +
> > fs/nfsd/nfssvc.c | 13 +++++++++++++
> > 3 files changed, 16 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 59ec449..afbd03c 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1363,8 +1363,9 @@ struct svc_version nfsd_version4 = {
> > .vs_vers = 4,
> > .vs_nproc = 2,
> > .vs_proc = nfsd_procedures4,
> > - .vs_dispatch = nfsd_dispatch,
> > + .vs_dispatch = nfsd4_dispatch,
> > .vs_xdrsize = NFS4_SVC_XDRSIZE,
> > + .vs_hidden_udp = true,
> > };
> >
> > /*
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 7237776..f36e07c 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -41,6 +41,7 @@ extern const struct seq_operations nfs_exports_op;
> > */
> > int nfsd_svc(unsigned short port, int nrservs);
> > int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp);
> > +int nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp);
> >
> > int nfsd_nrthreads(void);
> > int nfsd_nrpools(void);
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 6b59d32..0a04472 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -595,6 +595,19 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
> > return 1;
> > }
> >
> > +int
> > +nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp)
> > +{
> > + /* Reject any NFSv4 request over UDP. Don't bother caching it. */
> > + if (rqstp->rq_prot == IPPROTO_UDP) {
> > + dprintk("%s: rejecting vers 4 request over UDP\n", __func__);
> > + *statp = rpc_prog_mismatch;
> > + return 1;
> > + }
> > +
> > + return nfsd_dispatch(rqstp, statp);
> > +}
> > +
> > int nfsd_pool_stats_open(struct inode *inode, struct file *file)
> > {
> > int ret;
>
> Won't that lead to svc_process() returning garbage? As far as I can see,
> it will indeed lead to the status being set to RPC_PROG_MISMATCH, but it
> will fail to set the 'lower/upper' RPC prognumber arguments.
>
> Might something like the following perhaps work?
>
> statp[0] = rpc_prog_mismatch;
> statp[1] = 2;
> statp[2] = 3;
> rqstp->rq_res.head[0].iov_len += 2 * sizeof(__be32);
> return 0;
>
> Cheers
> Trond
>

Thanks. I misread the spec there -- we do need to specify a low and
high version. It's not quite that simple though -- I think we need to
base this on what nfsd versions are currently enabled.

--
Jeff Layton <[email protected]>

2010-06-25 18:45:15

by Trond Myklebust

Subject: Re: [PATCH 2/2] nfsd: reject NFSv4 requests over UDP (try #2)

On Fri, 2010-06-25 at 14:20 -0400, Jeff Layton wrote:
> RFC3530 states:
>
> Where an NFS version 4 implementation supports operation over the IP
> network protocol, the supported transports between NFS and IP MUST be
> among the IETF-approved congestion control transport protocols, which
> include TCP and SCTP
>
> This patch adds a wrapper for nfsd_dispatch that checks to see whether
> the call came in via UDP, and then rejects it with a PROG_MISMATCH RPC
> error. This replaces the earlier one I proposed -- it should mean no
> extra overhead for NFSv2/3.
>
> It also has the NFS server skip rpcbind registration for NFSv4 over UDP.
>
> Reported-by: Sachin Prabhu <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 3 ++-
> fs/nfsd/nfsd.h | 1 +
> fs/nfsd/nfssvc.c | 13 +++++++++++++
> 3 files changed, 16 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 59ec449..afbd03c 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1363,8 +1363,9 @@ struct svc_version nfsd_version4 = {
> .vs_vers = 4,
> .vs_nproc = 2,
> .vs_proc = nfsd_procedures4,
> - .vs_dispatch = nfsd_dispatch,
> + .vs_dispatch = nfsd4_dispatch,
> .vs_xdrsize = NFS4_SVC_XDRSIZE,
> + .vs_hidden_udp = true,
> };
>
> /*
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 7237776..f36e07c 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -41,6 +41,7 @@ extern const struct seq_operations nfs_exports_op;
> */
> int nfsd_svc(unsigned short port, int nrservs);
> int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp);
> +int nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp);
>
> int nfsd_nrthreads(void);
> int nfsd_nrpools(void);
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 6b59d32..0a04472 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -595,6 +595,19 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
> return 1;
> }
>
> +int
> +nfsd4_dispatch(struct svc_rqst *rqstp, __be32 *statp)
> +{
> + /* Reject any NFSv4 request over UDP. Don't bother caching it. */
> + if (rqstp->rq_prot == IPPROTO_UDP) {
> + dprintk("%s: rejecting vers 4 request over UDP\n", __func__);
> + *statp = rpc_prog_mismatch;
> + return 1;
> + }
> +
> + return nfsd_dispatch(rqstp, statp);
> +}
> +
> int nfsd_pool_stats_open(struct inode *inode, struct file *file)
> {
> int ret;

Won't that lead to svc_process() returning garbage? As far as I can see,
it will indeed lead to the status being set to RPC_PROG_MISMATCH, but it
will fail to set the 'lower/upper' RPC prognumber arguments.

Might something like the following perhaps work?

statp[0] = rpc_prog_mismatch;
statp[1] = 2;
statp[2] = 3;
rqstp->rq_res.head[0].iov_len += 2 * sizeof(__be32);
return 0;

Cheers
Trond