diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java index aa40c9227d..3a04dcdb49 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java @@ -373,12 +373,14 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand } return; } + // recvbuf and src are reused after this returns; ensure references to src are not kept. + final InetSocketAddress srcCopy = new InetSocketAddress(src.getAddress(), src.getPort()); if (DBG) { mSharedLog.v("Parsed packet with " + packet.questions.size() + " questions, " + packet.answers.size() + " answers, " + packet.authorityRecords.size() + " authority, " - + packet.additionalRecords.size() + " additional from " + src); + + packet.additionalRecords.size() + " additional from " + srcCopy); } for (int conflictServiceId : mRecordRepository.getConflictingServices(packet)) { @@ -389,7 +391,7 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand // happen when the incoming packet has answer records (not a question), so there will be no // answer. One exception is simultaneous probe tiebreaking (rfc6762 8.2), in which case the // conflicting service is still probing and won't reply either. - final MdnsReplyInfo answers = mRecordRepository.getReply(packet, src); + final MdnsReplyInfo answers = mRecordRepository.getReply(packet, srcCopy); if (answers == null) return; mReplySender.queueReply(answers); diff --git a/service-t/src/com/android/server/connectivity/mdns/MulticastPacketReader.java b/service-t/src/com/android/server/connectivity/mdns/MulticastPacketReader.java index 3cd77a4610..70451f3d55 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MulticastPacketReader.java +++ b/service-t/src/com/android/server/connectivity/mdns/MulticastPacketReader.java @@ -42,6 +42,12 @@ public class MulticastPacketReader extends FdEventsReader mPacketHandlers = MdnsUtils.newSet(); interface PacketHandler { + /** + * Handle an incoming packet. + * + * The recvbuf and src will be reused and modified after this method returns, so + * implementers must ensure that they are not accessed after handlePacket returns. + */ void handlePacket(byte[] recvbuf, int length, InetSocketAddress src); } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt index 0c04bff0d5..ee0bd1a5db 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt @@ -35,6 +35,7 @@ import com.android.testutils.waitForIdle import java.net.InetSocketAddress import kotlin.test.assertContentEquals import kotlin.test.assertEquals +import kotlin.test.assertNotSame import kotlin.test.assertTrue import org.junit.After import org.junit.Before @@ -213,7 +214,12 @@ class MdnsInterfaceAdvertiserTest { packetHandler.handlePacket(query, query.size, src) val packetCaptor = ArgumentCaptor.forClass(MdnsPacket::class.java) - verify(repository).getReply(packetCaptor.capture(), eq(src)) + val srcCaptor = ArgumentCaptor.forClass(InetSocketAddress::class.java) + verify(repository).getReply(packetCaptor.capture(), srcCaptor.capture()) + + assertEquals(src, srcCaptor.value) + assertNotSame(src, srcCaptor.value, "src will be reused by the packetHandler, references " + + "to it should not be used outside of handlePacket.") packetCaptor.value.let { assertEquals(1, it.questions.size)