From f93c0dc4894546ec71fe9677f7eaab153708ba0c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 6 Dec 2019 18:39:59 +0100 Subject: [PATCH] LibIPC: Get client/server PIDs using getsockopt(SO_PEERCRED) Instead of passing the PIDs back and forth in a handshake "Greet" message, just use getsockopt(SO_PEERCRED) on both sides to get the same information from the kernel. This is a nice little simplification of the IPC protocol, although it does not get rid of the handshake since we still have to pass the "client ID" from the server to each client so they know how to refer to themselves. This might not be necessary and we might be able to get rid of this later on. --- Libraries/LibAudio/AClientConnection.cpp | 3 +-- Libraries/LibGUI/GWindowServerConnection.cpp | 3 +-- Libraries/LibIPC/IClientConnection.h | 8 +++++++- Libraries/LibIPC/IServerConnection.h | 9 ++++++++- Libraries/LibProtocol/Client.cpp | 3 +-- Servers/AudioServer/ASClientConnection.cpp | 5 ++--- Servers/AudioServer/AudioServer.ipc | 2 +- Servers/ProtocolServer/PSClientConnection.cpp | 5 ++--- Servers/ProtocolServer/ProtocolServer.ipc | 2 +- Servers/WindowServer/WSClientConnection.cpp | 5 ++--- Servers/WindowServer/WindowServer.ipc | 2 +- 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/Libraries/LibAudio/AClientConnection.cpp b/Libraries/LibAudio/AClientConnection.cpp index c8ec8758a9..b0283680e7 100644 --- a/Libraries/LibAudio/AClientConnection.cpp +++ b/Libraries/LibAudio/AClientConnection.cpp @@ -9,8 +9,7 @@ AClientConnection::AClientConnection() void AClientConnection::handshake() { - auto response = send_sync(getpid()); - set_server_pid(response->server_pid()); + auto response = send_sync(); set_my_client_id(response->client_id()); } diff --git a/Libraries/LibGUI/GWindowServerConnection.cpp b/Libraries/LibGUI/GWindowServerConnection.cpp index fd4d533c86..b04ed1c7d0 100644 --- a/Libraries/LibGUI/GWindowServerConnection.cpp +++ b/Libraries/LibGUI/GWindowServerConnection.cpp @@ -20,8 +20,7 @@ GWindowServerConnection& GWindowServerConnection::the() void GWindowServerConnection::handshake() { - auto response = send_sync(getpid()); - set_server_pid(response->server_pid()); + auto response = send_sync(); set_my_client_id(response->client_id()); GDesktop::the().did_receive_screen_rect({}, response->screen_rect()); } diff --git a/Libraries/LibIPC/IClientConnection.h b/Libraries/LibIPC/IClientConnection.h index 7e816b5dcd..418365c23c 100644 --- a/Libraries/LibIPC/IClientConnection.h +++ b/Libraries/LibIPC/IClientConnection.h @@ -55,6 +55,13 @@ public: , m_socket(socket) , m_client_id(client_id) { + ASSERT(socket.is_connected()); + ucred creds; + socklen_t creds_size = sizeof(creds); + if (getsockopt(m_socket->fd(), SOL_SOCKET, SO_PEERCRED, &creds, &creds_size) < 0) { + ASSERT_NOT_REACHED(); + } + m_client_pid = creds.pid; add_child(socket); m_socket->on_ready_to_read = [this] { drain_messages_from_client(); }; } @@ -142,7 +149,6 @@ public: int client_id() const { return m_client_id; } pid_t client_pid() const { return m_client_pid; } - void set_client_pid(pid_t pid) { m_client_pid = pid; } virtual void die() = 0; diff --git a/Libraries/LibIPC/IServerConnection.h b/Libraries/LibIPC/IServerConnection.h index 70133ee9e9..60f6145914 100644 --- a/Libraries/LibIPC/IServerConnection.h +++ b/Libraries/LibIPC/IServerConnection.h @@ -39,12 +39,19 @@ public: usleep(10000); --retries; } + + ucred creds; + socklen_t creds_size = sizeof(creds); + if (getsockopt(m_connection->fd(), SOL_SOCKET, SO_PEERCRED, &creds, &creds_size) < 0) { + ASSERT_NOT_REACHED(); + } + m_server_pid = creds.pid; + ASSERT(m_connection->is_connected()); } virtual void handshake() = 0; - void set_server_pid(pid_t pid) { m_server_pid = pid; } pid_t server_pid() const { return m_server_pid; } void set_my_client_id(int id) { m_my_client_id = id; } int my_client_id() const { return m_my_client_id; } diff --git a/Libraries/LibProtocol/Client.cpp b/Libraries/LibProtocol/Client.cpp index dd7b1de771..9ec7928c03 100644 --- a/Libraries/LibProtocol/Client.cpp +++ b/Libraries/LibProtocol/Client.cpp @@ -12,8 +12,7 @@ Client::Client() void Client::handshake() { - auto response = send_sync(getpid()); - set_server_pid(response->server_pid()); + auto response = send_sync(); set_my_client_id(response->client_id()); } diff --git a/Servers/AudioServer/ASClientConnection.cpp b/Servers/AudioServer/ASClientConnection.cpp index 5dd25f94fa..caf64d6479 100644 --- a/Servers/AudioServer/ASClientConnection.cpp +++ b/Servers/AudioServer/ASClientConnection.cpp @@ -48,10 +48,9 @@ void ASClientConnection::did_change_muted_state(Badge, bool muted) post_message(AudioClient::MutedStateChanged(muted)); } -OwnPtr ASClientConnection::handle(const AudioServer::Greet& message) +OwnPtr ASClientConnection::handle(const AudioServer::Greet&) { - set_client_pid(message.client_pid()); - return make(getpid(), client_id()); + return make(client_id()); } OwnPtr ASClientConnection::handle(const AudioServer::GetMainMixVolume&) diff --git a/Servers/AudioServer/AudioServer.ipc b/Servers/AudioServer/AudioServer.ipc index b48cbc6c06..b48dcd7e46 100644 --- a/Servers/AudioServer/AudioServer.ipc +++ b/Servers/AudioServer/AudioServer.ipc @@ -1,7 +1,7 @@ endpoint AudioServer = 85 { // Basic protocol - Greet(i32 client_pid) => (i32 server_pid, i32 client_id) + Greet() => (i32 client_id) // Mixer functions SetMuted(bool muted) => () diff --git a/Servers/ProtocolServer/PSClientConnection.cpp b/Servers/ProtocolServer/PSClientConnection.cpp index fe7658720f..6fcea878a5 100644 --- a/Servers/ProtocolServer/PSClientConnection.cpp +++ b/Servers/ProtocolServer/PSClientConnection.cpp @@ -65,10 +65,9 @@ void PSClientConnection::did_progress_download(Badge, Download& downlo post_message(ProtocolClient::DownloadProgress(download.id(), download.total_size(), download.downloaded_size())); } -OwnPtr PSClientConnection::handle(const ProtocolServer::Greet& message) +OwnPtr PSClientConnection::handle(const ProtocolServer::Greet&) { - set_client_pid(message.client_pid()); - return make(getpid(), client_id()); + return make(client_id()); } OwnPtr PSClientConnection::handle(const ProtocolServer::DisownSharedBuffer& message) diff --git a/Servers/ProtocolServer/ProtocolServer.ipc b/Servers/ProtocolServer/ProtocolServer.ipc index 6ed9eba623..698273c3aa 100644 --- a/Servers/ProtocolServer/ProtocolServer.ipc +++ b/Servers/ProtocolServer/ProtocolServer.ipc @@ -1,7 +1,7 @@ endpoint ProtocolServer = 9 { // Basic protocol - Greet(i32 client_pid) => (i32 server_pid, i32 client_id) + Greet() => (i32 client_id) // FIXME: It would be nice if the kernel provided a way to avoid this DisownSharedBuffer(i32 shared_buffer_id) => () diff --git a/Servers/WindowServer/WSClientConnection.cpp b/Servers/WindowServer/WSClientConnection.cpp index 0e23387706..74a5931915 100644 --- a/Servers/WindowServer/WSClientConnection.cpp +++ b/Servers/WindowServer/WSClientConnection.cpp @@ -598,10 +598,9 @@ void WSClientConnection::handle(const WindowServer::WM_SetWindowMinimized& messa window.set_minimized(message.minimized()); } -OwnPtr WSClientConnection::handle(const WindowServer::Greet& message) +OwnPtr WSClientConnection::handle(const WindowServer::Greet&) { - set_client_pid(message.client_pid()); - return make(getpid(), client_id(), WSScreen::the().rect()); + return make(client_id(), WSScreen::the().rect()); } bool WSClientConnection::is_showing_modal_window() const diff --git a/Servers/WindowServer/WindowServer.ipc b/Servers/WindowServer/WindowServer.ipc index 3f78821d37..61ab691b9e 100644 --- a/Servers/WindowServer/WindowServer.ipc +++ b/Servers/WindowServer/WindowServer.ipc @@ -1,6 +1,6 @@ endpoint WindowServer = 2 { - Greet(i32 client_pid) => (i32 server_pid, i32 client_id, Rect screen_rect) + Greet() => (i32 client_id, Rect screen_rect) CreateMenubar() => (i32 menubar_id) DestroyMenubar(i32 menubar_id) => ()