From 2b05c1588ac60af26e1b16f9f27ef8d5e4e50a5f Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 24 Dec 2013 08:23:13 -0800 Subject: 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 Reviewed-on: http://review.gluster.org/6591 Reviewed-by: Santosh Pradhan Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- rpc/rpc-lib/src/rpcsvc-auth.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) (limited to 'rpc/rpc-lib/src/rpcsvc-auth.c') diff --git a/rpc/rpc-lib/src/rpcsvc-auth.c b/rpc/rpc-lib/src/rpcsvc-auth.c index 0ede19f74..384e4a75d 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; } -- cgit