Restrict scope of switch Repair Mode
Bug: 123969339
Test: -boot, flash
-atest FrameworksNetTests
Change-Id: Ie0fb685be5f7a2d06544065d67c605d87a19ff2f
diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
index 35d6860..0e3d82c 100644
--- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
+++ b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
@@ -42,7 +42,6 @@
import android.net.SocketKeepalive.InvalidPacketException;
import android.net.SocketKeepalive.InvalidSocketException;
import android.net.TcpKeepalivePacketData;
-import android.net.TcpKeepalivePacketData.TcpSocketInfo;
import android.net.util.IpUtils;
import android.os.Binder;
import android.os.Handler;
@@ -492,19 +491,14 @@
return;
}
- TcpKeepalivePacketData packet = null;
+ final TcpKeepalivePacketData packet;
try {
- TcpSocketInfo tsi = TcpKeepaliveController.switchToRepairMode(fd);
- packet = TcpKeepalivePacketData.tcpKeepalivePacket(tsi);
+ packet = TcpKeepaliveController.getTcpKeepalivePacket(fd);
} catch (InvalidPacketException | InvalidSocketException e) {
- try {
- TcpKeepaliveController.switchOutOfRepairMode(fd);
- } catch (ErrnoException e1) {
- Log.e(TAG, "Couldn't move fd out of repair mode after failure to start keepalive");
- }
notifyErrorCallback(cb, e.error);
return;
}
+
KeepaliveInfo ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
KeepaliveInfo.TYPE_TCP, fd);
Log.d(TAG, "Created keepalive: " + ki.toString());
diff --git a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java b/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java
index 3e21b5b..e78d25a 100644
--- a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java
+++ b/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java
@@ -28,7 +28,9 @@
import android.annotation.NonNull;
import android.net.NetworkUtils;
+import android.net.SocketKeepalive.InvalidPacketException;
import android.net.SocketKeepalive.InvalidSocketException;
+import android.net.TcpKeepalivePacketData;
import android.net.TcpKeepalivePacketData.TcpSocketInfo;
import android.net.TcpRepairWindow;
import android.os.Handler;
@@ -103,16 +105,25 @@
mFdHandlerQueue = connectivityServiceHandler.getLooper().getQueue();
}
+ /** Build tcp keepalive packet. */
+ public static TcpKeepalivePacketData getTcpKeepalivePacket(@NonNull FileDescriptor fd)
+ throws InvalidPacketException, InvalidSocketException {
+ try {
+ final TcpSocketInfo tsi = switchToRepairMode(fd);
+ return TcpKeepalivePacketData.tcpKeepalivePacket(tsi);
+ } catch (InvalidPacketException | InvalidSocketException e) {
+ switchOutOfRepairMode(fd);
+ throw e;
+ }
+ }
/**
- * Switch the tcp socket to repair mode and query tcp socket information.
+ * Switch the tcp socket to repair mode and query detail tcp information.
*
- * @param fd the fd of socket on which to use keepalive offload
+ * @param fd the fd of socket on which to use keepalive offload.
* @return a {@link TcpKeepalivePacketData#TcpSocketInfo} object for current
* tcp/ip information.
*/
- // TODO : make this private. It's far too confusing that this gets called from outside
- // at a time that nobody can understand.
- public static TcpSocketInfo switchToRepairMode(FileDescriptor fd)
+ private static TcpSocketInfo switchToRepairMode(FileDescriptor fd)
throws InvalidSocketException {
if (DBG) Log.i(TAG, "switchToRepairMode to start tcp keepalive : " + fd);
final SocketAddress srcSockAddr;
@@ -157,10 +168,12 @@
// Query sequence and ack number
dropAllIncomingPackets(fd, true);
try {
- // Enter tcp repair mode.
+ // Switch to tcp repair mode.
Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR, TCP_REPAIR_ON);
+
// Check if socket is idle.
if (!isSocketIdle(fd)) {
+ Log.e(TAG, "Socket is not idle");
throw new InvalidSocketException(ERROR_SOCKET_NOT_IDLE);
}
// Query write sequence number from SEND_QUEUE.
@@ -173,9 +186,14 @@
Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR_QUEUE, TCP_NO_QUEUE);
// Finally, check if socket is still idle. TODO : this check needs to move to
// after starting polling to prevent a race.
- if (!isSocketIdle(fd)) {
+ if (!isReceiveQueueEmpty(fd)) {
+ Log.e(TAG, "Fatal: receive queue of this socket is not empty");
throw new InvalidSocketException(ERROR_INVALID_SOCKET);
}
+ if (!isSendQueueEmpty(fd)) {
+ Log.e(TAG, "Socket is not idle");
+ throw new InvalidSocketException(ERROR_SOCKET_NOT_IDLE);
+ }
// Query tcp window size.
trw = NetworkUtils.getTcpRepairWindow(fd);
@@ -205,10 +223,13 @@
*
* @param fd the fd of socket to switch back to normal.
*/
- // TODO : make this private.
- public static void switchOutOfRepairMode(@NonNull final FileDescriptor fd)
- throws ErrnoException {
- Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR, TCP_REPAIR_OFF);
+ private static void switchOutOfRepairMode(@NonNull final FileDescriptor fd) {
+ try {
+ Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR, TCP_REPAIR_OFF);
+ } catch (ErrnoException e) {
+ Log.e(TAG, "Cannot switch socket out of repair mode", e);
+ // Well, there is not much to do here to recover
+ }
}
/**
@@ -262,13 +283,8 @@
mListeners.remove(slot);
}
mFdHandlerQueue.removeOnFileDescriptorEventListener(fd);
- try {
- if (DBG) Log.d(TAG, "Moving socket out of repair mode for stop : " + fd);
- switchOutOfRepairMode(fd);
- } catch (ErrnoException e) {
- Log.e(TAG, "Cannot switch socket out of repair mode", e);
- // Well, there is not much to do here to recover
- }
+ if (DBG) Log.d(TAG, "Moving socket out of repair mode for stop : " + fd);
+ switchOutOfRepairMode(fd);
}
private static InetAddress getAddress(InetSocketAddress inetAddr) {