Quellcode durchsuchen

Kernel: Use Userspace<T> for the recvfrom syscall, and Socket implementation

This fixes a bunch of unchecked kernel reads and writes, seems like they
would might exploitable :). Write of sockaddr_in size to any address you
please...
Brian Gianforcaro vor 5 Jahren
Ursprung
Commit
8e97de2df9

+ 2 - 2
Kernel/API/Syscall.h

@@ -298,8 +298,8 @@ struct SC_recvfrom_params {
     int sockfd;
     MutableBufferArgument<void, size_t> buffer;
     int flags;
-    sockaddr* addr;
-    socklen_t* addr_length;
+    Userspace<sockaddr*> addr;
+    Userspace<socklen_t*> addr_length;
 };
 
 struct SC_getsockopt_params {

+ 22 - 12
Kernel/Net/IPv4Socket.cpp

@@ -239,7 +239,7 @@ KResultOr<size_t> IPv4Socket::sendto(FileDescription&, const void* data, size_t
     return nsent_or_error;
 }
 
-KResultOr<size_t> IPv4Socket::receive_byte_buffered(FileDescription& description, void* buffer, size_t buffer_length, int, sockaddr*, socklen_t*)
+KResultOr<size_t> IPv4Socket::receive_byte_buffered(FileDescription& description, void* buffer, size_t buffer_length, int, Userspace<sockaddr*>, Userspace<socklen_t*>)
 {
     Locker locker(lock());
     if (m_receive_buffer.is_empty()) {
@@ -270,7 +270,7 @@ KResultOr<size_t> IPv4Socket::receive_byte_buffered(FileDescription& description
     return nreceived;
 }
 
-KResultOr<size_t> IPv4Socket::receive_packet_buffered(FileDescription& description, void* buffer, size_t buffer_length, int flags, sockaddr* addr, socklen_t* addr_length)
+KResultOr<size_t> IPv4Socket::receive_packet_buffered(FileDescription& description, void* buffer, size_t buffer_length, int flags, Userspace<sockaddr*> addr, Userspace<socklen_t*> addr_length)
 {
     Locker locker(lock());
     ReceivedPacket packet;
@@ -324,12 +324,17 @@ KResultOr<size_t> IPv4Socket::receive_packet_buffered(FileDescription& descripti
 #ifdef IPV4_SOCKET_DEBUG
         dbg() << "Incoming packet is from: " << packet.peer_address << ":" << packet.peer_port;
 #endif
-        auto& ia = *(sockaddr_in*)addr;
-        memcpy(&ia.sin_addr, &packet.peer_address, sizeof(IPv4Address));
-        ia.sin_port = htons(packet.peer_port);
-        ia.sin_family = AF_INET;
+
+        sockaddr_in out_addr {};
+        memcpy(&out_addr.sin_addr, &packet.peer_address, sizeof(IPv4Address));
+        out_addr.sin_port = htons(packet.peer_port);
+        out_addr.sin_family = AF_INET;
+        Userspace<sockaddr_in*> dest_addr = addr.ptr();
+        copy_to_user(dest_addr, &out_addr);
+
+        socklen_t out_length = sizeof(sockaddr_in);
         ASSERT(addr_length);
-        *addr_length = sizeof(sockaddr_in);
+        copy_to_user(addr_length, &out_length);
     }
 
     if (type() == SOCK_RAW) {
@@ -341,10 +346,15 @@ KResultOr<size_t> IPv4Socket::receive_packet_buffered(FileDescription& descripti
     return protocol_receive(packet.data.value(), buffer, buffer_length, flags);
 }
 
-KResultOr<size_t> IPv4Socket::recvfrom(FileDescription& description, void* buffer, size_t buffer_length, int flags, sockaddr* addr, socklen_t* addr_length)
+KResultOr<size_t> IPv4Socket::recvfrom(FileDescription& description, void* buffer, size_t buffer_length, int flags, Userspace<sockaddr*> user_addr, Userspace<socklen_t*> user_addr_length)
 {
-    if (addr_length && *addr_length < sizeof(sockaddr_in))
-        return KResult(-EINVAL);
+    if (user_addr_length) {
+        socklen_t addr_length;
+        if (!Process::current()->validate_read_and_copy_typed(&addr_length, user_addr_length))
+            return KResult(-EFAULT);
+        if (addr_length < sizeof(sockaddr_in))
+            return KResult(-EINVAL);
+    }
 
 #ifdef IPV4_SOCKET_DEBUG
     klog() << "recvfrom: type=" << type() << ", local_port=" << local_port();
@@ -352,9 +362,9 @@ KResultOr<size_t> IPv4Socket::recvfrom(FileDescription& description, void* buffe
 
     KResultOr<size_t> nreceived = 0;
     if (buffer_mode() == BufferMode::Bytes)
-        nreceived = receive_byte_buffered(description, buffer, buffer_length, flags, addr, addr_length);
+        nreceived = receive_byte_buffered(description, buffer, buffer_length, flags, user_addr, user_addr_length);
     else
-        nreceived = receive_packet_buffered(description, buffer, buffer_length, flags, addr, addr_length);
+        nreceived = receive_packet_buffered(description, buffer, buffer_length, flags, user_addr, user_addr_length);
 
     if (!nreceived.is_error())
         Thread::current()->did_ipv4_socket_read(nreceived.value());

+ 3 - 3
Kernel/Net/IPv4Socket.h

@@ -59,7 +59,7 @@ public:
     virtual bool can_read(const FileDescription&, size_t) const override;
     virtual bool can_write(const FileDescription&, size_t) const override;
     virtual KResultOr<size_t> sendto(FileDescription&, const void*, size_t, int, Userspace<const sockaddr*>, socklen_t) override;
-    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, sockaddr*, socklen_t*) override;
+    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, Userspace<sockaddr*>, Userspace<socklen_t*>) override;
     virtual KResult setsockopt(int level, int option, Userspace<const void*>, socklen_t) override;
     virtual KResult getsockopt(FileDescription&, int level, int option, Userspace<void*>, Userspace<socklen_t*>) override;
 
@@ -110,8 +110,8 @@ protected:
 private:
     virtual bool is_ipv4() const override { return true; }
 
-    KResultOr<size_t> receive_byte_buffered(FileDescription&, void* buffer, size_t buffer_length, int flags, sockaddr*, socklen_t*);
-    KResultOr<size_t> receive_packet_buffered(FileDescription&, void* buffer, size_t buffer_length, int flags, sockaddr*, socklen_t*);
+    KResultOr<size_t> receive_byte_buffered(FileDescription&, void* buffer, size_t buffer_length, int flags, Userspace<sockaddr*>, Userspace<socklen_t*>);
+    KResultOr<size_t> receive_packet_buffered(FileDescription&, void* buffer, size_t buffer_length, int flags, Userspace<sockaddr*>, Userspace<socklen_t*>);
 
     IPv4Address m_local_address;
     IPv4Address m_peer_address;

+ 1 - 1
Kernel/Net/LocalSocket.cpp

@@ -290,7 +290,7 @@ DoubleBuffer& LocalSocket::send_buffer_for(FileDescription& description)
     ASSERT_NOT_REACHED();
 }
 
-KResultOr<size_t> LocalSocket::recvfrom(FileDescription& description, void* buffer, size_t buffer_size, int, sockaddr*, socklen_t*)
+KResultOr<size_t> LocalSocket::recvfrom(FileDescription& description, void* buffer, size_t buffer_size, int, Userspace<sockaddr*>, Userspace<socklen_t*>)
 {
     auto& buffer_for_me = receive_buffer_for(description);
     if (!description.is_blocking()) {

+ 1 - 1
Kernel/Net/LocalSocket.h

@@ -61,7 +61,7 @@ public:
     virtual bool can_read(const FileDescription&, size_t) const override;
     virtual bool can_write(const FileDescription&, size_t) const override;
     virtual KResultOr<size_t> sendto(FileDescription&, const void*, size_t, int, Userspace<const sockaddr*>, socklen_t) override;
-    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, sockaddr*, socklen_t*) override;
+    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, Userspace<sockaddr*>, Userspace<socklen_t*>) override;
     virtual KResult getsockopt(FileDescription&, int level, int option, Userspace<void*>, Userspace<socklen_t*>) override;
     virtual KResult chown(FileDescription&, uid_t, gid_t) override;
     virtual KResult chmod(FileDescription&, mode_t) override;

+ 1 - 1
Kernel/Net/Socket.cpp

@@ -195,7 +195,7 @@ KResultOr<size_t> Socket::read(FileDescription& description, size_t, u8* buffer,
 {
     if (is_shut_down_for_reading())
         return 0;
-    return recvfrom(description, buffer, size, 0, nullptr, 0);
+    return recvfrom(description, buffer, size, 0, {}, 0);
 }
 
 KResultOr<size_t> Socket::write(FileDescription& description, size_t, const u8* data, size_t size)

+ 1 - 1
Kernel/Net/Socket.h

@@ -108,7 +108,7 @@ public:
     virtual void attach(FileDescription&) = 0;
     virtual void detach(FileDescription&) = 0;
     virtual KResultOr<size_t> sendto(FileDescription&, const void*, size_t, int flags, Userspace<const sockaddr*>, socklen_t) = 0;
-    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, sockaddr*, socklen_t*) = 0;
+    virtual KResultOr<size_t> recvfrom(FileDescription&, void*, size_t, int flags, Userspace<sockaddr*>, Userspace<socklen_t*>) = 0;
 
     virtual KResult setsockopt(int level, int option, Userspace<const void*>, socklen_t);
     virtual KResult getsockopt(FileDescription&, int level, int option, Userspace<void*>, Userspace<socklen_t*>);

+ 1 - 1
Kernel/Process.h

@@ -296,7 +296,7 @@ public:
     int sys$connect(int sockfd, Userspace<const sockaddr*>, socklen_t);
     int sys$shutdown(int sockfd, int how);
     ssize_t sys$sendto(Userspace<const Syscall::SC_sendto_params*>);
-    ssize_t sys$recvfrom(const Syscall::SC_recvfrom_params*);
+    ssize_t sys$recvfrom(Userspace<const Syscall::SC_recvfrom_params*>);
     int sys$getsockopt(Userspace<const Syscall::SC_getsockopt_params*>);
     int sys$setsockopt(Userspace<const Syscall::SC_setsockopt_params*>);
     int sys$getsockname(Userspace<const Syscall::SC_getsockname_params*>);

+ 11 - 8
Kernel/Syscalls/socket.cpp

@@ -218,7 +218,7 @@ ssize_t Process::sys$sendto(Userspace<const Syscall::SC_sendto_params*> user_par
     return result.value();
 }
 
-ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* user_params)
+ssize_t Process::sys$recvfrom(Userspace<const Syscall::SC_recvfrom_params*> user_params)
 {
     REQUIRE_PROMISE(stdio);
 
@@ -227,18 +227,21 @@ ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* user_params)
         return -EFAULT;
 
     int flags = params.flags;
-    sockaddr* addr = params.addr;
-    socklen_t* addr_length = params.addr_length;
+    Userspace<sockaddr*> user_addr = params.addr;
+    Userspace<socklen_t*> user_addr_length = params.addr_length;
 
     SmapDisabler disabler;
     if (!validate(params.buffer))
         return -EFAULT;
-    if (addr_length) {
-        if (!validate_write_typed(addr_length))
+    if (user_addr_length) {
+        socklen_t addr_length;
+        if (!validate_read_and_copy_typed(&addr_length, user_addr_length))
             return -EFAULT;
-        if (!validate_write(addr, *addr_length))
+        if (!validate_write_typed(user_addr_length))
             return -EFAULT;
-    } else if (addr) {
+        if (!validate_write(user_addr, addr_length))
+            return -EFAULT;
+    } else if (user_addr) {
         return -EINVAL;
     }
     auto description = file_description(params.sockfd);
@@ -255,7 +258,7 @@ ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* user_params)
     if (flags & MSG_DONTWAIT)
         description->set_blocking(false);
 
-    auto result = socket.recvfrom(*description, params.buffer.data, params.buffer.size, flags, addr, addr_length);
+    auto result = socket.recvfrom(*description, params.buffer.data, params.buffer.size, flags, user_addr, user_addr_length);
     if (flags & MSG_DONTWAIT)
         description->set_blocking(original_blocking);