libril: Fix double freeing of memory in SAP
authorGohulan Balachandran <gohulanb@quicinc.com>
Fri, 20 Oct 2017 16:37:52 +0000 (09:37 -0700)
committerStricted <info@stricted.net>
Wed, 24 Oct 2018 01:49:09 +0000 (03:49 +0200)
 service and add null-checks.

The payload of a SAP request could be freed twice in certain scenarios.
Also, add null-checks to prevent dereferencing of null pointers.

Bug: 64729356
Test: Manually run the fuzz tests and ensure that there is no crash in
      rild

Change-Id: Ib7ae269fa5297d6acea267337b220b8858c82bae

ril/libril/RilSapSocket.cpp
ril/libril/sap_service.cpp

index f58d327b7cab388881a2411e87ec2d3004c4ac34..8276de9f4882b81745210283b2e826d6bc683688 100644 (file)
@@ -55,10 +55,9 @@ void RilSapSocket::sOnRequestComplete (RIL_Token t,
         sap_socket->onRequestComplete(t,e,response,responselen);
     } else {
         RLOGE("Invalid socket id");
-        if (request->curr->payload) {
-            free(request->curr->payload);
+        if (request->curr) {
+            free(request->curr);
         }
-        free(request->curr);
         free(request);
     }
 }
@@ -234,6 +233,12 @@ void RilSapSocket::dispatchRequest(MsgHeader *req) {
 void RilSapSocket::onRequestComplete(RIL_Token t, RIL_Errno e, void *response,
         size_t response_len) {
     SapSocketRequest* request= (SapSocketRequest*)t;
+
+    if (!request || !request->curr) {
+        RLOGE("RilSapSocket::onRequestComplete: request/request->curr is NULL");
+        return;
+    }
+
     MsgHeader *hdr = request->curr;
 
     MsgHeader rsp;
index abfbfefefe8c22428ca26507a36e1587281d086a..962d564db5806041cda3a89c7d4a03d516907864 100644 (file)
@@ -106,11 +106,13 @@ MsgHeader* SapImpl::createMsgHeader(MsgId msgId, int32_t token) {
 
 Return<void> SapImpl::addPayloadAndDispatchRequest(MsgHeader *msg, uint16_t reqLen,
         uint8_t *reqPtr) {
-    msg->payload = (pb_bytes_array_t *)malloc(sizeof(pb_bytes_array_t) - 1 + reqLen);
-    if (msg->payload == NULL) {
+    pb_bytes_array_t *payload = (pb_bytes_array_t *) malloc(sizeof(pb_bytes_array_t) - 1 + reqLen);
+    if (payload == NULL) {
         sendFailedResponse(msg->id, msg->token, 2, reqPtr, msg);
         return Void();
     }
+
+    msg->payload = payload;
     msg->payload->size = reqLen;
     memcpy(msg->payload->bytes, reqPtr, reqLen);
 
@@ -120,7 +122,7 @@ Return<void> SapImpl::addPayloadAndDispatchRequest(MsgHeader *msg, uint16_t reqL
         sapSocket->dispatchRequest(msg);
     } else {
         RLOGE("SapImpl::addPayloadAndDispatchRequest: sapSocket is null");
-        sendFailedResponse(msg->id, msg->token, 3, msg->payload, reqPtr, msg);
+        sendFailedResponse(msg->id, msg->token, 3, payload, reqPtr, msg);
         return Void();
     }
     free(msg->payload);