summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNiels de Vos <ndevos@redhat.com>2015-07-29 09:51:45 +0200
committerNiels de Vos <ndevos@redhat.com>2015-07-29 07:14:41 -0700
commitcc2ebbd7f5a043cb953521bb9d65ddc3235cae43 (patch)
tree15d1716fae28aa775e279b0bde3447698053b1f1
parent382a6c00260d2a81cd596ca454768c78d61a14ee (diff)
rpc: fix concurrency bug in gf_authenticate
The basic problem is that gf_authenticate abused global variables to pass info through dict_foreach. This is not thread-safe, but it wasn't affecting most people until multi-threaded epoll came along. Now, if two threads get into this code at the same time - and there's nothing to prevent it - one of them could free one of the dicts involved while the other is still using it. The fix is to pass this same information using a temporary structure instead of globals. This makes the code smaller and more efficient too. Cherry picked from commit 8f04ec33bc86aa464a5f8b77f9d64e5608cb6f1b: > Change-Id: I72cccc440bb40d5f7ff695250dd014762c7efb73 > BUG: 1247765 > Signed-off-by: Jeff Darcy <jdarcy@redhat.com> > Reviewed-on: http://review.gluster.org/11780 > Tested-by: NetBSD Build System <jenkins@build.gluster.org> > Reviewed-by: Niels de Vos <ndevos@redhat.com> > Tested-by: Gluster Build System <jenkins@build.gluster.com> > Reviewed-by: Raghavendra G <rgowdapp@redhat.com> BUG: 1247850 Change-Id: I151dad436b859c64985421394f3dea572723c2aa Signed-off-by: Niels de Vos <ndevos@redhat.com> Reviewed-on: http://review.gluster.org/11785 Tested-by: NetBSD Build System <jenkins@build.gluster.org> Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
-rw-r--r--xlators/protocol/server/src/authenticate.c78
1 files changed, 31 insertions, 47 deletions
diff --git a/xlators/protocol/server/src/authenticate.c b/xlators/protocol/server/src/authenticate.c
index 8b45c41a59b..45bc656865a 100644
--- a/xlators/protocol/server/src/authenticate.c
+++ b/xlators/protocol/server/src/authenticate.c
@@ -167,72 +167,57 @@ out:
return ret;
}
-static dict_t *__input_params;
-static dict_t *__config_params;
+typedef struct {
+ dict_t *iparams;
+ dict_t *cparams;
+ int64_t result;
+} gf_auth_args_t;
-int
-map (dict_t *this, char *key, data_t *value, void *data)
+static int
+gf_auth_one_method (dict_t *this, char *key, data_t *value, void *data)
{
- dict_t *res = data;
- auth_fn_t authenticate;
- auth_handle_t *handle = NULL;
+ gf_auth_args_t *args = data;
+ auth_handle_t *handle = NULL;
- if (value && (handle = data_to_ptr (value)) &&
- (authenticate = handle->authenticate)) {
- dict_set (res, key,
- int_to_data (authenticate (__input_params,
- __config_params)));
- } else {
- dict_set (res, key, int_to_data (AUTH_DONT_CARE));
+ if (!value) {
+ return 0;
}
- return 0;
-}
-int
-reduce (dict_t *this, char *key, data_t *value, void *data)
-{
- int64_t val = 0;
- int64_t *res = data;
- if (!data)
+ handle = data_to_ptr (value);
+ if (!handle || !handle->authenticate) {
return 0;
+ }
- val = data_to_int64 (value);
- switch (val)
- {
+ switch (handle->authenticate (args->iparams, args->cparams)) {
case AUTH_ACCEPT:
- if (AUTH_DONT_CARE == *res)
- *res = AUTH_ACCEPT;
- break;
-
+ if (args->result != AUTH_REJECT) {
+ args->result = AUTH_ACCEPT;
+ }
+ /* FALLTHROUGH */
+ default:
+ return 0;
case AUTH_REJECT:
- *res = AUTH_REJECT;
- break;
-
- case AUTH_DONT_CARE:
- break;
+ args->result = AUTH_REJECT;
+ return -1;
}
- return 0;
}
-
auth_result_t
gf_authenticate (dict_t *input_params,
dict_t *config_params,
dict_t *auth_modules)
{
char *name = NULL;
- dict_t *results = NULL;
- int64_t result = AUTH_DONT_CARE;
data_t *peerinfo_data = NULL;
+ gf_auth_args_t args;
- results = get_new_dict ();
- __input_params = input_params;
- __config_params = config_params;
+ args.iparams = input_params;
+ args.cparams = config_params;
+ args.result = AUTH_DONT_CARE;
- dict_foreach (auth_modules, map, results);
+ dict_foreach (auth_modules, gf_auth_one_method, &args);
- dict_foreach (results, reduce, &result);
- if (AUTH_DONT_CARE == result) {
+ if (AUTH_DONT_CARE == args.result) {
peerinfo_data = dict_get (input_params, "peer-info-name");
if (peerinfo_data) {
@@ -242,11 +227,10 @@ gf_authenticate (dict_t *input_params,
gf_msg ("auth", GF_LOG_ERROR, 0, PS_MSG_REMOTE_CLIENT_REFUSED,
"no authentication module is interested in "
"accepting remote-client %s", name);
- result = AUTH_REJECT;
+ args.result = AUTH_REJECT;
}
- dict_destroy (results);
- return result;
+ return args.result;
}
void