From ce15b9fc8930edb034938afd972d1f2e3fd1974c Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 8 Sep 2022 17:42:45 +0000 Subject: libbinder: fix buffer free race Well, so the race is: - client sends a large transaction (buffer A) - server processes result - server sends reply (1) - client gets reply - client sends another large transaction (buffer B) - transaction fails, not enough space - server frees buffer A (2) This CL moves (2) to happen before (1). We set the Parcel size to 0, which has the effect of freeing data, before the destructor runs. Test: binderLibTest Test: binderLibTest --gtest_filter="*Garg*" --gtest_repeat=1000 --gtest_break_on_failure Fixes: 238777741 Change-Id: Ic223a98c55904bb3f77ca13729cdf24a992cef1e --- libs/binder/IPCThreadState.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'libs/binder/IPCThreadState.cpp') diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index b50cfb3d19..bfcf39ad30 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -1318,6 +1318,13 @@ status_t IPCThreadState::executeCommand(int32_t cmd) LOG_ONEWAY("Sending reply to %d!", mCallingPid); if (error < NO_ERROR) reply.setError(error); + // b/238777741: clear buffer before we send the reply. + // Otherwise, there is a race where the client may + // receive the reply and send another transaction + // here and the space used by this transaction won't + // be freed for the client. + buffer.setDataSize(0); + constexpr uint32_t kForwardReplyFlags = TF_CLEAR_BUF; sendReply(reply, (tr.flags & kForwardReplyFlags)); } else { -- cgit v1.2.3-59-g8ed1b