summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHarshavardhana <harsha@harshavardhana.net>2013-12-24 08:23:13 -0800
committerVijay Bellur <vbellur@redhat.com>2014-01-08 20:42:15 -0800
commit2b05c1588ac60af26e1b16f9f27ef8d5e4e50a5f (patch)
treecff1a430e5c8eed85789ff7d87324a5bfb50709d
parent88816bf4b2933da8fa2717cb0e25c521895da4e1 (diff)
rpc/auth: Avoid NULL dereference in rpcsvc_auth_request_init()
Code section is bogus! ------------------------------------------ 370: if (!auth->authops->request_init) 371: ret = auth->authops->request_init (req, auth->authprivate); ------------------------------------------ Seems to have been never been used historically since logically above code has never been true to actually execute "authops->request_init() --> auth_glusterfs_{v2,}_request_init()" On top of that under "rpcsvc_request_init()" verf.flavour and verf.datalen are initialized from what is provided through 'callmsg'. ------------------------------------------ req->verf.flavour = rpc_call_verf_flavour (callmsg); req->verf.datalen = rpc_call_verf_len (callmsg); /* AUTH */ rpcsvc_auth_request_init (req); return req; ------------------------------------------ So the code in 'auth_glusterfs_{v2,}_request_init()' performing this operation will over-write the original flavour and datalen. ------------------------------------------ if (!req) return -1; memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES); req->verf.datalen = 0; req->verf.flavour = AUTH_NULL; ------------------------------------------ Refactoring the whole code into a more understandable version and also avoiding a potential NULL dereference Change-Id: I1a430fcb4d26de8de219bd0cb3c46c141649d47d BUG: 1049735 Signed-off-by: Harshavardhana <harsha@harshavardhana.net> Reviewed-on: http://review.gluster.org/6591 Reviewed-by: Santosh Pradhan <spradhan@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--rpc/rpc-lib/src/auth-glusterfs.c12
-rw-r--r--rpc/rpc-lib/src/auth-null.c9
-rw-r--r--rpc/rpc-lib/src/auth-unix.c6
-rw-r--r--rpc/rpc-lib/src/rpcsvc-auth.c33
-rw-r--r--rpc/rpc-lib/src/rpcsvc.c8
-rw-r--r--rpc/rpc-lib/src/rpcsvc.h2
6 files changed, 24 insertions, 46 deletions
diff --git a/rpc/rpc-lib/src/auth-glusterfs.c b/rpc/rpc-lib/src/auth-glusterfs.c
index db488434c98..48871ffb350 100644
--- a/rpc/rpc-lib/src/auth-glusterfs.c
+++ b/rpc/rpc-lib/src/auth-glusterfs.c
@@ -50,12 +50,6 @@ ret:
int
auth_glusterfs_request_init (rpcsvc_request_t *req, void *priv)
{
- if (!req)
- return -1;
- memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
- req->verf.datalen = 0;
- req->verf.flavour = AUTH_NULL;
-
return 0;
}
@@ -172,12 +166,6 @@ ret:
int
auth_glusterfs_v2_request_init (rpcsvc_request_t *req, void *priv)
{
- if (!req)
- return -1;
- memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
- req->verf.datalen = 0;
- req->verf.flavour = AUTH_NULL;
-
return 0;
}
diff --git a/rpc/rpc-lib/src/auth-null.c b/rpc/rpc-lib/src/auth-null.c
index ebdcc8ff8e7..b030341abb4 100644
--- a/rpc/rpc-lib/src/auth-null.c
+++ b/rpc/rpc-lib/src/auth-null.c
@@ -22,15 +22,6 @@
int
auth_null_request_init (rpcsvc_request_t *req, void *priv)
{
- if (!req)
- return -1;
-
- memset (req->cred.authdata, 0, GF_MAX_AUTH_BYTES);
- req->cred.datalen = 0;
-
- memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
- req->verf.datalen = 0;
-
return 0;
}
diff --git a/rpc/rpc-lib/src/auth-unix.c b/rpc/rpc-lib/src/auth-unix.c
index fa5f0576e31..27351f66911 100644
--- a/rpc/rpc-lib/src/auth-unix.c
+++ b/rpc/rpc-lib/src/auth-unix.c
@@ -24,12 +24,6 @@
int
auth_unix_request_init (rpcsvc_request_t *req, void *priv)
{
- if (!req)
- return -1;
- memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
- req->verf.datalen = 0;
- req->verf.flavour = AUTH_NULL;
-
return 0;
}
diff --git a/rpc/rpc-lib/src/rpcsvc-auth.c b/rpc/rpc-lib/src/rpcsvc-auth.c
index 0ede19f741b..384e4a75db2 100644
--- a/rpc/rpc-lib/src/rpcsvc-auth.c
+++ b/rpc/rpc-lib/src/rpcsvc-auth.c
@@ -369,25 +369,36 @@ ret:
int
-rpcsvc_auth_request_init (rpcsvc_request_t *req)
+rpcsvc_auth_request_init (rpcsvc_request_t *req, struct rpc_msg *callmsg)
{
- int ret = -1;
+ int32_t ret = 0;
rpcsvc_auth_t *auth = NULL;
- if (!req)
- return -1;
+ if (!req || !callmsg) {
+ ret = -1;
+ goto err;
+ }
+
+ req->cred.flavour = rpc_call_cred_flavour (callmsg);
+ req->cred.datalen = rpc_call_cred_len (callmsg);
+ req->verf.flavour = rpc_call_verf_flavour (callmsg);
+ req->verf.datalen = rpc_call_verf_len (callmsg);
auth = rpcsvc_auth_get_handler (req);
- if (!auth)
+ if (!auth) {
+ ret = -1;
goto err;
- ret = 0;
+ }
+
gf_log (GF_RPCSVC, GF_LOG_TRACE, "Auth handler: %s", auth->authname);
- if (!auth->authops->request_init)
- ret = auth->authops->request_init (req, auth->authprivate);
- req->auxgids = req->auxgidsmall; /* reset to auxgidlarge during
- unsersialize if necessary */
- req->auxgidlarge = NULL;
+ if (auth->authops->request_init)
+ ret = auth->authops->request_init (req, auth->authprivate);
+
+ /* reset to auxgidlarge during
+ unsersialize if necessary */
+ req->auxgids = req->auxgidsmall;
+ req->auxgidlarge = NULL;
err:
return ret;
}
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index d19a3ca0c3b..69db8b70b59 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -367,13 +367,7 @@ rpcsvc_request_init (rpcsvc_t *svc, rpc_transport_t *trans,
* been copied into the required sections of the req structure,
* we just need to fill in the meta-data about it now.
*/
- req->cred.flavour = rpc_call_cred_flavour (callmsg);
- req->cred.datalen = rpc_call_cred_len (callmsg);
- req->verf.flavour = rpc_call_verf_flavour (callmsg);
- req->verf.datalen = rpc_call_verf_len (callmsg);
-
- /* AUTH */
- rpcsvc_auth_request_init (req);
+ rpcsvc_auth_request_init (req, callmsg);
return req;
}
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index 28ec93e11a5..30a969b11c0 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -553,7 +553,7 @@ struct rpcsvc_auth_list {
};
extern int
-rpcsvc_auth_request_init (rpcsvc_request_t *req);
+rpcsvc_auth_request_init (rpcsvc_request_t *req, struct rpc_msg *callmsg);
extern int
rpcsvc_auth_init (rpcsvc_t *svc, dict_t *options);