Browse Source

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.
Andreas Kling 3 years ago
parent
commit
a1be135891
4 changed files with 17 additions and 1 deletions
  1. 4 0
      Kernel/Net/IPv4Socket.cpp
  2. 2 0
      Kernel/Net/LocalSocket.cpp
  3. 4 0
      Kernel/Net/Socket.cpp
  4. 7 1
      Kernel/Net/Socket.h

+ 4 - 0
Kernel/Net/IPv4Socket.cpp

@@ -505,6 +505,8 @@ ErrorOr<void> IPv4Socket::setsockopt(int level, int option, Userspace<const void
     if (level != IPPROTO_IP)
     if (level != IPPROTO_IP)
         return Socket::setsockopt(level, option, user_value, user_value_size);
         return Socket::setsockopt(level, option, user_value, user_value_size);
 
 
+    MutexLocker locker(mutex());
+
     switch (option) {
     switch (option) {
     case IP_TTL: {
     case IP_TTL: {
         if (user_value_size < sizeof(int))
         if (user_value_size < sizeof(int))
@@ -569,6 +571,8 @@ ErrorOr<void> IPv4Socket::getsockopt(OpenFileDescription& description, int level
     if (level != IPPROTO_IP)
     if (level != IPPROTO_IP)
         return Socket::getsockopt(description, level, option, value, value_size);
         return Socket::getsockopt(description, level, option, value, value_size);
 
 
+    MutexLocker locker(mutex());
+
     socklen_t size;
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
 

+ 2 - 0
Kernel/Net/LocalSocket.cpp

@@ -386,6 +386,8 @@ ErrorOr<void> LocalSocket::getsockopt(OpenFileDescription& description, int leve
     if (level != SOL_SOCKET)
     if (level != SOL_SOCKET)
         return Socket::getsockopt(description, level, option, value, value_size);
         return Socket::getsockopt(description, level, option, value, value_size);
 
 
+    MutexLocker locker(mutex());
+
     socklen_t size;
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
 

+ 4 - 0
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)
 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)
     if (level != SOL_SOCKET)
         return ENOPROTOOPT;
         return ENOPROTOOPT;
     VERIFY(level == SOL_SOCKET);
     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)
 ErrorOr<void> Socket::getsockopt(OpenFileDescription&, int level, int option, Userspace<void*> value, Userspace<socklen_t*> value_size)
 {
 {
+    MutexLocker locker(mutex());
+
     socklen_t size;
     socklen_t size;
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
     TRY(copy_from_user(&size, value_size.unsafe_userspace_ptr()));
 
 

+ 7 - 1
Kernel/Net/Socket.h

@@ -130,16 +130,22 @@ protected:
 
 
     Role m_role { Role::None };
     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)
     Error set_so_error(ErrnoCode error_code)
     {
     {
+        MutexLocker locker(mutex());
         auto error = Error::from_errno(error_code);
         auto error = Error::from_errno(error_code);
         m_so_error = error;
         m_so_error = error;
         return error;
         return error;
     }
     }
     Error set_so_error(Error error)
     Error set_so_error(Error error)
     {
     {
+        MutexLocker locker(mutex());
         m_so_error = error;
         m_so_error = error;
         return error;
         return error;
     }
     }