summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNiels de Vos <ndevos@redhat.com>2017-07-17 16:43:30 +0200
committerShyamsundar Ranganathan <srangana@redhat.com>2017-07-21 14:21:11 +0000
commitb5d0c9b48e87455b961a3e0022de4091d9a4cdf8 (patch)
treee3cb4a02285148cc4fdb586e5459b5d2d6790a84
parenta8d93a1498458a121f8670504dcb639635566347 (diff)
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: 1471869 Signed-off-by: Niels de Vos <ndevos@redhat.com> Reviewed-on: https://review.gluster.org/17797 Smoke: Gluster Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
-rw-r--r--xlators/nfs/server/src/nfs3.c63
-rw-r--r--xlators/nfs/server/src/nfs3.h3
-rw-r--r--xlators/nfs/server/src/nlm4.c15
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;