From c1ff78dc03af63b226289ea0d1618223addfbd58 Mon Sep 17 00:00:00 2001

From: Niels de Vos <ndevos@redhat.com>

Date: Fri, 23 Jun 2017 10:01:27 +0200

Subject: [PATCH 546/557] 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.

Cherry picked from commit daed52b8ebcac7ef36f11e944f83826f46593867:

> 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>

Change-Id: I2e15bcf12af74e8a46c2727e4a160e9444d29ece

BUG: 1411344

Signed-off-by: Niels de Vos <ndevos@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/111765

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

---

 xlators/nfs/server/src/nfs3.c | 63 ++++++++++++++++++++++++-------------------

 xlators/nfs/server/src/nfs3.h |  3 +++

 xlators/nfs/server/src/nlm4.c | 15 +++--------

 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c

index 8b1d62b..bb68bc1 100644

--- a/xlators/nfs/server/src/nfs3.c

+++ b/xlators/nfs/server/src/nfs3.c

@@ -483,6 +483,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)

 {

@@ -490,7 +522,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) {

@@ -500,6 +532,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;

@@ -514,33 +547,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 4cb3e67..187fb7e 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 8580448..dc7d23d 100644

--- a/xlators/nfs/server/src/nlm4.c

+++ b/xlators/nfs/server/src/nlm4.c

@@ -52,6 +52,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;

@@ -71,9 +74,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));       \

@@ -271,17 +271,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;

-- 

1.8.3.1