From 60a4e9d2958b8d39892c045a6defec9eaf947f2c Mon Sep 17 00:00:00 2001 From: Gohulan Balachandran Date: Fri, 20 Oct 2017 09:37:52 -0700 Subject: [PATCH] libril: Fix double freeing of memory in SAP 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 | 11 ++++++++--- ril/libril/sap_service.cpp | 8 +++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ril/libril/RilSapSocket.cpp b/ril/libril/RilSapSocket.cpp index f58d327b..8276de9f 100644 --- a/ril/libril/RilSapSocket.cpp +++ b/ril/libril/RilSapSocket.cpp @@ -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; diff --git a/ril/libril/sap_service.cpp b/ril/libril/sap_service.cpp index abfbfefe..962d564d 100644 --- a/ril/libril/sap_service.cpp +++ b/ril/libril/sap_service.cpp @@ -106,11 +106,13 @@ MsgHeader* SapImpl::createMsgHeader(MsgId msgId, int32_t token) { Return 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 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);