From 5c59f5fdb33b70d032e1b6578c9fb583b97070ca Mon Sep 17 00:00:00 2001
From: Nicholaz Beresford
Date: Tue, 16 Sep 2008 21:49:15 -0500
Subject: VWR-3877: A nasty possible memory overwrite and two minor leaks.

---
 linden/indra/llmessage/llassetstorage.cpp | 41 ++++++++++++++++---------------
 1 file changed, 21 insertions(+), 20 deletions(-)

(limited to 'linden/indra/llmessage/llassetstorage.cpp')

diff --git a/linden/indra/llmessage/llassetstorage.cpp b/linden/indra/llmessage/llassetstorage.cpp
index a6077e5..83026ba 100644
--- a/linden/indra/llmessage/llassetstorage.cpp
+++ b/linden/indra/llmessage/llassetstorage.cpp
@@ -515,16 +515,19 @@ void LLAssetStorage::downloadCompleteCallback(
 	S32 result,
 	const LLUUID& file_id,
 	LLAssetType::EType file_type,
-	void* user_data, LLExtStat ext_status)
+	void* callback_parm_req, LLExtStat ext_status)
 {
 	lldebugs << "LLAssetStorage::downloadCompleteCallback() for " << file_id
 		 << "," << LLAssetType::lookup(file_type) << llendl;
-	LLAssetRequest* req = (LLAssetRequest*)user_data;
+
+	// be careful! req may be a ptr to memory already freed (a timeout does this)
+	LLAssetRequest* req = (LLAssetRequest*)callback_parm_req;	
 	if(!req)
 	{
 		llwarns << "LLAssetStorage::downloadCompleteCallback called without"
 			"a valid request." << llendl;
-		return;
+		// we can live with a null pointer, we're not allowed to deref the ptr anyway (see above)
+		// return;  
 	}
 	if (!gAssetStorage)
 	{
@@ -532,12 +535,10 @@ void LLAssetStorage::downloadCompleteCallback(
 		return;
 	}
 
-	req->setUUID(file_id);
-	req->setType(file_type);
 	if (LL_ERR_NOERR == result)
 	{
 		// we might have gotten a zero-size file
-		LLVFile vfile(gAssetStorage->mVFS, req->getUUID(), req->getType());
+		LLVFile vfile(gAssetStorage->mVFS, file_id, file_type);
 		if (vfile.getSize() <= 0)
 		{
 			llwarns << "downloadCompleteCallback has non-existent or zero-size asset " << req->getUUID() << llendl;
@@ -556,7 +557,7 @@ void LLAssetStorage::downloadCompleteCallback(
 	{
 		request_list_t::iterator curiter = iter++;
 		LLAssetRequest* tmp = *curiter;
-		if ((tmp->getUUID() == req->getUUID()) && (tmp->getType()== req->getType()))
+		if ((tmp->getUUID() == file_id) && (tmp->getType() == file_type))
 		{
 			requests.push_front(tmp);
 			iter = gAssetStorage->mPendingDownloads.erase(curiter);
@@ -569,7 +570,7 @@ void LLAssetStorage::downloadCompleteCallback(
 		LLAssetRequest* tmp = *curiter;
 		if (tmp->mDownCallback)
 		{
-			tmp->mDownCallback(gAssetStorage->mVFS, req->getUUID(), req->getType(), tmp->mUserData, result, ext_status);
+			tmp->mDownCallback(gAssetStorage->mVFS, tmp->getUUID(), tmp->getType(), tmp->mUserData, result, ext_status);
 		}
 		delete tmp;
 	}
@@ -665,10 +666,10 @@ void LLAssetStorage::downloadEstateAssetCompleteCallback(
 	S32 result,
 	const LLUUID& file_id,
 	LLAssetType::EType file_type,
-	void* user_data,
+	void* callback_parm_req,
 	LLExtStat ext_status)
 {
-	LLEstateAssetRequest *req = (LLEstateAssetRequest*)user_data;
+	LLEstateAssetRequest *req = (LLEstateAssetRequest*)callback_parm_req;
 	if(!req)
 	{
 		llwarns << "LLAssetStorage::downloadEstateAssetCompleteCallback called"
@@ -682,12 +683,10 @@ void LLAssetStorage::downloadEstateAssetCompleteCallback(
 		return;
 	}
 
-	req->setUUID(file_id);
-	req->setType(file_type);
 	if (LL_ERR_NOERR == result)
 	{
 		// we might have gotten a zero-size file
-		LLVFile vfile(gAssetStorage->mVFS, req->getUUID(), req->getAType());
+		LLVFile vfile(gAssetStorage->mVFS, file_id, file_type);
 		if (vfile.getSize() <= 0)
 		{
 			llwarns << "downloadCompleteCallback has non-existent or zero-size asset!" << llendl;
@@ -697,7 +696,9 @@ void LLAssetStorage::downloadEstateAssetCompleteCallback(
 		}
 	}
 
-	req->mDownCallback(gAssetStorage->mVFS, req->getUUID(), req->getAType(), req->mUserData, result, ext_status);
+	req->mDownCallback(gAssetStorage->mVFS, file_id, file_type, req->mUserData, result, ext_status);
+
+	delete req;
 }
 
 void LLAssetStorage::getInvItemAsset(const LLHost &object_sim, const LLUUID &agent_id, const LLUUID &session_id,
@@ -802,10 +803,10 @@ void LLAssetStorage::downloadInvItemCompleteCallback(
 	S32 result,
 	const LLUUID& file_id,
 	LLAssetType::EType file_type,
-	void* user_data,
+	void* callback_parm_req,
 	LLExtStat ext_status)
 {
-	LLInvItemRequest *req = (LLInvItemRequest*)user_data;
+	LLInvItemRequest *req = (LLInvItemRequest*)callback_parm_req;
 	if(!req)
 	{
 		llwarns << "LLAssetStorage::downloadEstateAssetCompleteCallback called"
@@ -818,12 +819,10 @@ void LLAssetStorage::downloadInvItemCompleteCallback(
 		return;
 	}
 
-	req->setUUID(file_id);
-	req->setType(file_type);
 	if (LL_ERR_NOERR == result)
 	{
 		// we might have gotten a zero-size file
-		LLVFile vfile(gAssetStorage->mVFS, req->getUUID(), req->getType());
+		LLVFile vfile(gAssetStorage->mVFS, file_id, file_type);
 		if (vfile.getSize() <= 0)
 		{
 			llwarns << "downloadCompleteCallback has non-existent or zero-size asset!" << llendl;
@@ -833,7 +832,9 @@ void LLAssetStorage::downloadInvItemCompleteCallback(
 		}
 	}
 
-	req->mDownCallback(gAssetStorage->mVFS, req->getUUID(), req->getType(), req->mUserData, result, ext_status);
+	req->mDownCallback(gAssetStorage->mVFS, file_id, file_type, req->mUserData, result, ext_status);
+
+	delete req;
 }
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-- 
cgit v1.1