VWR-3877: A nasty possible memory overwrite and two minor leaks.

2 files changed, 25 insertions, 20 deletions

diff --git a/ChangeLog.txt b/ChangeLog.txt
index e69de29..e06910f 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -0,0 +1,4 @@
+2008-09-16  Nicholaz Beresford  <nicholaz@blueflash.cc>
+
+        * linden/indra/llmessage/llassetstorage.cpp:
+        VWR-3877: A nasty possible memory overwrite and two minor leaks.
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;
 }
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////