From 92a9f5616ee7a4c94af6e9f5e37783fae57f59bd Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Tue, 9 Jan 2024 23:41:52 +0800 Subject: [PATCH] Fix error codes in `bind`, `listen`, and `accept` --- kernel/aster-nix/src/net/iface/common.rs | 4 +- .../src/net/socket/ip/stream/init.rs | 5 ++- .../src/net/socket/ip/stream/listen.rs | 24 +++++++--- .../aster-nix/src/net/socket/ip/stream/mod.rs | 24 +++++++--- kernel/aster-nix/src/net/socket/mod.rs | 22 ++++----- regression/apps/network/tcp_err.c | 45 +++++++++++++++++++ regression/apps/network/udp_err.c | 35 +++++++++++++++ 7 files changed, 132 insertions(+), 27 deletions(-) diff --git a/kernel/aster-nix/src/net/iface/common.rs b/kernel/aster-nix/src/net/iface/common.rs index f43de3fa6..aa7d8e1c2 100644 --- a/kernel/aster-nix/src/net/iface/common.rs +++ b/kernel/aster-nix/src/net/iface/common.rs @@ -77,7 +77,7 @@ impl IfaceCommon { return Ok(port); } } - return_errno_with_message!(Errno::EAGAIN, "cannot find unused high port"); + return_errno_with_message!(Errno::EAGAIN, "no ephemeral port is available"); } fn bind_port(&self, port: u16, can_reuse: bool) -> Result<()> { @@ -86,7 +86,7 @@ impl IfaceCommon { if *used_times == 0 || can_reuse { *used_times += 1; } else { - return_errno_with_message!(Errno::EADDRINUSE, "cannot bind port"); + return_errno_with_message!(Errno::EADDRINUSE, "the address is already in use"); } } else { used_ports.insert(port, 1); diff --git a/kernel/aster-nix/src/net/socket/ip/stream/init.rs b/kernel/aster-nix/src/net/socket/ip/stream/init.rs index 43f7d0c96..9c84655da 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/init.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/init.rs @@ -73,8 +73,11 @@ impl InitStream { pub fn listen(self, backlog: usize) -> core::result::Result { let InitStream::Bound(bound_socket) = self else { + // FIXME: The socket should be bound to INADDR_ANY (i.e., 0.0.0.0) with an ephemeral + // port. However, INADDR_ANY is not yet supported, so we need to return an error first. + debug_assert!(false, "listen() without bind() is not implemented"); return Err(( - Error::with_message(Errno::EINVAL, "cannot listen without bound"), + Error::with_message(Errno::EINVAL, "listen() without bind() is not implemented"), self, )); }; diff --git a/kernel/aster-nix/src/net/socket/ip/stream/listen.rs b/kernel/aster-nix/src/net/socket/ip/stream/listen.rs index 239d9ae9b..3625f1b66 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/listen.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/listen.rs @@ -1,4 +1,5 @@ // SPDX-License-Identifier: MPL-2.0 +use smoltcp::socket::tcp::ListenError; use super::connected::ConnectedStream; use crate::{ @@ -57,7 +58,9 @@ impl ListenStream { let index = backlog_sockets .iter() .position(|backlog_socket| backlog_socket.is_active()) - .ok_or_else(|| Error::with_message(Errno::EAGAIN, "try to accept again"))?; + .ok_or_else(|| { + Error::with_message(Errno::EAGAIN, "no pending connection is available") + })?; let active_backlog_socket = backlog_sockets.remove(index); match BacklogSocket::new(&self.bound_socket) { @@ -119,6 +122,8 @@ struct BacklogSocket { } impl BacklogSocket { + // FIXME: All of the error codes below seem to have no Linux equivalents, and I see no reason + // why the error may occur. Perhaps it is better to call `unwrap()` directly? fn new(bound_socket: &Arc) -> Result { let local_endpoint = bound_socket.local_endpoint().ok_or(Error::with_message( Errno::EINVAL, @@ -133,13 +138,18 @@ impl BacklogSocket { .bind_socket(unbound_socket, bind_port_config) .map_err(|(err, _)| err)? }; - bound_socket.raw_with(|raw_tcp_socket: &mut RawTcpSocket| { - raw_tcp_socket - .listen(local_endpoint) - .map_err(|_| Error::with_message(Errno::EINVAL, "fail to listen")) - })?; - Ok(Self { bound_socket }) + let result = bound_socket + .raw_with(|raw_tcp_socket: &mut RawTcpSocket| raw_tcp_socket.listen(local_endpoint)); + match result { + Ok(()) => Ok(Self { bound_socket }), + Err(ListenError::Unaddressable) => { + return_errno_with_message!(Errno::EINVAL, "the listening address is invalid") + } + Err(ListenError::InvalidState) => { + return_errno_with_message!(Errno::EINVAL, "the listening socket is invalid") + } + } } fn is_active(&self) -> bool { diff --git a/kernel/aster-nix/src/net/socket/ip/stream/mod.rs b/kernel/aster-nix/src/net/socket/ip/stream/mod.rs index b7be8c645..3e425b5e7 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/mod.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/mod.rs @@ -335,7 +335,10 @@ impl Socket for StreamSocket { let State::Init(init_stream) = owned_state else { return ( owned_state, - Err(Error::with_message(Errno::EINVAL, "cannot bind")), + Err(Error::with_message( + Errno::EINVAL, + "the socket is already bound to an address", + )), ); }; @@ -363,11 +366,20 @@ impl Socket for StreamSocket { let mut state = self.state.write(); state.borrow_result(|owned_state| { - let State::Init(init_stream) = owned_state else { - return ( - owned_state, - Err(Error::with_message(Errno::EINVAL, "cannot listen")), - ); + let init_stream = match owned_state { + State::Init(init_stream) => init_stream, + State::Listen(listen_stream) => { + return (State::Listen(listen_stream), Ok(())); + } + State::Connecting(_) | State::Connected(_) => { + return ( + owned_state, + Err(Error::with_message( + Errno::EINVAL, + "the socket is already connected", + )), + ); + } }; let listen_stream = match init_stream.listen(backlog) { diff --git a/kernel/aster-nix/src/net/socket/mod.rs b/kernel/aster-nix/src/net/socket/mod.rs index ceb2eb90f..e58bc6769 100644 --- a/kernel/aster-nix/src/net/socket/mod.rs +++ b/kernel/aster-nix/src/net/socket/mod.rs @@ -16,53 +16,53 @@ mod util; pub trait Socket: FileLike + Send + Sync { /// Assign the address specified by socket_addr to the socket fn bind(&self, socket_addr: SocketAddr) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "bind not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "bind() is not supported"); } /// Build connection for a given address fn connect(&self, socket_addr: SocketAddr) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "connect not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "connect() is not supported"); } /// Listen for connections on a socket fn listen(&self, backlog: usize) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "connect not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "connect() is not supported"); } /// Accept a connection on a socket fn accept(&self) -> Result<(Arc, SocketAddr)> { - return_errno_with_message!(Errno::EINVAL, "accept not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "accept() is not supported"); } /// Shut down part of a full-duplex connection fn shutdown(&self, cmd: SockShutdownCmd) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "shutdown not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "shutdown() is not supported"); } /// Get address of this socket. fn addr(&self) -> Result { - return_errno_with_message!(Errno::EINVAL, "getsockname not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "getsockname() is not supported"); } /// Get address of peer socket fn peer_addr(&self) -> Result { - return_errno_with_message!(Errno::EINVAL, "getpeername not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "getpeername() is not supported"); } /// Get options on the socket. The resulted option will put in the `option` parameter, if /// this method returns success. fn get_option(&self, option: &mut dyn SocketOption) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "getsockopt not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "getsockopt() is not supported"); } /// Set options on the socket. fn set_option(&self, option: &dyn SocketOption) -> Result<()> { - return_errno_with_message!(Errno::EINVAL, "setsockopt not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "setsockopt() is not supported"); } /// Receive a message from a socket fn recvfrom(&self, buf: &mut [u8], flags: SendRecvFlags) -> Result<(usize, SocketAddr)> { - return_errno_with_message!(Errno::EINVAL, "recvfrom not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "recvfrom() is not supported"); } /// Send a message on a socket @@ -72,6 +72,6 @@ pub trait Socket: FileLike + Send + Sync { remote: Option, flags: SendRecvFlags, ) -> Result { - return_errno_with_message!(Errno::EINVAL, "recvfrom not implemented"); + return_errno_with_message!(Errno::EOPNOTSUPP, "recvfrom() is not supported"); } } diff --git a/regression/apps/network/tcp_err.c b/regression/apps/network/tcp_err.c index ead55d4fd..034d93c97 100644 --- a/regression/apps/network/tcp_err.c +++ b/regression/apps/network/tcp_err.c @@ -181,3 +181,48 @@ FN_TEST(send_and_recv) TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); } END_TEST() + +FN_TEST(bind) +{ + struct sockaddr *psaddr = (struct sockaddr *)&sk_addr; + socklen_t addrlen = sizeof(sk_addr); + + TEST_ERRNO(bind(sk_bound, psaddr, addrlen), EINVAL); + + TEST_ERRNO(bind(sk_listen, psaddr, addrlen), EINVAL); + + TEST_ERRNO(bind(sk_connected, psaddr, addrlen), EINVAL); + + TEST_ERRNO(bind(sk_accepted, psaddr, addrlen), EINVAL); +} +END_TEST() + +FN_TEST(listen) +{ + // The second `listen` does nothing but succeed. + // TODO: Will it update the backlog? + TEST_SUCC(listen(sk_listen, 2)); + + TEST_ERRNO(listen(sk_connected, 2), EINVAL); + + TEST_ERRNO(listen(sk_accepted, 2), EINVAL); +} +END_TEST() + +FN_TEST(accept) +{ + struct sockaddr_in saddr; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_ERRNO(accept(sk_unbound, psaddr, &addrlen), EINVAL); + + TEST_ERRNO(accept(sk_bound, psaddr, &addrlen), EINVAL); + + TEST_ERRNO(accept(sk_listen, psaddr, &addrlen), EAGAIN); + + TEST_ERRNO(accept(sk_connected, psaddr, &addrlen), EINVAL); + + TEST_ERRNO(accept(sk_accepted, psaddr, &addrlen), EINVAL); +} +END_TEST() diff --git a/regression/apps/network/udp_err.c b/regression/apps/network/udp_err.c index 31a89d70e..41abc38ce 100644 --- a/regression/apps/network/udp_err.c +++ b/regression/apps/network/udp_err.c @@ -135,3 +135,38 @@ FN_TEST(send_and_recv) saddr.sin_port != C_PORT && buf[0] == 'b'); } END_TEST() + +FN_TEST(bind) +{ + struct sockaddr *psaddr = (struct sockaddr *)&sk_addr; + socklen_t addrlen = sizeof(sk_addr); + + TEST_ERRNO(bind(sk_bound, psaddr, addrlen), EINVAL); + + TEST_ERRNO(bind(sk_connected, psaddr, addrlen), EINVAL); +} +END_TEST() + +FN_TEST(listen) +{ + TEST_ERRNO(listen(sk_unbound, 2), EOPNOTSUPP); + + TEST_ERRNO(listen(sk_bound, 2), EOPNOTSUPP); + + TEST_ERRNO(listen(sk_connected, 2), EOPNOTSUPP); +} +END_TEST() + +FN_TEST(accept) +{ + struct sockaddr_in saddr; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_ERRNO(accept(sk_unbound, psaddr, &addrlen), EOPNOTSUPP); + + TEST_ERRNO(accept(sk_bound, psaddr, &addrlen), EOPNOTSUPP); + + TEST_ERRNO(accept(sk_connected, psaddr, &addrlen), EOPNOTSUPP); +} +END_TEST()