From 409c6eecdca80e95a110d63bc70b1c2dfcf49100 Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Thu, 23 Mar 2017 17:45:40 -0700 Subject: Make init process create PDX sockets for services To help us control the creation of PDX sockets and properly labeling them for SELinux, let the init process create sockets for us based on the settings specified in .rc files for service processes. For (test) services that are meant to be started manually from command line (e.g. test services), keep the old functionality as an option so that UDS endpoint can be created in a way that it automatically creates the socket in the service itself. Bug: 35220925 Test: `m -j32` succeeds. Ran sailfish in VR mode and made sure all the services (surfaceflinger, performanced, sensord, bufferhub). `m -j32 checkbuild` succeeds as well. Change-Id: Ief733b41b534cea19b1bea31de76b06051aa50ab --- libs/vr/libpdx_default_transport/Android.bp | 1 + libs/vr/libpdx_uds/Android.bp | 2 + libs/vr/libpdx_uds/channel_event_set.cpp | 2 +- libs/vr/libpdx_uds/private/uds/service_endpoint.h | 3 +- libs/vr/libpdx_uds/service_dispatcher.cpp | 2 +- libs/vr/libpdx_uds/service_endpoint.cpp | 105 ++++++++++++---------- services/surfaceflinger/surfaceflinger.rc | 4 + services/vr/bufferhubd/bufferhubd.rc | 2 +- services/vr/performanced/performanced.rc | 1 + services/vr/sensord/sensord.rc | 2 + 10 files changed, 72 insertions(+), 52 deletions(-) diff --git a/libs/vr/libpdx_default_transport/Android.bp b/libs/vr/libpdx_default_transport/Android.bp index 655adb8ddc..8cfa86fa44 100644 --- a/libs/vr/libpdx_default_transport/Android.bp +++ b/libs/vr/libpdx_default_transport/Android.bp @@ -57,6 +57,7 @@ cc_binary { "pdx_benchmarks.cpp", ], shared_libs: [ + "libbase", "libchrome", "libcutils", "liblog", diff --git a/libs/vr/libpdx_uds/Android.bp b/libs/vr/libpdx_uds/Android.bp index 9f308ecfde..a73ba34177 100644 --- a/libs/vr/libpdx_uds/Android.bp +++ b/libs/vr/libpdx_uds/Android.bp @@ -20,6 +20,8 @@ cc_library_static { "service_endpoint.cpp", ], static_libs: [ + "libcutils", + "libbase", "libpdx", ], } diff --git a/libs/vr/libpdx_uds/channel_event_set.cpp b/libs/vr/libpdx_uds/channel_event_set.cpp index f8baeabe88..ac4dea993b 100644 --- a/libs/vr/libpdx_uds/channel_event_set.cpp +++ b/libs/vr/libpdx_uds/channel_event_set.cpp @@ -12,7 +12,7 @@ ChannelEventSet::ChannelEventSet() { const int flags = EFD_CLOEXEC | EFD_NONBLOCK; LocalHandle epoll_fd, event_fd; - if (!SetupHandle(epoll_create(1), &epoll_fd, "epoll") || + if (!SetupHandle(epoll_create1(EPOLL_CLOEXEC), &epoll_fd, "epoll") || !SetupHandle(eventfd(0, flags), &event_fd, "event")) { return; } diff --git a/libs/vr/libpdx_uds/private/uds/service_endpoint.h b/libs/vr/libpdx_uds/private/uds/service_endpoint.h index 9d038cb2a0..2b24f62b84 100644 --- a/libs/vr/libpdx_uds/private/uds/service_endpoint.h +++ b/libs/vr/libpdx_uds/private/uds/service_endpoint.h @@ -107,7 +107,8 @@ class Endpoint : public pdx::Endpoint { }; // This class must be instantiated using Create() static methods above. - Endpoint(const std::string& endpoint_path, bool blocking); + Endpoint(const std::string& endpoint_path, bool blocking, + bool use_init_socket_fd = true); Endpoint(const Endpoint&) = delete; void operator=(const Endpoint&) = delete; diff --git a/libs/vr/libpdx_uds/service_dispatcher.cpp b/libs/vr/libpdx_uds/service_dispatcher.cpp index fa98f26826..2c52578d1c 100644 --- a/libs/vr/libpdx_uds/service_dispatcher.cpp +++ b/libs/vr/libpdx_uds/service_dispatcher.cpp @@ -30,7 +30,7 @@ ServiceDispatcher::ServiceDispatcher() { return; } - epoll_fd_.Reset(epoll_create(1)); // Size arg is ignored, but must be > 0. + epoll_fd_.Reset(epoll_create1(EPOLL_CLOEXEC)); if (!epoll_fd_) { ALOGE("Failed to create epoll fd because: %s\n", strerror(errno)); return; diff --git a/libs/vr/libpdx_uds/service_endpoint.cpp b/libs/vr/libpdx_uds/service_endpoint.cpp index f89b8a8d0b..6f32867dca 100644 --- a/libs/vr/libpdx_uds/service_endpoint.cpp +++ b/libs/vr/libpdx_uds/service_endpoint.cpp @@ -7,6 +7,9 @@ #include #include // std::min +#include +#include +#include #include #include #include @@ -124,43 +127,50 @@ namespace android { namespace pdx { namespace uds { -Endpoint::Endpoint(const std::string& endpoint_path, bool blocking) +Endpoint::Endpoint(const std::string& endpoint_path, bool blocking, + bool use_init_socket_fd) : endpoint_path_{ClientChannelFactory::GetEndpointPath(endpoint_path)}, is_blocking_{blocking} { - LocalHandle fd{socket(AF_UNIX, SOCK_STREAM, 0)}; - if (!fd) { - ALOGE("Endpoint::Endpoint: Failed to create socket: %s", strerror(errno)); - return; - } - - sockaddr_un local; - local.sun_family = AF_UNIX; - strncpy(local.sun_path, endpoint_path_.c_str(), sizeof(local.sun_path)); - local.sun_path[sizeof(local.sun_path) - 1] = '\0'; - - unlink(local.sun_path); - if (bind(fd.Get(), (struct sockaddr*)&local, sizeof(local)) == -1) { - ALOGE("Endpoint::Endpoint: bind error: %s", strerror(errno)); - return; - } - if (listen(fd.Get(), kMaxBackLogForSocketListen) == -1) { - ALOGE("Endpoint::Endpoint: listen error: %s", strerror(errno)); - return; + LocalHandle fd; + if (use_init_socket_fd) { + // Cut off the /dev/socket/ prefix from the full socket path and use the + // resulting "name" to retrieve the file descriptor for the socket created + // by the init process. + constexpr char prefix[] = "/dev/socket/"; + CHECK(android::base::StartsWith(endpoint_path_, prefix)) + << "Endpoint::Endpoint: Socket name '" << endpoint_path_ + << "' must begin with '" << prefix << "'"; + std::string socket_name = endpoint_path_.substr(sizeof(prefix) - 1); + fd.Reset(android_get_control_socket(socket_name.c_str())); + CHECK(fd.IsValid()) + << "Endpoint::Endpoint: Unable to obtain the control socket fd for '" + << socket_name << "'"; + fcntl(fd.Get(), F_SETFD, FD_CLOEXEC); + } else { + fd.Reset(socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)); + CHECK(fd.IsValid()) << "Endpoint::Endpoint: Failed to create socket: " + << strerror(errno); + + sockaddr_un local; + local.sun_family = AF_UNIX; + strncpy(local.sun_path, endpoint_path_.c_str(), sizeof(local.sun_path)); + local.sun_path[sizeof(local.sun_path) - 1] = '\0'; + + unlink(local.sun_path); + int ret = + bind(fd.Get(), reinterpret_cast(&local), sizeof(local)); + CHECK_EQ(ret, 0) << "Endpoint::Endpoint: bind error: " << strerror(errno); } + CHECK_EQ(listen(fd.Get(), kMaxBackLogForSocketListen), 0) + << "Endpoint::Endpoint: listen error: " << strerror(errno); cancel_event_fd_.Reset(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); - if (!cancel_event_fd_) { - ALOGE("Endpoint::Endpoint: Failed to create event fd: %s\n", - strerror(errno)); - return; - } + CHECK(cancel_event_fd_.IsValid()) + << "Endpoint::Endpoint: Failed to create event fd: " << strerror(errno); - epoll_fd_.Reset(epoll_create(1)); // Size arg is ignored, but must be > 0. - if (!epoll_fd_) { - ALOGE("Endpoint::Endpoint: Failed to create epoll fd: %s\n", - strerror(errno)); - return; - } + epoll_fd_.Reset(epoll_create1(EPOLL_CLOEXEC)); + CHECK(epoll_fd_.IsValid()) + << "Endpoint::Endpoint: Failed to create epoll fd: " << strerror(errno); epoll_event socket_event; socket_event.events = EPOLLIN | EPOLLRDHUP | EPOLLONESHOT; @@ -170,16 +180,16 @@ Endpoint::Endpoint(const std::string& endpoint_path, bool blocking) cancel_event.events = EPOLLIN; cancel_event.data.fd = cancel_event_fd_.Get(); - if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_ADD, fd.Get(), &socket_event) < 0 || - epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_ADD, cancel_event_fd_.Get(), - &cancel_event) < 0) { - ALOGE("Endpoint::Endpoint: Failed to add event fd to epoll fd: %s\n", - strerror(errno)); - cancel_event_fd_.Close(); - epoll_fd_.Close(); - } else { - socket_fd_ = std::move(fd); - } + int ret = epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_ADD, fd.Get(), &socket_event); + CHECK_EQ(ret, 0) + << "Endpoint::Endpoint: Failed to add socket fd to epoll fd: " + << strerror(errno); + ret = epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_ADD, cancel_event_fd_.Get(), + &cancel_event); + CHECK_EQ(ret, 0) + << "Endpoint::Endpoint: Failed to add cancel event fd to epoll fd: " + << strerror(errno); + socket_fd_ = std::move(fd); } void* Endpoint::AllocateMessageState() { return new MessageState; } @@ -191,8 +201,9 @@ void Endpoint::FreeMessageState(void* state) { Status Endpoint::AcceptConnection(Message* message) { sockaddr_un remote; socklen_t addrlen = sizeof(remote); - LocalHandle channel_fd{ - accept(socket_fd_.Get(), reinterpret_cast(&remote), &addrlen)}; + LocalHandle channel_fd{accept4(socket_fd_.Get(), + reinterpret_cast(&remote), &addrlen, + SOCK_CLOEXEC)}; if (!channel_fd) { ALOGE("Endpoint::AcceptConnection: failed to accept connection: %s", strerror(errno)); @@ -317,7 +328,7 @@ Status Endpoint::PushChannel(Message* message, Channel* channel, int* channel_id) { int channel_pair[2] = {}; - if (socketpair(AF_UNIX, SOCK_STREAM, 0, channel_pair) == -1) { + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, channel_pair) == -1) { ALOGE("Endpoint::PushChannel: Failed to create a socket pair: %s", strerror(errno)); return ErrorStatus(errno); @@ -643,10 +654,8 @@ std::unique_ptr Endpoint::Create(const std::string& endpoint_path, std::unique_ptr Endpoint::CreateAndBindSocket( const std::string& endpoint_path, bool blocking) { - // TODO(avakulenko): When Endpoint can differentiate between absolute paths - // and relative paths/socket names created by the init process, change this - // code to reflect the fact that we want to use absolute paths here. - return std::unique_ptr(new Endpoint(endpoint_path, blocking)); + return std::unique_ptr( + new Endpoint(endpoint_path, blocking, false)); } } // namespace uds diff --git a/services/surfaceflinger/surfaceflinger.rc b/services/surfaceflinger/surfaceflinger.rc index 0b482f7591..41b6225302 100644 --- a/services/surfaceflinger/surfaceflinger.rc +++ b/services/surfaceflinger/surfaceflinger.rc @@ -4,3 +4,7 @@ service surfaceflinger /system/bin/surfaceflinger group graphics drmrpc readproc onrestart restart zygote writepid /dev/stune/foreground/tasks + socket pdx/system/vr/display/client stream 0666 system graphics + socket pdx/system/vr/display/manager stream 0660 system graphics + socket pdx/system/vr/display/screenshot stream 0660 system graphics + socket pdx/system/vr/display/vsync stream 0666 system graphics diff --git a/services/vr/bufferhubd/bufferhubd.rc b/services/vr/bufferhubd/bufferhubd.rc index 65b7293160..8d57723994 100644 --- a/services/vr/bufferhubd/bufferhubd.rc +++ b/services/vr/bufferhubd/bufferhubd.rc @@ -3,4 +3,4 @@ service bufferhubd /system/bin/bufferhubd user system group system writepid /dev/cpuset/tasks - + socket pdx/system/buffer_hub/client stream 0660 system system diff --git a/services/vr/performanced/performanced.rc b/services/vr/performanced/performanced.rc index 5042982d80..6283f37178 100644 --- a/services/vr/performanced/performanced.rc +++ b/services/vr/performanced/performanced.rc @@ -3,3 +3,4 @@ service performanced /system/bin/performanced user root group system readproc writepid /dev/cpuset/tasks + socket pdx/system/performance/client stream 0666 system system diff --git a/services/vr/sensord/sensord.rc b/services/vr/sensord/sensord.rc index f8d28fd448..36cd377369 100644 --- a/services/vr/sensord/sensord.rc +++ b/services/vr/sensord/sensord.rc @@ -7,3 +7,5 @@ service sensord /system/bin/sensord user system group system camera sdcard_rw writepid /dev/cpuset/system/tasks + socket pdx/system/vr/sensors/client stream 0666 system system + socket pdx/system/vr/pose/client stream 0666 system system -- cgit v1.2.3-59-g8ed1b