joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Only use source addresses in handlePacket

Browse source 
MulticastPacketReader uses an InetSocketAddress as a buffer that gets
filled for each incoming packet for the source address. Similarly to the
receive buffer, it should not be used outside of the handlePacket
method, as it will be modified on the handler thread when further
packets are received.

Fix the current usage in MdnsInterfaceAdvertiser/MdnsRecordRepository.
Fortunately due to a bug packets are never replied unicast, so today the
MdnsRecordRepository code using the source address never triggers, even
though it may be circulating an incorrect source address instance that
changes over time.

Test: atest
Change-Id: I4cadfa6e54d1b37d9a6b0f8ca0269193a229bc51
This commit is contained in:
Remi NGUYEN VAN 2023-12-28 18:01:52 +09:00
commit 7078ac59b9
3 changed files with 17 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

6
service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java
View file

@ -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);

6
service-t/src/com/android/server/connectivity/mdns/MulticastPacketReader.java
View file

@ -42,6 +42,12 @@ public class MulticastPacketReader extends FdEventsReader<MulticastPacketReader.
private final Set<PacketHandler> mPacketHandlers = MdnsUtils.newSet();
interface PacketHandler {
/**
* Handle an incoming packet.
*
* The recvbuf and src <b>will be reused and modified</b> after this method returns, so
* implementers must ensure that they are not accessed after handlePacket returns.
*/
void handlePacket(byte[] recvbuf, int length, InetSocketAddress src);
}

8
tests/unit/java/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiserTest.kt
View file

@ -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)