From 51080c19255b267b12d738bc33bb65943382a155 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Tue, 9 Jan 2024 23:14:45 +0800 Subject: [PATCH] Fix error codes in `get{sock,peer}name` --- .../src/net/socket/ip/datagram/bound.rs | 3 +- .../src/net/socket/ip/datagram/mod.rs | 17 +++--- kernel/aster-nix/src/net/socket/ip/mod.rs | 10 ++++ .../src/net/socket/ip/stream/init.rs | 8 +-- .../aster-nix/src/net/socket/ip/stream/mod.rs | 12 ++-- regression/apps/network/tcp_err.c | 59 +++++++++++++++++++ regression/apps/network/udp_err.c | 32 ++++++++++ 7 files changed, 121 insertions(+), 20 deletions(-) diff --git a/kernel/aster-nix/src/net/socket/ip/datagram/bound.rs b/kernel/aster-nix/src/net/socket/ip/datagram/bound.rs index a161405c6..317a93f57 100644 --- a/kernel/aster-nix/src/net/socket/ip/datagram/bound.rs +++ b/kernel/aster-nix/src/net/socket/ip/datagram/bound.rs @@ -27,9 +27,8 @@ impl BoundDatagram { self.bound_socket.local_endpoint().unwrap() } - pub fn remote_endpoint(&self) -> Result { + pub fn remote_endpoint(&self) -> Option { self.remote_endpoint - .ok_or_else(|| Error::with_message(Errno::EINVAL, "remote endpoint is not specified")) } pub fn set_remote_endpoint(&mut self, endpoint: &IpEndpoint) { diff --git a/kernel/aster-nix/src/net/socket/ip/datagram/mod.rs b/kernel/aster-nix/src/net/socket/ip/datagram/mod.rs index 7db41acaf..b05611fe4 100644 --- a/kernel/aster-nix/src/net/socket/ip/datagram/mod.rs +++ b/kernel/aster-nix/src/net/socket/ip/datagram/mod.rs @@ -5,7 +5,7 @@ use core::sync::atomic::{AtomicBool, Ordering}; use takeable::Takeable; use self::{bound::BoundDatagram, unbound::UnboundDatagram}; -use super::common::get_ephemeral_endpoint; +use super::{common::get_ephemeral_endpoint, UNSPECIFIED_LOCAL_ENDPOINT}; use crate::{ events::{IoEvents, Observer}, fs::{file_handle::FileLike, utils::StatusFlags}, @@ -237,18 +237,21 @@ impl Socket for DatagramSocket { fn addr(&self) -> Result { let inner = self.inner.read(); - let Inner::Bound(bound_datagram) = inner.as_ref() else { - return_errno_with_message!(Errno::EINVAL, "the socket is not bound"); - }; - Ok(bound_datagram.local_endpoint().into()) + match inner.as_ref() { + Inner::Unbound(unbound_datagram) => Ok(UNSPECIFIED_LOCAL_ENDPOINT.into()), + Inner::Bound(bound_datagram) => Ok(bound_datagram.local_endpoint().into()), + } } fn peer_addr(&self) -> Result { let inner = self.inner.read(); let Inner::Bound(bound_datagram) = inner.as_ref() else { - return_errno_with_message!(Errno::EINVAL, "the socket is not bound"); + return_errno_with_message!(Errno::ENOTCONN, "the socket is not connected") }; - Ok(bound_datagram.remote_endpoint()?.into()) + bound_datagram + .remote_endpoint() + .map(|endpoint| endpoint.into()) + .ok_or_else(|| Error::with_message(Errno::ENOTCONN, "the socket is not connected")) } // FIXME: respect RecvFromFlags diff --git a/kernel/aster-nix/src/net/socket/ip/mod.rs b/kernel/aster-nix/src/net/socket/ip/mod.rs index 30a8c4ee1..c9826fb77 100644 --- a/kernel/aster-nix/src/net/socket/ip/mod.rs +++ b/kernel/aster-nix/src/net/socket/ip/mod.rs @@ -1,8 +1,18 @@ // SPDX-License-Identifier: MPL-2.0 +use crate::net::iface::{IpAddress, IpEndpoint, Ipv4Address}; + mod common; mod datagram; pub mod stream; pub use datagram::DatagramSocket; pub use stream::StreamSocket; + +/// A local endpoint, which indicates that the local endpoint is unspecified. +/// +/// According to the Linux man pages and the Linux implementation, `getsockname()` will _not_ fail +/// even if the socket is unbound. Instead, it will return an unspecified socket address. This +/// unspecified endpoint helps with that. +const UNSPECIFIED_LOCAL_ENDPOINT: IpEndpoint = + IpEndpoint::new(IpAddress::Ipv4(Ipv4Address::UNSPECIFIED), 0); 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 3f8626b32..43f7d0c96 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/init.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/init.rs @@ -83,12 +83,10 @@ impl InitStream { .map_err(|(err, bound_socket)| (err, InitStream::Bound(bound_socket))) } - pub fn local_endpoint(&self) -> Result { + pub fn local_endpoint(&self) -> Option { match self { - InitStream::Unbound(_) => { - return_errno_with_message!(Errno::EINVAL, "does not has local endpoint") - } - InitStream::Bound(bound_socket) => Ok(bound_socket.local_endpoint().unwrap()), + InitStream::Unbound(_) => None, + InitStream::Bound(bound_socket) => Some(bound_socket.local_endpoint().unwrap()), } } 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 564bb7d08..d235c84ad 100644 --- a/kernel/aster-nix/src/net/socket/ip/stream/mod.rs +++ b/kernel/aster-nix/src/net/socket/ip/stream/mod.rs @@ -11,6 +11,7 @@ use smoltcp::wire::IpEndpoint; use takeable::Takeable; use util::{TcpOptionSet, DEFAULT_MAXSEG}; +use super::UNSPECIFIED_LOCAL_ENDPOINT; use crate::{ events::{IoEvents, Observer}, fs::{file_handle::FileLike, utils::StatusFlags}, @@ -385,7 +386,9 @@ impl Socket for StreamSocket { fn addr(&self) -> Result { let state = self.state.read(); let local_endpoint = match state.as_ref() { - State::Init(init_stream) => init_stream.local_endpoint()?, + State::Init(init_stream) => init_stream + .local_endpoint() + .unwrap_or(UNSPECIFIED_LOCAL_ENDPOINT), State::Connecting(connecting_stream) => connecting_stream.local_endpoint(), State::Listen(listen_stream) => listen_stream.local_endpoint(), State::Connected(connected_stream) => connected_stream.local_endpoint(), @@ -396,13 +399,10 @@ impl Socket for StreamSocket { fn peer_addr(&self) -> Result { let state = self.state.read(); let remote_endpoint = match state.as_ref() { - State::Init(init_stream) => { - return_errno_with_message!(Errno::EINVAL, "init socket does not have peer") + State::Init(_) | State::Listen(_) => { + return_errno_with_message!(Errno::ENOTCONN, "the socket is not connected") } State::Connecting(connecting_stream) => connecting_stream.remote_endpoint(), - State::Listen(listen_stream) => { - return_errno_with_message!(Errno::EINVAL, "listening socket does not have peer") - } State::Connected(connected_stream) => connected_stream.remote_endpoint(), }; Ok(remote_endpoint.into()) diff --git a/regression/apps/network/tcp_err.c b/regression/apps/network/tcp_err.c index 3317f184e..49c9b1af3 100644 --- a/regression/apps/network/tcp_err.c +++ b/regression/apps/network/tcp_err.c @@ -77,3 +77,62 @@ FN_SETUP(accpected) } while (sk_accepted < 0); } END_SETUP() + +FN_TEST(getsockname) +{ + struct sockaddr_in saddr = { .sin_port = 0xbeef }; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_RES(getsockname(sk_unbound, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == 0); + + TEST_RES(getsockname(sk_bound, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == C_PORT); + + TEST_RES(getsockname(sk_listen, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == S_PORT); + + TEST_RES(getsockname(sk_connected, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port != S_PORT); + + TEST_RES(getsockname(sk_accepted, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == S_PORT); +} +END_TEST() + +FN_TEST(getpeername) +{ + struct sockaddr_in saddr = { .sin_port = 0xbeef }; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_ERRNO(getpeername(sk_unbound, psaddr, &addrlen), ENOTCONN); + + TEST_ERRNO(getpeername(sk_bound, psaddr, &addrlen), ENOTCONN); + + TEST_ERRNO(getpeername(sk_listen, psaddr, &addrlen), ENOTCONN); + + TEST_RES(getpeername(sk_connected, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == S_PORT); + + TEST_RES(getpeername(sk_accepted, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port != S_PORT); +} +END_TEST() + +FN_TEST(peername_is_peer_sockname) +{ + struct sockaddr_in saddr = { .sin_port = 0xbeef }; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + int em_port; + + TEST_RES(getsockname(sk_connected, psaddr, &addrlen), + addrlen == sizeof(saddr)); + em_port = saddr.sin_port; + + TEST_RES(getpeername(sk_accepted, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == em_port); +} +END_TEST() diff --git a/regression/apps/network/udp_err.c b/regression/apps/network/udp_err.c index 6169c2bc9..7bdb4bb1b 100644 --- a/regression/apps/network/udp_err.c +++ b/regression/apps/network/udp_err.c @@ -50,3 +50,35 @@ FN_SETUP(connected) sizeof(sk_addr))); } END_SETUP() + +FN_TEST(getsockname) +{ + struct sockaddr_in saddr = { .sin_port = 0xbeef }; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_RES(getsockname(sk_unbound, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == 0); + + TEST_RES(getsockname(sk_bound, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == C_PORT); + + TEST_RES(getsockname(sk_connected, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port != C_PORT); +} +END_TEST() + +FN_TEST(getpeername) +{ + struct sockaddr_in saddr = { .sin_port = 0xbeef }; + struct sockaddr *psaddr = (struct sockaddr *)&saddr; + socklen_t addrlen = sizeof(saddr); + + TEST_ERRNO(getpeername(sk_unbound, psaddr, &addrlen), ENOTCONN); + + TEST_ERRNO(getpeername(sk_bound, psaddr, &addrlen), ENOTCONN); + + TEST_RES(getpeername(sk_connected, psaddr, &addrlen), + addrlen == sizeof(saddr) && saddr.sin_port == C_PORT); +} +END_TEST()