From 9f699ccd42712e5b991bd33319caf1e5c902f894 Mon Sep 17 00:00:00 2001 From: Santosh Kumar Pradhan Date: Mon, 3 Feb 2014 15:12:22 +0530 Subject: gNFS: Possible NULL pointer dereference In NFS-ACL code (acl3.c) i.e. acl3svc_setacl(), contol can go to "acl3err" block from setaclargs.mask validation or acl3_validate_gluster_fh() and acl3_map_fh_to_volume() macros. But at this point of time "cs" is yet to be init'd (the macro acl3_handle_call_state_init() is not yet invoked) which can cause a NULL ptr deref. FIX: Refactor the acl3 code. Coverity ID (CID): 1124491 Change-Id: I3aca38770e03ce59d1705653b6d8349e6cc153b2 BUG: 789278 Signed-off-by: Santosh Kumar Pradhan Reviewed-on: http://review.gluster.org/6890 Reviewed-by: Rajesh Joseph Reviewed-by: Niels de Vos Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/nfs/server/src/acl3.c | 69 +++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/xlators/nfs/server/src/acl3.c b/xlators/nfs/server/src/acl3.c index 25476ebbe..43156eb44 100644 --- a/xlators/nfs/server/src/acl3.c +++ b/xlators/nfs/server/src/acl3.c @@ -229,13 +229,21 @@ acl3svc_null (rpcsvc_request_t *req) } int -acl3_getacl_reply (nfs3_call_state_t *cs, getaclreply *reply) +acl3_getacl_reply (rpcsvc_request_t *req, getaclreply *reply) { - acl3svc_submit_reply (cs->req, (void *)reply, + acl3svc_submit_reply (req, (void *)reply, (acl3_serializer)xdr_serialize_getaclreply); return 0; } +int +acl3_setacl_reply (rpcsvc_request_t *req, setaclreply *reply) +{ + acl3svc_submit_reply (req, (void *)reply, + (acl3_serializer)xdr_serialize_setaclreply); + return 0; +} + int acl3_getacl_cbk (call_frame_t *frame, void *cookie, xlator_t *this, @@ -300,14 +308,14 @@ acl3_getacl_cbk (call_frame_t *frame, void *cookie, xlator_t *this, getaclreply->daclentry.daclentry_len = aclcount; } - acl3_getacl_reply (cs, getaclreply); + acl3_getacl_reply (cs->req, getaclreply); nfs3_call_state_wipe (cs); return 0; err: if (getaclreply) getaclreply->status = stat; - acl3_getacl_reply (cs, getaclreply); + acl3_getacl_reply (cs->req, getaclreply); nfs3_call_state_wipe (cs); return 0; } @@ -354,7 +362,7 @@ acl3_stat_cbk (call_frame_t *frame, void *cookie, xlator_t *this, return 0; err: getaclreply->status = stat; - acl3_getacl_reply (cs, getaclreply); + acl3_getacl_reply (cs->req, getaclreply); nfs3_call_state_wipe (cs); return 0; } @@ -382,7 +390,7 @@ acl3err: if (ret < 0) { gf_log (GF_ACL, GF_LOG_ERROR, "unable to open_and_resume"); cs->args.getaclreply.status = nfs3_errno_to_nfsstat3 (stat); - acl3_getacl_reply (cs, &cs->args.getaclreply); + acl3_getacl_reply (cs->req, &cs->args.getaclreply); nfs3_call_state_wipe (cs); } @@ -401,6 +409,7 @@ acl3svc_getacl (rpcsvc_request_t *req) nfsstat3 stat = NFS3ERR_SERVERFAULT; struct nfs3_fh fh, *fhp = NULL; getaclargs getaclargs; + getaclreply getaclreply; if (!req) return ret; @@ -408,6 +417,7 @@ acl3svc_getacl (rpcsvc_request_t *req) acl3_validate_nfs3_state (req, nfs3, stat, rpcerr, ret); nfs = nfs_state (nfs3->nfsx); memset (&getaclargs, 0, sizeof (getaclargs)); + memset (&getaclreply, 0, sizeof (getaclreply)); getaclargs.fh.n_bytes = (char *)&fh; if (xdr_to_getaclargs(req->msg[0], &getaclargs) <= 0) { gf_log (GF_ACL, GF_LOG_ERROR, "Error decoding args"); @@ -423,26 +433,23 @@ acl3svc_getacl (rpcsvc_request_t *req) fhp = &fh; acl3_validate_gluster_fh (&fh, stat, acl3err); - acl3_map_fh_to_volume (nfs->nfs3state, fhp, req, - vol, stat, acl3err); + acl3_map_fh_to_volume (nfs->nfs3state, fhp, req, vol, stat, acl3err); + acl3_volume_started_check (nfs3, vol, ret, rpcerr); acl3_handle_call_state_init (nfs->nfs3state, cs, req, - vol, stat, rpcerr); + vol, stat, acl3err); cs->vol = vol; cs->args.getaclreply.mask = getaclargs.mask; - acl3_volume_started_check (nfs3, vol, ret, acl3err); - ret = nfs3_fh_resolve_and_resume (cs, fhp, - NULL, acl3_getacl_resume); + ret = nfs3_fh_resolve_and_resume (cs, fhp, NULL, acl3_getacl_resume); + stat = nfs3_errno_to_nfsstat3 (-ret); acl3err: if (ret < 0) { gf_log (GF_ACL, GF_LOG_ERROR, "unable to resolve and resume"); - if (cs) { - cs->args.getaclreply.status = stat; - acl3_getacl_reply (cs, &cs->args.getaclreply); - nfs3_call_state_wipe (cs); - } + getaclreply.status = stat; + acl3_getacl_reply (req, &getaclreply); + nfs3_call_state_wipe (cs); return 0; } @@ -462,8 +469,8 @@ acl3_setacl_cbk (call_frame_t *frame, void *cookie, cs->args.setaclreply.status = status; } - acl3svc_submit_reply (cs->req, (void *)&cs->args.setaclreply, - (acl3_serializer)xdr_serialize_setaclreply); + acl3_setacl_reply (cs->req, &cs->args.setaclreply); + return 0; } @@ -500,8 +507,7 @@ acl3err: stat = -ret; gf_log (GF_ACL, GF_LOG_ERROR, "unable to open_and_resume"); cs->args.setaclreply.status = nfs3_errno_to_nfsstat3 (stat); - acl3svc_submit_reply (cs->req, (void *)&cs->args.setaclreply, - (acl3_serializer)xdr_serialize_setaclreply); + acl3_setacl_reply (cs->req, &cs->args.setaclreply); nfs3_call_state_wipe (cs); } @@ -521,8 +527,9 @@ acl3svc_setacl (rpcsvc_request_t *req) struct nfs3_fh fh; struct nfs3_fh *fhp = NULL; setaclargs setaclargs; + setaclreply setaclreply; + aclentry *daclentry = NULL; aclentry *aclentry = NULL; - struct aclentry *daclentry = NULL; int aclerrno = 0; int defacl = 1; @@ -542,6 +549,7 @@ acl3svc_setacl (rpcsvc_request_t *req) acl3_validate_nfs3_state (req, nfs3, stat, rpcerr, ret); nfs = nfs_state (nfs3->nfsx); memset (&setaclargs, 0, sizeof (setaclargs)); + memset (&setaclreply, 0, sizeof (setaclreply)); memset (&fh, 0, sizeof (fh)); setaclargs.fh.n_bytes = (char *)&fh; setaclargs.aclentry.aclentry_val = aclentry; @@ -560,14 +568,12 @@ acl3svc_setacl (rpcsvc_request_t *req) fhp = &fh; acl3_validate_gluster_fh (fhp, stat, acl3err); - acl3_map_fh_to_volume (nfs->nfs3state, fhp, req, - vol, stat, acl3err); + acl3_map_fh_to_volume (nfs->nfs3state, fhp, req, vol, stat, acl3err); + acl3_volume_started_check (nfs3, vol, ret, rpcerr); acl3_handle_call_state_init (nfs->nfs3state, cs, req, - vol, stat, rpcerr); + vol, stat, acl3err); cs->vol = vol; - acl3_volume_started_check (nfs3, vol, ret, rpcerr); - cs->aclcount = setaclargs.aclcount; cs->daclcount = setaclargs.daclcount; @@ -593,15 +599,14 @@ acl3svc_setacl (rpcsvc_request_t *req) goto acl3err; } - ret = nfs3_fh_resolve_and_resume (cs, fhp, - NULL, acl3_setacl_resume); + ret = nfs3_fh_resolve_and_resume (cs, fhp, NULL, acl3_setacl_resume); + stat = nfs3_errno_to_nfsstat3 (-ret); acl3err: if (ret < 0) { gf_log (GF_ACL, GF_LOG_ERROR, "unable to resolve and resume"); - cs->args.setaclreply.status = stat; - acl3svc_submit_reply (cs->req, (void *)&cs->args.setaclreply, - (acl3_serializer)xdr_serialize_setaclreply); + setaclreply.status = stat; + acl3_setacl_reply (req, &setaclreply); nfs3_call_state_wipe (cs); GF_FREE(aclentry); GF_FREE(daclentry); -- cgit