From c23ae79df596fc1a0a9bc1b9622ddc1a16638825 Mon Sep 17 00:00:00 2001
From: Luke Huang <huangluke@google.com>
Date: Tue, 11 Jun 2019 09:08:00 -0700
Subject: [PATCH] Fix race condition caused by fd reused for DnsResolver

There might be a gap between fd close and fd event listener unregister.
If fd is reused for another query during that gap, it might cause the
query failed with no response since addOnFileDescriptorEventListener
method failed. To fix this problem, we must ensure that fd event
listener is unregistered before fd closing.

Bug: 134310704
Test: atest DnsResolverTest

Merged-In: I443bb11b15845b079ee4370a7797e692e62fa3c8
(cherry picked from commit 07de4cf82ac09f8b9f37afa9eb1b7a44b43b6fe6)

Change-Id: I7041e67d8c906cbf88050e7d94245f8e15dcdbb4
---
 core/java/android/net/DnsResolver.java | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java
index 7a85dcbfc8..0b1a84534e 100644
--- a/core/java/android/net/DnsResolver.java
+++ b/core/java/android/net/DnsResolver.java
@@ -34,6 +34,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.os.CancellationSignal;
 import android.os.Looper;
+import android.os.MessageQueue;
 import android.system.ErrnoException;
 import android.util.Log;
 
@@ -466,10 +467,20 @@ public final class DnsResolver {
     private void registerFDListener(@NonNull Executor executor,
             @NonNull FileDescriptor queryfd, @NonNull Callback<? super byte[]> answerCallback,
             @Nullable CancellationSignal cancellationSignal, @NonNull Object lock) {
-        Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener(
+        final MessageQueue mainThreadMessageQueue = Looper.getMainLooper().getQueue();
+        mainThreadMessageQueue.addOnFileDescriptorEventListener(
                 queryfd,
                 FD_EVENTS,
                 (fd, events) -> {
+                    // b/134310704
+                    // Unregister fd event listener before resNetworkResult is called to prevent
+                    // race condition caused by fd reused.
+                    // For example when querying v4 and v6, it's possible that the first query ends
+                    // and the fd is closed before the second request starts, which might return
+                    // the same fd for the second request. By that time, the looper must have
+                    // unregistered the fd, otherwise another event listener can't be registered.
+                    mainThreadMessageQueue.removeOnFileDescriptorEventListener(fd);
+
                     executor.execute(() -> {
                         DnsResponse resp = null;
                         ErrnoException exception = null;
@@ -490,7 +501,11 @@ public final class DnsResolver {
                         }
                         answerCallback.onAnswer(resp.answerbuf, resp.rcode);
                     });
-                    // Unregister this fd listener
+
+                    // The file descriptor has already been unregistered, so it does not really
+                    // matter what is returned here. In spirit 0 (meaning "unregister this FD")
+                    // is still the closest to what the looper needs to do. When returning 0,
+                    // Looper knows to ignore the fd if it has already been unregistered.
                     return 0;
                 });
     }