diff options
author | Niels de Vos <ndevos@redhat.com> | 2017-07-17 16:43:30 +0200 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2017-08-11 10:51:58 +0000 |
commit | 6c2077618d336edea299d946084ebb4edbbfd47e (patch) | |
tree | ef60da69cfd774c0cdfb8ca9ae800d5df402ea29 | |
parent | a060485aae8cb508e05e0f9e46e3f0ec823a7f22 (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: 1471870
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: https://review.gluster.org/17792
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: jiffin tony Thottan <jthottan@redhat.com>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
-rw-r--r-- | xlators/nfs/server/src/nfs3.c | 63 | ||||
-rw-r--r-- | xlators/nfs/server/src/nfs3.h | 3 | ||||
-rw-r--r-- | 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 aa26cc8c064..e4181779511 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 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 8580448b762..dc7d23dbef5 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; |