LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix cancellation race problem for aysnc DNS API

Browse Source 
This problem might cause double-close fd and result in app crash
or unexpected behaviour

Bug: 129317069
Test: atest DnsResolverTest
      manual test with delaying response callback/cancel
Change-Id: I223234f527edafc51d34fa6be390419c05def8d8
This commit is contained in:
Luke Huang
2019-03-26 15:50:10 +08:00
parent 5c83418408
commit e17bb2dff4
2 changed files with 37 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
core/java/android/net/DnsResolver.java
View File

@@ -205,6 +205,7 @@ public final class DnsResolver {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
final Object lock = new Object();
final FileDescriptor queryfd; final FileDescriptor queryfd;
try { try {
queryfd = resNetworkSend((network != null queryfd = resNetworkSend((network != null
@@ -214,8 +215,8 @@ public final class DnsResolver {
return; return;
} }
maybeAddCancellationSignal(cancellationSignal, queryfd); maybeAddCancellationSignal(cancellationSignal, queryfd, lock);
registerFDListener(executor, queryfd, callback); registerFDListener(executor, queryfd, callback, cancellationSignal, lock);
} }
/** /**
@@ -242,6 +243,7 @@ public final class DnsResolver {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
final Object lock = new Object();
final FileDescriptor queryfd; final FileDescriptor queryfd;
try { try {
queryfd = resNetworkQuery((network != null queryfd = resNetworkQuery((network != null
@@ -251,31 +253,37 @@ public final class DnsResolver {
return; return;
} }
maybeAddCancellationSignal(cancellationSignal, queryfd); maybeAddCancellationSignal(cancellationSignal, queryfd, lock);
registerFDListener(executor, queryfd, callback); registerFDListener(executor, queryfd, callback, cancellationSignal, lock);
} }
private <T> void registerFDListener(@NonNull Executor executor, private <T> void registerFDListener(@NonNull Executor executor,
@NonNull FileDescriptor queryfd, @NonNull AnswerCallback<T> answerCallback) { @NonNull FileDescriptor queryfd, @NonNull AnswerCallback<T> answerCallback,
@Nullable CancellationSignal cancellationSignal, @NonNull Object lock) {
Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener( Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener(
queryfd, queryfd,
FD_EVENTS, FD_EVENTS,
(fd, events) -> { (fd, events) -> {
executor.execute(() -> { executor.execute(() -> {
byte[] answerbuf = null; synchronized (lock) {
try { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
answerbuf = resNetworkResult(fd); return;
} catch (ErrnoException e) { }
Log.e(TAG, "resNetworkResult:" + e.toString()); byte[] answerbuf = null;
answerCallback.onQueryException(e); try {
return; answerbuf = resNetworkResult(fd);
} } catch (ErrnoException e) {
Log.e(TAG, "resNetworkResult:" + e.toString());
answerCallback.onQueryException(e);
return;
}
try { try {
answerCallback.onAnswer( answerCallback.onAnswer(
answerCallback.parser.parse(answerbuf)); answerCallback.parser.parse(answerbuf));
} catch (ParseException e) { } catch (ParseException e) {
answerCallback.onParseException(e); answerCallback.onParseException(e);
}
} }
}); });
// Unregister this fd listener // Unregister this fd listener
@@ -284,14 +292,16 @@ public final class DnsResolver {
} }
private void maybeAddCancellationSignal(@Nullable CancellationSignal cancellationSignal, private void maybeAddCancellationSignal(@Nullable CancellationSignal cancellationSignal,
@NonNull FileDescriptor queryfd) { @NonNull FileDescriptor queryfd, @NonNull Object lock) {
if (cancellationSignal == null) return; if (cancellationSignal == null) return;
cancellationSignal.setOnCancelListener( cancellationSignal.setOnCancelListener(() -> {
() -> { synchronized (lock) {
Looper.getMainLooper().getQueue() if (!queryfd.valid()) return;
.removeOnFileDescriptorEventListener(queryfd); Looper.getMainLooper().getQueue()
resNetworkCancel(queryfd); .removeOnFileDescriptorEventListener(queryfd);
}); resNetworkCancel(queryfd);
}
});
} }
private static class DnsAddressAnswer extends DnsPacket { private static class DnsAddressAnswer extends DnsPacket {

2
core/jni/android_net_NetUtils.cpp
View File

@@ -470,6 +470,7 @@ static jbyteArray android_net_utils_resNetworkResult(JNIEnv *env, jobject thiz,
std::vector<uint8_t> buf(MAXPACKETSIZE, 0); std::vector<uint8_t> buf(MAXPACKETSIZE, 0);
int res = resNetworkResult(fd, &rcode, buf.data(), MAXPACKETSIZE); int res = resNetworkResult(fd, &rcode, buf.data(), MAXPACKETSIZE);
jniSetFileDescriptorOfFD(env, javaFd, -1);
if (res < 0) { if (res < 0) {
throwErrnoException(env, "resNetworkResult", -res); throwErrnoException(env, "resNetworkResult", -res);
return nullptr; return nullptr;
@@ -490,6 +491,7 @@ static jbyteArray android_net_utils_resNetworkResult(JNIEnv *env, jobject thiz,
static void android_net_utils_resNetworkCancel(JNIEnv *env, jobject thiz, jobject javaFd) { static void android_net_utils_resNetworkCancel(JNIEnv *env, jobject thiz, jobject javaFd) {
int fd = jniGetFDFromFileDescriptor(env, javaFd); int fd = jniGetFDFromFileDescriptor(env, javaFd);
resNetworkCancel(fd); resNetworkCancel(fd);
jniSetFileDescriptorOfFD(env, javaFd, -1);
} }
static jobject android_net_utils_getTcpRepairWindow(JNIEnv *env, jobject thiz, jobject javaFd) { static jobject android_net_utils_getTcpRepairWindow(JNIEnv *env, jobject thiz, jobject javaFd) {