From a1be1358918505eedbcdb2d5600194c96b8260c7 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Tue, 28 Dec 2021 15:16:57 +0100
Subject: Kernel: Lock socket mutex across {get,set}sockopt() and SO_ERROR
 updates

Since a socket can be accessed by multiple threads concurrently, we need
to protect shared data behind the socket mutex.

There's very likely more places where we need to fix this, the purpose
of this patch is to fix a VERIFY() failure in getsockopt() seen on CI.
---
 Kernel/Net/IPv4Socket.cpp  | 4 ++++
 Kernel/Net/LocalSocket.cpp | 2 ++
 Kernel/Net/Socket.cpp      | 4 ++++
 Kernel/Net/Socket.h        | 8 +++++++-
 4 files changed, 17 insertions(+), 1 deletion(-)

(limited to 'Kernel')

diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp
index d51f4d3258..772ac201d4 100644
--- a/Kernel/Net/IPv4Socket.cpp
+++ b/Kernel/Net/IPv4Socket.cpp
@@ -505,6 +505,8 @@ ErrorOr<void> IPv4Socket::setsockopt(int level, int option, Userspace<const void
     if (level != IPPROTO_IP)
         return Socket::setsockopt(level, option, user_value, user_value_size);
 
+    MutexLocker locker(mutex());
+
     switch (option) {
     case IP_TTL: {
         if (user_value_size < sizeof(int))
@@ -569,6 +571,8 @@ ErrorOr<void> IPv4Socket::getsockopt(OpenFileDescription& description, int level
     if (level != IPPROTO_IP)
         return Socket::getsockopt(description, level, option, value, value_size);
 
+    MutexLocker locker(mutex());
+
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp
index ea8adaf303..73205c85c2 100644
--- a/Kernel/Net/LocalSocket.cpp
+++ b/Kernel/Net/LocalSocket.cpp
@@ -386,6 +386,8 @@ ErrorOr<void> LocalSocket::getsockopt(OpenFileDescription& description, int leve
     if (level != SOL_SOCKET)
         return Socket::getsockopt(description, level, option, value, value_size);
 
+    MutexLocker locker(mutex());
+
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
diff --git a/Kernel/Net/Socket.cpp b/Kernel/Net/Socket.cpp
index 8655b913cb..623495f5c7 100644
--- a/Kernel/Net/Socket.cpp
+++ b/Kernel/Net/Socket.cpp
@@ -78,6 +78,8 @@ ErrorOr<void> Socket::queue_connection_from(NonnullRefPtr<Socket> peer)
 
 ErrorOr<void> Socket::setsockopt(int level, int option, Userspace<const void*> user_value, socklen_t user_value_size)
 {
+    MutexLocker locker(mutex());
+
     if (level != SOL_SOCKET)
         return ENOPROTOOPT;
     VERIFY(level == SOL_SOCKET);
@@ -133,6 +135,8 @@ ErrorOr<void> Socket::setsockopt(int level, int option, Userspace<const void*> u
 
 ErrorOr<void> Socket::getsockopt(OpenFileDescription&, int level, int option, Userspace<void*> value, Userspace<socklen_t*> value_size)
 {
+    MutexLocker locker(mutex());
+
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
diff --git a/Kernel/Net/Socket.h b/Kernel/Net/Socket.h
index 72a159c843..013172c11c 100644
--- a/Kernel/Net/Socket.h
+++ b/Kernel/Net/Socket.h
@@ -130,16 +130,22 @@ protected:
 
     Role m_role { Role::None };
 
-    ErrorOr<void> so_error() const { return m_so_error; }
+    ErrorOr<void> so_error() const
+    {
+        VERIFY(m_mutex.is_locked_by_current_thread());
+        return m_so_error;
+    }
 
     Error set_so_error(ErrnoCode error_code)
     {
+        MutexLocker locker(mutex());
         auto error = Error::from_errno(error_code);
         m_so_error = error;
         return error;
     }
     Error set_so_error(Error error)
     {
+        MutexLocker locker(mutex());
         m_so_error = error;
         return error;
     }
-- 
cgit v1.2.3