From 5fb00398ef02bf453195a0bc50f5df95788557c7 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Tue, 31 May 2011 08:28:51 +0100 Subject: [PATCH] vos: Don't leak/overflow bulkaddrs The vos listaddrs command repeatedly reuses a bulkaddrs array. It zeros it once (without freeing the allocated memory), and then repeatedly uses it without zeroing in a loop. This means that the XDR library assumes that a sufficiently large block is already allocated, doesn't reallocate for the incoming data, or check limits. This means that if the first call to VL_GetAddrsU returns a set of addresses smaller than subsequent calls, we'll write past the end of the array, causing memory corruption. Fix this by freeing the arrays correctly with each pass of the call. Reviewed-on: http://gerrit.openafs.org/4756 Reviewed-by: Jeffrey Altman Tested-by: BuildBot Reviewed-by: Derrick Brashear (cherry picked from commit b6add117ad210665a811213fe17a30fabbda3a3c) Change-Id: Ic3ae8f506e87d18fdc121ff21221f61c359e38aa Reviewed-on: http://gerrit.openafs.org/6302 Tested-by: Derrick Brashear Reviewed-by: Derrick Brashear --- src/volser/vos.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/volser/vos.c b/src/volser/vos.c index f9f14b11b..3ee98e4ee 100644 --- a/src/volser/vos.c +++ b/src/volser/vos.c @@ -5317,7 +5317,6 @@ ListAddrs(struct cmd_syndesc *as, void *arock) memset(&m_attrs, 0, sizeof(struct ListAddrByAttributes)); m_attrs.Mask = VLADDR_INDEX; - memset(&m_addrs, 0, sizeof(bulkaddrs)); memset(&askuuid, 0, sizeof(afsUUID)); if (as->parms[0].items) { /* -uuid */ @@ -5347,8 +5346,8 @@ ListAddrs(struct cmd_syndesc *as, void *arock) printuuid = 1; } - m_addrs.bulkaddrs_val = 0; - m_addrs.bulkaddrs_len = 0; + memset(&m_addrs, 0, sizeof(bulkaddrs)); + memset(&vlcb, 0, sizeof(struct VLCallBack)); vcode = ubik_VL_GetAddrs(cstruct, UBIK_CALL_NEW, 0, 0, &vlcb, &nentries, @@ -5356,16 +5355,15 @@ ListAddrs(struct cmd_syndesc *as, void *arock) if (vcode) { fprintf(STDERR, "vos: could not list the server addresses\n"); PrintError("", vcode); - return (vcode); + goto out; } m_nentries = 0; - m_addrs.bulkaddrs_val = 0; - m_addrs.bulkaddrs_len = 0; i = 1; while (1) { m_attrs.index = i; + xdr_free((xdrproc_t)xdr_bulkaddrs, &m_addrs); /* reset addr list */ vcode = ubik_VL_GetAddrsU(cstruct, UBIK_CALL_NEW, &m_attrs, &m_uuid, &m_uniq, &m_nentries, &m_addrs); @@ -5387,23 +5385,26 @@ ListAddrs(struct cmd_syndesc *as, void *arock) } if (vcode == VL_INDEXERANGE) { - break; + vcode = 0; /* not an error, just means we're done */ + goto out; } if (vcode) { fprintf(STDERR, "vos: could not list the server addresses\n"); PrintError("", vcode); - return (vcode); + goto out; } print_addrs(&m_addrs, &m_uuid, m_nentries, printuuid); i++; if ((as->parms[1].items) || (as->parms[0].items) || (i > nentries)) - break; + goto out; } - return 0; +out: + xdr_free((xdrproc_t)xdr_bulkaddrs, &m_addrs); + return vcode; } -- 2.39.5