nfs: make nfs3_call_state_t refcounted

There is no refcounting done of the nfs3_call_state_t structure, which
seems to result in use-after-free problems in the NLM part of
Gluster/NFS. The structure is initialized with two different functions,
it is easier to have a single place to do this.

The Gluster/NFS part will not use the refcounting, for now. This is
being added to make the NLM code more stable. nfs3_call_state_wipe()
will behave as before for Gluster/NFS, but cleanup is triggered through
the refcounting now. This prevents major changes to the stable part of
the NFS-server, and makes it possible to improve the NLM component
separately.

Change-Id: I2e15bcf12af74e8a46c2727e4a160e9444d29ece
BUG: 1467313
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: https://review.gluster.org/17696
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Reviewed-by: jiffin tony Thottan <jthottan@redhat.com>

3 files changed, 42 insertions, 39 deletions

diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c
index 6399d624ff8..7da412d0a35 100644
--- a/xlators/nfs/server/src/nfs3.c
+++ b/xlators/nfs/server/src/nfs3.c
@@ -526,6 +526,38 @@ nfs3_solaris_zerolen_fh (struct nfs3_fh *fh, int fhlen)
  */
 typedef ssize_t (*nfs3_serializer) (struct iovec outmsg, void *args);
 
+static void
+__nfs3_call_state_wipe (nfs3_call_state_t *cs)
+{
+        if (!cs)
+                return;
+
+        if (cs->fd) {
+                gf_msg_trace (GF_NFS3, 0, "fd 0x%lx ref: %d",
+                        (long)cs->fd, cs->fd->refcount);
+                fd_unref (cs->fd);
+        }
+
+        GF_FREE (cs->resolventry);
+
+        GF_FREE (cs->pathname);
+
+        if (!list_empty (&cs->entries.list))
+                gf_dirent_free (&cs->entries);
+
+        nfs_loc_wipe (&cs->oploc);
+        nfs_loc_wipe (&cs->resolvedloc);
+        if (cs->iob)
+                iobuf_unref (cs->iob);
+        if (cs->iobref)
+                iobref_unref (cs->iobref);
+        if (cs->trans)
+                rpc_transport_unref (cs->trans);
+        memset (cs, 0, sizeof (*cs));
+        mem_put (cs);
+        /* Already refd by fd_lookup, so no need to ref again. */
+}
+
 nfs3_call_state_t *
 nfs3_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req, xlator_t *v)
 {
@@ -533,7 +565,7 @@ nfs3_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req, xlator_t *v)
 
         GF_VALIDATE_OR_GOTO (GF_NFS3, s, err);
         GF_VALIDATE_OR_GOTO (GF_NFS3, req, err);
-        GF_VALIDATE_OR_GOTO (GF_NFS3, v, err);
+        /* GF_VALIDATE_OR_GOTO (GF_NFS3, v, err); NLM sets this later */
 
         cs = (nfs3_call_state_t *) mem_get (s->localpool);
         if (!cs) {
@@ -543,6 +575,7 @@ nfs3_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req, xlator_t *v)
         }
 
         memset (cs, 0, sizeof (*cs));
+        GF_REF_INIT (cs, __nfs3_call_state_wipe);
         INIT_LIST_HEAD (&cs->entries.list);
         INIT_LIST_HEAD (&cs->openwait_q);
         cs->operrno = EINVAL;
@@ -557,33 +590,7 @@ err:
 void
 nfs3_call_state_wipe (nfs3_call_state_t *cs)
 {
-        if (!cs)
-                return;
-
-        if (cs->fd) {
-                gf_msg_trace (GF_NFS3, 0, "fd 0x%lx ref: %d",
-                        (long)cs->fd, cs->fd->refcount);
-                fd_unref (cs->fd);
-        }
-
-        GF_FREE (cs->resolventry);
-
-        GF_FREE (cs->pathname);
-
-        if (!list_empty (&cs->entries.list))
-                gf_dirent_free (&cs->entries);
-
-        nfs_loc_wipe (&cs->oploc);
-        nfs_loc_wipe (&cs->resolvedloc);
-        if (cs->iob)
-                iobuf_unref (cs->iob);
-        if (cs->iobref)
-                iobref_unref (cs->iobref);
-        if (cs->trans)
-                rpc_transport_unref (cs->trans);
-        memset (cs, 0, sizeof (*cs));
-        mem_put (cs);
-        /* Already refd by fd_lookup, so no need to ref again. */
+        GF_REF_PUT (cs);
 }
 
 
diff --git a/xlators/nfs/server/src/nfs3.h b/xlators/nfs/server/src/nfs3.h
index 4cb3e67528d..187fb7e1912 100644
--- a/xlators/nfs/server/src/nfs3.h
+++ b/xlators/nfs/server/src/nfs3.h
@@ -23,6 +23,7 @@
 #include "nlm4.h"
 #include "acl3-xdr.h"
 #include "acl3.h"
+#include "refcount.h"
 #include <sys/statvfs.h>
 
 #define GF_NFS3                 GF_NFS"-nfsv3"
@@ -184,6 +185,8 @@ typedef int (*nfs3_resume_fn_t) (void *cs);
  * Imagine the chaos if we need a mem-pool for each one of those sub-structures.
  */
 struct nfs3_local {
+        GF_REF_DECL;
+
         rpcsvc_request_t        *req;
         xlator_t                *vol;
         nfs3_resume_fn_t        resume_fn;
diff --git a/xlators/nfs/server/src/nlm4.c b/xlators/nfs/server/src/nlm4.c
index 281eaee5fab..d0478f6250e 100644
--- a/xlators/nfs/server/src/nlm4.c
+++ b/xlators/nfs/server/src/nlm4.c
@@ -48,6 +48,9 @@ typedef ssize_t (*nlm4_serializer) (struct iovec outmsg, void *args);
 
 extern void nfs3_call_state_wipe (nfs3_call_state_t *cs);
 
+nfs3_call_state_t *
+nfs3_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req, xlator_t *v);
+
 struct list_head nlm_client_list;
 gf_lock_t nlm_client_list_lk;
 
@@ -67,9 +70,6 @@ int nlm_grace_period = 50;
                 }                                                       \
         } while (0);                                                    \
 
-nfs3_call_state_t *
-nfs3_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req, xlator_t *v);
-
 #define nlm4_handle_call_state_init(nfs3state, calls, rq, opstat, errlabel)\
         do {                                                            \
                 calls = nlm4_call_state_init ((nfs3state), (rq));       \
@@ -267,17 +267,10 @@ nlm4_call_state_init (struct nfs3_state *s, rpcsvc_request_t *req)
         if ((!s) || (!req))
                 return NULL;
 
-        cs = (nfs3_call_state_t *) mem_get (s->localpool);
+        cs = nfs3_call_state_init (s, req, NULL);
         if (!cs)
                 return NULL;
 
-        memset (cs, 0, sizeof (*cs));
-        INIT_LIST_HEAD (&cs->entries.list);
-        INIT_LIST_HEAD (&cs->openwait_q);
-        cs->operrno = EINVAL;
-        cs->req = req;
-        cs->nfsx = s->nfsx;
-        cs->nfs3state = s;
         cs->monitor = 1;
 
         return cs;