Pass rule modification errors back to CommandListener.
Change-Id: If01334dccad8b6230648713a57fd58be180ac66b
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index d858e60..2b6d348 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -1625,12 +1625,15 @@
}
// strtoul() returns 0 on errors, which is fine because 0 is an invalid netId.
unsigned netId = strtoul(argv[2], NULL, 0);
+ int ret;
if (!strcmp(argv[1], "addiface")) {
- if (!sNetCtrl->addInterfaceToNetwork(netId, argv[3])) {
+ if ((ret = sNetCtrl->addInterfaceToNetwork(netId, argv[3])) != 0) {
+ errno = -ret;
return operationError(client, "addInterfaceToNetwork() failed");
}
} else {
- if (!sNetCtrl->removeInterfaceFromNetwork(netId, argv[3])) {
+ if ((ret = sNetCtrl->removeInterfaceFromNetwork(netId, argv[3])) != 0) {
+ errno = -ret;
return operationError(client, "removeInterfaceFromNetwork() failed");
}
}
@@ -1666,11 +1669,11 @@
return syntaxError(client, "Missing id");
}
if (!strcmp(argv[2], "user")) {
- if (!sNetCtrl->setPermissionForUser(permission, ids)) {
- return operationError(client, "setPermissionForUser() failed");
- }
+ sNetCtrl->setPermissionForUser(permission, ids);
} else if (!strcmp(argv[2], "network")) {
- if (!sNetCtrl->setPermissionForNetwork(permission, ids)) {
+ int ret = sNetCtrl->setPermissionForNetwork(permission, ids);
+ if (ret) {
+ errno = -ret;
return operationError(client, "setPermissionForNetwork() failed");
}
} else {
diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp
index de433b7..af95c1e 100644
--- a/server/NetworkController.cpp
+++ b/server/NetworkController.cpp
@@ -197,71 +197,70 @@
return true;
}
-bool NetworkController::addInterfaceToNetwork(unsigned netId, const char* interface) {
+int NetworkController::addInterfaceToNetwork(unsigned netId, const char* interface) {
if (!isValidNetwork(netId) || !interface) {
ALOGE("invalid netId %u or interface null", netId);
- errno = EINVAL;
- return false;
+ return -EINVAL;
}
unsigned existingNetId = getNetworkId(interface);
if (existingNetId != NETID_UNSET) {
ALOGE("interface %s already assigned to netId %u", interface, existingNetId);
- errno = EBUSY;
- return false;
+ return -EBUSY;
}
+ int ret;
Permission permission = mPermissionsController->getPermissionForNetwork(netId);
- if (!mRouteController->addInterfaceToNetwork(netId, interface, permission)) {
+ if ((ret = mRouteController->addInterfaceToNetwork(netId, interface, permission)) != 0) {
ALOGE("failed to add interface %s to netId %u", interface, netId);
- return false;
+ return ret;
}
mNetIdToInterfaces.insert(std::pair<unsigned, std::string>(netId, interface));
if (netId == getDefaultNetwork() &&
- !mRouteController->addToDefaultNetwork(interface, permission)) {
+ (ret = mRouteController->addToDefaultNetwork(interface, permission)) != 0) {
ALOGE("failed to add interface %s to default netId %u", interface, netId);
- return false;
+ return ret;
}
- return true;
+ return 0;
}
-bool NetworkController::removeInterfaceFromNetwork(unsigned netId, const char* interface) {
+int NetworkController::removeInterfaceFromNetwork(unsigned netId, const char* interface) {
if (!isValidNetwork(netId) || !interface) {
ALOGE("invalid netId %u or interface null", netId);
- errno = EINVAL;
- return false;
+ return -EINVAL;
}
- bool status = false;
+ int ret = -ENOENT;
InterfaceRange range = mNetIdToInterfaces.equal_range(netId);
for (InterfaceIterator iter = range.first; iter != range.second; ++iter) {
if (iter->second == interface) {
mNetIdToInterfaces.erase(iter);
- status = true;
+ ret = 0;
break;
}
}
- if (!status) {
+
+ if (ret) {
ALOGE("interface %s not assigned to netId %u", interface, netId);
- errno = ENOENT;
+ return ret;
}
Permission permission = mPermissionsController->getPermissionForNetwork(netId);
if (netId == getDefaultNetwork() &&
- !mRouteController->removeFromDefaultNetwork(interface, permission)) {
+ (ret = mRouteController->removeFromDefaultNetwork(interface, permission)) != 0) {
ALOGE("failed to remove interface %s from default netId %u", interface, netId);
- status = false;
+ return ret;
}
- if (!mRouteController->removeInterfaceFromNetwork(netId, interface, permission)) {
+ if ((ret = mRouteController->removeInterfaceFromNetwork(netId, interface, permission)) != 0) {
ALOGE("failed to remove interface %s from netId %u", interface, netId);
- status = false;
+ return ret;
}
- return status;
+ return 0;
}
bool NetworkController::destroyNetwork(unsigned netId) {
@@ -301,24 +300,19 @@
return status;
}
-bool NetworkController::setPermissionForUser(Permission permission,
+void NetworkController::setPermissionForUser(Permission permission,
const std::vector<unsigned>& uid) {
for (size_t i = 0; i < uid.size(); ++i) {
mPermissionsController->setPermissionForUser(permission, uid[i]);
}
- return true;
}
-bool NetworkController::setPermissionForNetwork(Permission newPermission,
- const std::vector<unsigned>& netId) {
- bool status = true;
-
+int NetworkController::setPermissionForNetwork(Permission newPermission,
+ const std::vector<unsigned>& netId) {
for (size_t i = 0; i < netId.size(); ++i) {
if (!isValidNetwork(netId[i])) {
ALOGE("invalid netId %u", netId[i]);
- errno = EINVAL;
- status = false;
- continue;
+ return -EINVAL;
}
Permission oldPermission = mPermissionsController->getPermissionForNetwork(netId[i]);
@@ -331,18 +325,19 @@
InterfaceRange range = mNetIdToInterfaces.equal_range(netId[i]);
for (InterfaceIteratorConst iter = range.first; iter != range.second; ++iter) {
- if (!mRouteController->modifyNetworkPermission(netId[i], iter->second.c_str(),
- oldPermission, newPermission)) {
+ int ret = mRouteController->modifyNetworkPermission(netId[i], iter->second.c_str(),
+ oldPermission, newPermission);
+ if (ret) {
ALOGE("failed to change permission on interface %s of netId %u from %x to %x",
iter->second.c_str(), netId[i], oldPermission, newPermission);
- status = false;
+ return ret;
}
}
mPermissionsController->setPermissionForNetwork(newPermission, netId[i]);
}
- return status;
+ return 0;
}
int NetworkController::addRoute(unsigned netId, const char* interface, const char* destination,
diff --git a/server/NetworkController.h b/server/NetworkController.h
index 4e93fa1..869f593 100644
--- a/server/NetworkController.h
+++ b/server/NetworkController.h
@@ -59,11 +59,11 @@
bool createNetwork(unsigned netId, Permission permission);
bool destroyNetwork(unsigned netId);
- bool addInterfaceToNetwork(unsigned netId, const char* interface);
- bool removeInterfaceFromNetwork(unsigned netId, const char* interface);
+ int addInterfaceToNetwork(unsigned netId, const char* interface);
+ int removeInterfaceFromNetwork(unsigned netId, const char* interface);
- bool setPermissionForUser(Permission permission, const std::vector<unsigned>& uid);
- bool setPermissionForNetwork(Permission permission, const std::vector<unsigned>& netId);
+ void setPermissionForUser(Permission permission, const std::vector<unsigned>& uid);
+ int setPermissionForNetwork(Permission permission, const std::vector<unsigned>& netId);
// Routes are added to tables determined by the interface, so only |interface| is actually used.
// |netId| is given only to sanity check that the interface has the correct netId.
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 9dbd021..af5df04 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -123,8 +123,10 @@
// + If |mask| is non-zero, the rule matches the specified fwmark and mask. Otherwise, |fwmark| is
// ignored.
// + If |interface| is non-NULL, the rule matches the specified outgoing interface.
-bool modifyIpRule(uint16_t action, uint32_t priority, uint32_t table, uint32_t fwmark,
- uint32_t mask, const char* interface) {
+//
+// Returns 0 on success or negative errno on failure.
+int modifyIpRule(uint16_t action, uint32_t priority, uint32_t table, uint32_t fwmark, uint32_t mask,
+ const char* interface) {
// The interface name must include exactly one terminating NULL and be properly padded, or older
// kernels will refuse to delete rules.
uint8_t padding[RTA_ALIGNTO] = {0, 0, 0, 0};
@@ -172,12 +174,11 @@
rule.family = family[i];
int ret = sendNetlinkRequest(action, flags, iov, ARRAY_SIZE(iov));
if (ret) {
- errno = -ret;
- return false;
+ return ret;
}
}
- return true;
+ return 0;
}
// Adds or deletes an IPv4 or IPv6 route.
@@ -247,14 +248,15 @@
return sendNetlinkRequest(action, flags, iov, ARRAY_SIZE(iov));
}
-bool modifyPerNetworkRules(unsigned netId, const char* interface, Permission permission, bool add,
- bool modifyIptables) {
+int modifyPerNetworkRules(unsigned netId, const char* interface, Permission permission, bool add,
+ bool modifyIptables) {
uint32_t table = getRouteTableForInterface(interface);
if (!table) {
- return false;
+ return -ESRCH;
}
uint16_t action = add ? RTM_NEWRULE : RTM_DELRULE;
+ int ret;
Fwmark fwmark;
fwmark.permission = permission;
@@ -266,9 +268,9 @@
//
// Supports apps that use SO_BINDTODEVICE or IP_PKTINFO options and the kernel that already
// knows the outgoing interface (typically for link-local communications).
- if (!modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_INTERFACE, table, fwmark.intValue,
- mask.intValue, interface)) {
- return false;
+ if ((ret = modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_INTERFACE, table, fwmark.intValue,
+ mask.intValue, interface)) != 0) {
+ return ret;
}
// A rule to route traffic based on the chosen network.
@@ -278,9 +280,9 @@
// network stay on that network even if the default network changes.
fwmark.netId = netId;
mask.netId = FWMARK_NET_ID_MASK;
- if (!modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_NORMAL, table, fwmark.intValue,
- mask.intValue, NULL)) {
- return false;
+ if ((ret = modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_NORMAL, table, fwmark.intValue,
+ mask.intValue, NULL)) != 0) {
+ return ret;
}
// A rule to route traffic based on an explicitly chosen network.
@@ -291,9 +293,9 @@
// checked at the time the netId was set into the fwmark, but we do so to be consistent.
fwmark.explicitlySelected = true;
mask.explicitlySelected = true;
- if (!modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_EXPLICIT, table, fwmark.intValue,
- mask.intValue, NULL)) {
- return false;
+ if ((ret = modifyIpRule(action, RULE_PRIORITY_PER_NETWORK_EXPLICIT, table, fwmark.intValue,
+ mask.intValue, NULL)) != 0) {
+ return ret;
}
// An iptables rule to mark incoming packets on a network with the netId of the network.
@@ -309,17 +311,17 @@
snprintf(markString, sizeof(markString), "0x%x", netId);
if (execIptables(V4V6, "-t", "mangle", iptablesAction, "INPUT", "-i", interface,
"-j", "MARK", "--set-mark", markString, NULL)) {
- return false;
+ return -EREMOTEIO;
}
}
- return true;
+ return 0;
}
-bool modifyDefaultNetworkRules(const char* interface, Permission permission, uint16_t action) {
+int modifyDefaultNetworkRules(const char* interface, Permission permission, uint16_t action) {
uint32_t table = getRouteTableForInterface(interface);
if (!table) {
- return false;
+ return -ESRCH;
}
Fwmark fwmark;
@@ -454,29 +456,29 @@
#endif
}
-bool RouteController::addInterfaceToNetwork(unsigned netId, const char* interface,
- Permission permission) {
+int RouteController::addInterfaceToNetwork(unsigned netId, const char* interface,
+ Permission permission) {
return modifyPerNetworkRules(netId, interface, permission, true, true);
}
-bool RouteController::removeInterfaceFromNetwork(unsigned netId, const char* interface,
- Permission permission) {
+int RouteController::removeInterfaceFromNetwork(unsigned netId, const char* interface,
+ Permission permission) {
return modifyPerNetworkRules(netId, interface, permission, false, true) &&
flushRoutes(interface);
}
-bool RouteController::modifyNetworkPermission(unsigned netId, const char* interface,
- Permission oldPermission, Permission newPermission) {
+int RouteController::modifyNetworkPermission(unsigned netId, const char* interface,
+ Permission oldPermission, Permission newPermission) {
// Add the new rules before deleting the old ones, to avoid race conditions.
return modifyPerNetworkRules(netId, interface, newPermission, true, false) &&
modifyPerNetworkRules(netId, interface, oldPermission, false, false);
}
-bool RouteController::addToDefaultNetwork(const char* interface, Permission permission) {
+int RouteController::addToDefaultNetwork(const char* interface, Permission permission) {
return modifyDefaultNetworkRules(interface, permission, RTM_NEWRULE);
}
-bool RouteController::removeFromDefaultNetwork(const char* interface, Permission permission) {
+int RouteController::removeFromDefaultNetwork(const char* interface, Permission permission) {
return modifyDefaultNetworkRules(interface, permission, RTM_DELRULE);
}
diff --git a/server/RouteController.h b/server/RouteController.h
index 4695a6c..b826e08 100644
--- a/server/RouteController.h
+++ b/server/RouteController.h
@@ -32,14 +32,14 @@
static void Init();
- static bool addInterfaceToNetwork(unsigned netId, const char* interface, Permission permission);
- static bool removeInterfaceFromNetwork(unsigned netId, const char* interface,
- Permission permission);
- static bool modifyNetworkPermission(unsigned netId, const char* interface,
- Permission oldPermission, Permission newPermission);
+ static int addInterfaceToNetwork(unsigned netId, const char* interface, Permission permission);
+ static int removeInterfaceFromNetwork(unsigned netId, const char* interface,
+ Permission permission);
+ static int modifyNetworkPermission(unsigned netId, const char* interface,
+ Permission oldPermission, Permission newPermission);
- static bool addToDefaultNetwork(const char* interface, Permission permission);
- static bool removeFromDefaultNetwork(const char* interface, Permission permission);
+ static int addToDefaultNetwork(const char* interface, Permission permission);
+ static int removeFromDefaultNetwork(const char* interface, Permission permission);
static int addRoute(const char* interface, const char* destination, const char* nexthop,
TableType tableType, unsigned uid);