From dc0020c72d5c2d20328b89224b149ebb87002277 Mon Sep 17 00:00:00 2001 From: Kotresh HR Date: Fri, 24 Apr 2015 17:31:03 +0530 Subject: rpc: Maintain separate xlator pointer in 'rpcsvc_state' The structure 'rpcsvc_state', which maintains rpc server state had no separate pointer to track the translator. It was using the mydata pointer itself. So callers were forced to send xlator pointer as mydata which is opaque (void pointer) by function prototype. 'rpcsvc_register_init' is setting svc->mydata with xlator pointer. 'rpcsvc_register_notify' is overwriting svc->mydata with mydata pointer. And rpc interprets svc->mydata as xlator pointer internally. If someone passes non xlator structure pointer to rpcsvc_register_notify as libgfchangelog currently does, it might corrupt mydata. So interpreting opaque mydata as xlator pointer is incorrect as it is caller's choice to send mydata as any type of data to 'rpcsvc_register_notify'. Maintaining two different pointers in 'rpcsvc_state' for xlator and mydata solves the issue. Change-Id: I7874933fefc68f3fe01d44f92016a8e4e9768378 BUG: 1215161 Signed-off-by: Kotresh HR Reviewed-on: http://review.gluster.org/10366 Tested-by: Gluster Build System Reviewed-by: Raghavendra G Tested-by: Raghavendra G --- rpc/rpc-lib/src/rpcsvc-common.h | 4 +++- rpc/rpc-lib/src/rpcsvc.c | 8 ++++---- xlators/features/changelog/src/changelog-rpc-common.c | 4 ++-- xlators/features/changelog/src/changelog-rpc.c | 2 +- xlators/features/quota/src/quotad-helpers.c | 2 +- xlators/protocol/server/src/server-handshake.c | 6 +++--- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/rpc/rpc-lib/src/rpcsvc-common.h b/rpc/rpc-lib/src/rpcsvc-common.h index 832645bd12a..dd958032336 100644 --- a/rpc/rpc-lib/src/rpcsvc-common.h +++ b/rpc/rpc-lib/src/rpcsvc-common.h @@ -16,6 +16,7 @@ #include "compat.h" #include "glusterfs.h" #include "dict.h" +#include "xlator.h" typedef enum { RPCSVC_EVENT_ACCEPT, @@ -69,7 +70,8 @@ typedef struct rpcsvc_state { struct list_head notify; int notify_count; - void *mydata; /* This is xlator */ + xlator_t *xl; /* xlator */ + void *mydata; rpcsvc_notify_t notifyfn; struct mem_pool *rxpool; rpcsvc_drc_globals_t *drc; diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c index b395149e032..be95d25b1b1 100644 --- a/rpc/rpc-lib/src/rpcsvc.c +++ b/rpc/rpc-lib/src/rpcsvc.c @@ -677,7 +677,7 @@ rpcsvc_handle_rpc_call (rpcsvc_t *svc, rpc_transport_t *trans, if (req->rpc_err == SUCCESS) { /* Before going to xlator code, set the THIS properly */ - THIS = svc->mydata; + THIS = svc->xl; actor_fn = actor->actor; @@ -1802,7 +1802,7 @@ rpcsvc_register_notify (rpcsvc_t *svc, rpcsvc_notify_t notify, void *mydata) if (!wrapper) { goto out; } - svc->mydata = mydata; /* this_xlator */ + svc->mydata = mydata; wrapper->data = mydata; wrapper->notify = notify; @@ -2045,7 +2045,7 @@ rpcsvc_reconfigure_options (rpcsvc_t *svc, dict_t *options) return (-1); /* Fetch the xlator from svc */ - xlator = (xlator_t *) svc->mydata; + xlator = svc->xl; if (!xlator) return (-1); @@ -2311,7 +2311,7 @@ rpcsvc_init (xlator_t *xl, glusterfs_ctx_t *ctx, dict_t *options, ret = -1; svc->options = options; svc->ctx = ctx; - svc->mydata = xl; + svc->xl = xl; gf_log (GF_RPCSVC, GF_LOG_DEBUG, "RPC service inited."); gluster_dump_prog.options = options; diff --git a/xlators/features/changelog/src/changelog-rpc-common.c b/xlators/features/changelog/src/changelog-rpc-common.c index 76db6696ae8..de3a730534e 100644 --- a/xlators/features/changelog/src/changelog-rpc-common.c +++ b/xlators/features/changelog/src/changelog-rpc-common.c @@ -249,8 +249,8 @@ changelog_rpc_server_destroy (xlator_t *this, rpcsvc_t *rpc, char *sockfile, rpcsvc_notify_t fn, struct rpcsvc_program **progs) { rpcsvc_listener_t *listener = NULL; - rpcsvc_listener_t *next = NULL; - struct rpcsvc_program *prog = NULL; + rpcsvc_listener_t *next = NULL; + struct rpcsvc_program *prog = NULL; while (*progs) { prog = *progs; diff --git a/xlators/features/changelog/src/changelog-rpc.c b/xlators/features/changelog/src/changelog-rpc.c index 04326456d31..ffbc61e40f9 100644 --- a/xlators/features/changelog/src/changelog-rpc.c +++ b/xlators/features/changelog/src/changelog-rpc.c @@ -253,7 +253,7 @@ changelog_handle_probe (rpcsvc_request_t *req) /* ->xl hidden in rpcsvc */ svc = rpcsvc_request_service (req); - this = svc->mydata; + this = svc->xl; priv = this->private; c_clnt = &priv->connections; diff --git a/xlators/features/quota/src/quotad-helpers.c b/xlators/features/quota/src/quotad-helpers.c index 9a98ab17a24..70298fc87f5 100644 --- a/xlators/features/quota/src/quotad-helpers.c +++ b/xlators/features/quota/src/quotad-helpers.c @@ -62,7 +62,7 @@ quotad_aggregator_alloc_frame (rpcsvc_request_t *req) GF_VALIDATE_OR_GOTO ("server", req->svc, out); GF_VALIDATE_OR_GOTO ("server", req->svc->ctx, out); - this = req->svc->mydata; + this = req->svc->xl; frame = create_frame (this, req->svc->ctx->pool); if (!frame) diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c index ecb80b9aece..556b0e21e7f 100644 --- a/xlators/protocol/server/src/server-handshake.c +++ b/xlators/protocol/server/src/server-handshake.c @@ -257,7 +257,7 @@ server_getspec (rpcsvc_request_t *req) gf_getspec_req args = {0,}; gf_getspec_rsp rsp = {0,}; - this = req->svc->mydata; + this = req->svc->xl; conf = this->private; ret = xdr_to_generic (req->msg[0], &args, (xdrproc_t)xdr_gf_getspec_req); @@ -367,7 +367,7 @@ server_setvolume (rpcsvc_request_t *req) goto fail; } - this = req->svc->mydata; + this = req->svc->xl; config_params = dict_copy_with_ref (this->options, NULL); conf = this->private; @@ -741,7 +741,7 @@ server_set_lk_version (rpcsvc_request_t *req) server_ctx_t *serv_ctx = NULL; xlator_t *this = NULL; - this = req->svc->mydata; + this = req->svc->xl; //TODO: Decide on an appropriate errno for the error-path //below if (!this) -- cgit