From 5c63617191c92820a497c4d1081c92f1410027fc Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Tue, 8 Oct 2024 21:08:14 +0200 Subject: [PATCH] net: fix leaking sockets in listen_tcp if an error occurs --- core/net/socket_darwin.odin | 1 + core/net/socket_freebsd.odin | 1 + core/net/socket_linux.odin | 34 +++++++++++++++++++++------------- core/net/socket_windows.odin | 1 + 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/core/net/socket_darwin.odin b/core/net/socket_darwin.odin index e26c7407d..27927e973 100644 --- a/core/net/socket_darwin.odin +++ b/core/net/socket_darwin.odin @@ -120,6 +120,7 @@ _listen_tcp :: proc(interface_endpoint: Endpoint, backlog := 1000) -> (skt: TCP_ family := family_from_endpoint(interface_endpoint) sock := create_socket(family, .TCP) or_return skt = sock.(TCP_Socket) + defer if err != nil { close(skt) } // NOTE(tetra): This is so that if we crash while the socket is open, we can // bypass the cooldown period, and allow the next run of the program to diff --git a/core/net/socket_freebsd.odin b/core/net/socket_freebsd.odin index 2c193b0d8..3a3774007 100644 --- a/core/net/socket_freebsd.odin +++ b/core/net/socket_freebsd.odin @@ -137,6 +137,7 @@ _listen_tcp :: proc(interface_endpoint: Endpoint, backlog := 1000) -> (socket: T family := family_from_endpoint(interface_endpoint) new_socket := create_socket(family, .TCP) or_return socket = new_socket.(TCP_Socket) + defer if err != nil { close(socket) } bind(socket, interface_endpoint) or_return diff --git a/core/net/socket_linux.odin b/core/net/socket_linux.odin index be189f5c1..b7816b0b6 100644 --- a/core/net/socket_linux.odin +++ b/core/net/socket_linux.odin @@ -167,40 +167,48 @@ _bind :: proc(sock: Any_Socket, endpoint: Endpoint) -> (Network_Error) { } @(private) -_listen_tcp :: proc(endpoint: Endpoint, backlog := 1000) -> (TCP_Socket, Network_Error) { +_listen_tcp :: proc(endpoint: Endpoint, backlog := 1000) -> (socket: TCP_Socket, err: Network_Error) { errno: linux.Errno assert(backlog > 0 && i32(backlog) < max(i32)) + // Figure out the address family and address of the endpoint ep_family := _unwrap_os_family(family_from_endpoint(endpoint)) ep_address := _unwrap_os_addr(endpoint) + // Create TCP socket os_sock: linux.Fd os_sock, errno = linux.socket(ep_family, .STREAM, {.CLOEXEC}, .TCP) if errno != .NONE { - // TODO(flysand): should return invalid file descriptor here casted as TCP_Socket - return {}, Create_Socket_Error(errno) + err = Create_Socket_Error(errno) + return } + socket = cast(TCP_Socket)os_sock + defer if err != nil { close(socket) } + // NOTE(tetra): This is so that if we crash while the socket is open, we can // bypass the cooldown period, and allow the next run of the program to // use the same address immediately. // // TODO(tetra, 2022-02-15): Confirm that this doesn't mean other processes can hijack the address! do_reuse_addr: b32 = true - errno = linux.setsockopt(os_sock, linux.SOL_SOCKET, linux.Socket_Option.REUSEADDR, &do_reuse_addr) - if errno != .NONE { - return cast(TCP_Socket) os_sock, Listen_Error(errno) + if errno = linux.setsockopt(os_sock, linux.SOL_SOCKET, linux.Socket_Option.REUSEADDR, &do_reuse_addr); errno != .NONE { + err = Listen_Error(errno) + return } + // Bind the socket to endpoint address - errno = linux.bind(os_sock, &ep_address) - if errno != .NONE { - return cast(TCP_Socket) os_sock, Bind_Error(errno) + if errno = linux.bind(os_sock, &ep_address); errno != .NONE { + err = Bind_Error(errno) + return } + // Listen on bound socket - errno = linux.listen(os_sock, cast(i32) backlog) - if errno != .NONE { - return cast(TCP_Socket) os_sock, Listen_Error(errno) + if errno = linux.listen(os_sock, cast(i32) backlog); errno != .NONE { + err = Listen_Error(errno) + return } - return cast(TCP_Socket) os_sock, nil + + return } @(private) diff --git a/core/net/socket_windows.odin b/core/net/socket_windows.odin index 8e18f4f88..747d5cab3 100644 --- a/core/net/socket_windows.odin +++ b/core/net/socket_windows.odin @@ -107,6 +107,7 @@ _listen_tcp :: proc(interface_endpoint: Endpoint, backlog := 1000) -> (socket: T family := family_from_endpoint(interface_endpoint) sock := create_socket(family, .TCP) or_return socket = sock.(TCP_Socket) + defer if err != nil { close(socket) } // NOTE(tetra): While I'm not 100% clear on it, my understanding is that this will // prevent hijacking of the server's endpoint by other applications.