NFSD: Fix checksum mismatches in the duplicate reply cache

nfsd_cache_csum() currently assumes that the server's RPC layer has
been advancing rq_arg.head[0].iov_base as it decodes an incoming
request, because that's the way it used to work. On entry, it
expects that buf->head[0].iov_base points to the start of the NFS
header, and excludes the already-decoded RPC header.

These days however, head[0].iov_base now points to the start of the
RPC header during all processing. It no longer points at the NFS
Call header when execution arrives at nfsd_cache_csum().

In a retransmitted RPC the XID and the NFS header are supposed to
be the same as the original message, but the contents of the
retransmitted RPC header can be different. For example, for krb5,
the GSS sequence number will be different between the two. Thus if
the RPC header is always included in the DRC checksum computation,
the checksum of the retransmitted message might not match the
checksum of the original message, even though the NFS part of these
messages is identical.

The result is that, even if a matching XID is found in the DRC,
the checksum mismatch causes the server to execute the
retransmitted RPC transaction again.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Tested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index 929248c6ca84c47f828feecbdcbbb18286d5819f..4cbe0434cbb8ce973865153ddf4a9d7c332bea5e 100644 (file)
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -84,8 +84,8 @@ int   nfsd_net_reply_cache_init(struct nfsd_net *nn);
 void   nfsd_net_reply_cache_destroy(struct nfsd_net *nn);
 int    nfsd_reply_cache_init(struct nfsd_net *);
 void   nfsd_reply_cache_shutdown(struct nfsd_net *);
-int    nfsd_cache_lookup(struct svc_rqst *rqstp,
-                         struct nfsd_cacherep **cacherep);
+int    nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
+                         unsigned int len, struct nfsd_cacherep **cacherep);
 void   nfsd_cache_update(struct svc_rqst *rqstp, struct nfsd_cacherep *rp,
                          int cachetype, __be32 *statp);
 int    nfsd_reply_cache_stats_show(struct seq_file *m, void *v);

diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index abb453be71ca3f106c8a62caa0911103fdd40b5b..6cd36af2f97e10610cdd1ba4821c5bb01de19bcb 100644 (file)
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -368,33 +368,52 @@ nfsd_reply_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
        return freed;
 }
 
-/*
- * Walk an xdr_buf and get a CRC for at most the first RC_CSUMLEN bytes
+/**
+ * nfsd_cache_csum - Checksum incoming NFS Call arguments
+ * @buf: buffer containing a whole RPC Call message
+ * @start: starting byte of the NFS Call header
+ * @remaining: size of the NFS Call header, in bytes
+ *
+ * Compute a weak checksum of the leading bytes of an NFS procedure
+ * call header to help verify that a retransmitted Call matches an
+ * entry in the duplicate reply cache.
+ *
+ * To avoid assumptions about how the RPC message is laid out in
+ * @buf and what else it might contain (eg, a GSS MIC suffix), the
+ * caller passes us the exact location and length of the NFS Call
+ * header.
+ *
+ * Returns a 32-bit checksum value, as defined in RFC 793.
  */
-static __wsum
-nfsd_cache_csum(struct svc_rqst *rqstp)
+static __wsum nfsd_cache_csum(struct xdr_buf *buf, unsigned int start,
+                             unsigned int remaining)
 {
+       unsigned int base, len;
+       struct xdr_buf subbuf;
+       __wsum csum = 0;
+       void *p;
        int idx;
-       unsigned int base;
-       __wsum csum;
-       struct xdr_buf *buf = &rqstp->rq_arg;
-       const unsigned char *p = buf->head[0].iov_base;
-       size_t csum_len = min_t(size_t, buf->head[0].iov_len + buf->page_len,
-                               RC_CSUMLEN);
-       size_t len = min(buf->head[0].iov_len, csum_len);
+
+       if (remaining > RC_CSUMLEN)
+               remaining = RC_CSUMLEN;
+       if (xdr_buf_subsegment(buf, &subbuf, start, remaining))
+               return csum;
 
        /* rq_arg.head first */
-       csum = csum_partial(p, len, 0);
-       csum_len -= len;
+       if (subbuf.head[0].iov_len) {
+               len = min_t(unsigned int, subbuf.head[0].iov_len, remaining);
+               csum = csum_partial(subbuf.head[0].iov_base, len, csum);
+               remaining -= len;
+       }
 
        /* Continue into page array */
-       idx = buf->page_base / PAGE_SIZE;
-       base = buf->page_base & ~PAGE_MASK;
-       while (csum_len) {
-               p = page_address(buf->pages[idx]) + base;
-               len = min_t(size_t, PAGE_SIZE - base, csum_len);
+       idx = subbuf.page_base / PAGE_SIZE;
+       base = subbuf.page_base & ~PAGE_MASK;
+       while (remaining) {
+               p = page_address(subbuf.pages[idx]) + base;
+               len = min_t(unsigned int, PAGE_SIZE - base, remaining);
                csum = csum_partial(p, len, csum);
-               csum_len -= len;
+               remaining -= len;
                base = 0;
                ++idx;
        }
@@ -465,6 +484,8 @@ out:
 /**
  * nfsd_cache_lookup - Find an entry in the duplicate reply cache
  * @rqstp: Incoming Call to find
+ * @start: starting byte in @rqstp->rq_arg of the NFS Call header
+ * @len: size of the NFS Call header, in bytes
  * @cacherep: OUT: DRC entry for this request
  *
  * Try to find an entry matching the current call in the cache. When none
@@ -478,7 +499,8 @@ out:
  *   %RC_REPLY: Reply from cache
  *   %RC_DROPIT: Do not process the request further
  */
-int nfsd_cache_lookup(struct svc_rqst *rqstp, struct nfsd_cacherep **cacherep)
+int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
+                     unsigned int len, struct nfsd_cacherep **cacherep)
 {
        struct nfsd_net         *nn;
        struct nfsd_cacherep    *rp, *found;
@@ -494,7 +516,7 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, struct nfsd_cacherep **cacherep)
                goto out;
        }
 
-       csum = nfsd_cache_csum(rqstp);
+       csum = nfsd_cache_csum(&rqstp->rq_arg, start, len);
 
        /*
         * Since the common case is a cache miss followed by an insert,

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index b4e4e04f99317ac9221035b10108d87bfd90eaa7..fe61d9bbcc1faa2d704f9ec926812a022d78381d 100644 (file)
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -981,6 +981,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
        const struct svc_procedure *proc = rqstp->rq_procinfo;
        __be32 *statp = rqstp->rq_accept_statp;
        struct nfsd_cacherep *rp;
+       unsigned int start, len;
        __be32 *nfs_reply;
 
        /*
@@ -989,6 +990,13 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
         */
        rqstp->rq_cachetype = proc->pc_cachetype;
 
+       /*
+        * ->pc_decode advances the argument stream past the NFS
+        * Call header, so grab the header's starting location and
+        * size now for the call to nfsd_cache_lookup().
+        */
+       start = xdr_stream_pos(&rqstp->rq_arg_stream);
+       len = xdr_stream_remaining(&rqstp->rq_arg_stream);
        if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
                goto out_decode_err;
 
@@ -1002,7 +1010,7 @@ int nfsd_dispatch(struct svc_rqst *rqstp)
        smp_store_release(&rqstp->rq_status_counter, rqstp->rq_status_counter | 1);
 
        rp = NULL;
-       switch (nfsd_cache_lookup(rqstp, &rp)) {
+       switch (nfsd_cache_lookup(rqstp, start, len, &rp)) {
        case RC_DOIT:
                break;
        case RC_REPLY: