qtaguid.cpp - improvements
Resolves a pair of TODO's, and makes a pair of error return
code paths not return null function pointers.
Note that:
system/netd/client/NetdClient.cpp
implements this as:
int checkSocket(int socketFd) {
if (socketFd < 0) {
return -EBADF;
}
int family;
socklen_t familyLen = sizeof(family);
if (getsockopt(socketFd, SOL_SOCKET, SO_DOMAIN, &family, &familyLen) == -1) {
return -errno;
}
if (!FwmarkClient::shouldSetFwmark(family)) {
return -EAFNOSUPPORT;
}
return 0;
}
$define CHECK_SOCKET_IS_MARKABLE(sock) \
do { \
int err = checkSocket(sock); \
if (err) return err; \
} while (false)
extern "C" int tagSocket(int socketFd, uint32_t tag, uid_t uid) {
CHECK_SOCKET_IS_MARKABLE(socketFd);
FwmarkCommand command = {FwmarkCommand::TAG_SOCKET, 0, uid, tag};
return FwmarkClient().send(&command, socketFd, nullptr);
}
extern "C" int untagSocket(int socketFd) {
CHECK_SOCKET_IS_MARKABLE(socketFd);
FwmarkCommand command = {FwmarkCommand::UNTAG_SOCKET, 0, 0, 0};
return FwmarkClient().send(&command, socketFd, nullptr);
}
which means it *already* verifies that the passed in sockfd
is >= 0 and a socket via getsockopt(SOL_SOCKET, SO_DOMAIN),
as such the 'fcntl(sockfd, F_GETFD)' check is spurious.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I91ef68be5b0cc6b1972d514c13a76eaf834a3d5d
diff --git a/libcutils/qtaguid.cpp b/libcutils/qtaguid.cpp
index a987b85..f847782 100644
--- a/libcutils/qtaguid.cpp
+++ b/libcutils/qtaguid.cpp
@@ -22,76 +22,60 @@
#include <dlfcn.h>
#include <errno.h>
-#include <fcntl.h>
#include <inttypes.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
#include <log/log.h>
-class netdHandler {
- public:
+struct netdHandler {
int (*netdTagSocket)(int, uint32_t, uid_t);
int (*netdUntagSocket)(int);
};
-int stubTagSocket(int, uint32_t, uid_t) {
+static int stubTagSocket(int, uint32_t, uid_t) {
return -EREMOTEIO;
}
-int stubUntagSocket(int) {
+static int stubUntagSocket(int) {
return -EREMOTEIO;
}
-netdHandler initHandler(void) {
- netdHandler handler = {stubTagSocket, stubUntagSocket};
+static netdHandler initHandler(void) {
+ const netdHandler stubHandler = { stubTagSocket, stubUntagSocket };
void* netdClientHandle = dlopen("libnetd_client.so", RTLD_NOW);
if (!netdClientHandle) {
ALOGE("Failed to open libnetd_client.so: %s", dlerror());
- return handler;
+ return stubHandler;
}
+ netdHandler handler;
handler.netdTagSocket = (int (*)(int, uint32_t, uid_t))dlsym(netdClientHandle, "tagSocket");
if (!handler.netdTagSocket) {
ALOGE("load netdTagSocket handler failed: %s", dlerror());
+ return stubHandler;
}
handler.netdUntagSocket = (int (*)(int))dlsym(netdClientHandle, "untagSocket");
if (!handler.netdUntagSocket) {
ALOGE("load netdUntagSocket handler failed: %s", dlerror());
+ return stubHandler;
}
return handler;
}
// The language guarantees that this object will be initialized in a thread-safe way.
-static netdHandler& getHandler() {
- static netdHandler instance = initHandler();
+static const netdHandler& getHandler() {
+ static const netdHandler instance = initHandler();
return instance;
}
int qtaguid_tagSocket(int sockfd, int tag, uid_t uid) {
- // Check the socket fd passed to us is still valid before we load the netd
- // client. Pass a already closed socket fd to netd client may let netd open
- // the unix socket with the same fd number and pass it to server for
- // tagging.
- // TODO: move the check into netdTagSocket.
- int res = fcntl(sockfd, F_GETFD);
- if (res < 0) return res;
-
ALOGV("Tagging socket %d with tag %u for uid %d", sockfd, tag, uid);
return getHandler().netdTagSocket(sockfd, tag, uid);
}
int qtaguid_untagSocket(int sockfd) {
- // Similiar to tag socket. We need a check before untag to make sure untag a closed socket fail
- // as expected.
- // TODO: move the check into netdTagSocket.
- int res = fcntl(sockfd, F_GETFD);
- if (res < 0) return res;
-
ALOGV("Untagging socket %d", sockfd);
return getHandler().netdUntagSocket(sockfd);
}