diff --git a/kernel/src/fs/pipe.rs b/kernel/src/fs/pipe.rs index 2205288b3..0a933f8f2 100644 --- a/kernel/src/fs/pipe.rs +++ b/kernel/src/fs/pipe.rs @@ -60,12 +60,17 @@ impl Pollable for PipeReader { impl FileLike for PipeReader { fn read(&self, writer: &mut VmWriter) -> Result { - let read_len = if self.status_flags().contains(StatusFlags::O_NONBLOCK) { - self.consumer.try_read(writer)? + if !writer.has_avail() { + // Even the peer endpoint (`PipeWriter`) has been closed, reading an empty buffer is + // still fine. + return Ok(0); + } + + if self.status_flags().contains(StatusFlags::O_NONBLOCK) { + self.consumer.try_read(writer) } else { - self.wait_events(IoEvents::IN, None, || self.consumer.try_read(writer))? - }; - Ok(read_len) + self.wait_events(IoEvents::IN, None, || self.consumer.try_read(writer)) + } } fn status_flags(&self) -> StatusFlags { @@ -130,6 +135,12 @@ impl Pollable for PipeWriter { impl FileLike for PipeWriter { fn write(&self, reader: &mut VmReader) -> Result { + if !reader.has_remain() { + // Even the peer endpoint (`PipeReader`) has been closed, writing an empty buffer is + // still fine. + return Ok(0); + } + if self.status_flags().contains(StatusFlags::O_NONBLOCK) { self.producer.try_write(reader) } else { diff --git a/kernel/src/fs/utils/channel.rs b/kernel/src/fs/utils/channel.rs index b10b2d606..97944b202 100644 --- a/kernel/src/fs/utils/channel.rs +++ b/kernel/src/fs/utils/channel.rs @@ -95,6 +95,14 @@ macro_rules! impl_common_methods_for_channel { self.0.common.is_shutdown() } + pub fn is_full(&self) -> bool { + self.this_end().rb().is_full() + } + + pub fn is_empty(&self) -> bool { + self.this_end().rb().is_empty() + } + pub fn poll(&self, mask: IoEvents, poller: Option<&mut PollHandle>) -> IoEvents { self.this_end() .pollee @@ -134,11 +142,10 @@ impl Producer { /// - Returns `Ok(_)` with the number of bytes written if successful. /// - Returns `Err(EPIPE)` if the channel is shut down. /// - Returns `Err(EAGAIN)` if the channel is full. + /// + /// The caller should not pass an empty `reader` to this method. pub fn try_write(&self, reader: &mut dyn MultiRead) -> Result { - if reader.is_empty() { - // Even after shutdown, writing an empty buffer is still fine. - return Ok(0); - } + debug_assert!(!reader.is_empty()); if self.is_shutdown() { return_errno_with_message!(Errno::EPIPE, "the channel is shut down"); @@ -217,10 +224,10 @@ impl Consumer { /// - Returns `Ok(_)` with the number of bytes read if successful. /// - Returns `Ok(0)` if the channel is shut down and there is no data left. /// - Returns `Err(EAGAIN)` if the channel is empty. + /// + /// The caller should not pass an empty `writer` to this method. pub fn try_read(&self, writer: &mut dyn MultiWrite) -> Result { - if writer.is_empty() { - return Ok(0); - } + debug_assert!(!writer.is_empty()); // This must be recorded before the actual operation to avoid race conditions. let is_shutdown = self.is_shutdown(); diff --git a/kernel/src/net/socket/mod.rs b/kernel/src/net/socket/mod.rs index e0a20464e..90479c7e0 100644 --- a/kernel/src/net/socket/mod.rs +++ b/kernel/src/net/socket/mod.rs @@ -125,6 +125,11 @@ pub trait Socket: private::SocketPrivate + Send + Sync { impl FileLike for T { fn read(&self, writer: &mut VmWriter) -> Result { + if !writer.has_avail() { + // Linux always returns `Ok(0)` in this case, so we follow it. + return Ok(0); + } + // TODO: Set correct flags self.recvmsg(writer, SendRecvFlags::empty()) .map(|(len, _)| len) diff --git a/kernel/src/net/socket/netlink/common/mod.rs b/kernel/src/net/socket/netlink/common/mod.rs index 6189796b5..329456909 100644 --- a/kernel/src/net/socket/netlink/common/mod.rs +++ b/kernel/src/net/socket/netlink/common/mod.rs @@ -142,6 +142,11 @@ where warn!("sending control message is not supported"); } + if reader.is_empty() { + // Based on how Linux behaves, zero-sized messages are not allowed for netlink sockets. + return_errno_with_message!(Errno::ENODATA, "there are no data to send"); + } + // TODO: Make sure our blocking behavior matches that of Linux self.try_send(reader, remote.as_ref(), flags) } diff --git a/kernel/src/net/socket/unix/stream/connected.rs b/kernel/src/net/socket/unix/stream/connected.rs index df1a5ff07..70ce4a37f 100644 --- a/kernel/src/net/socket/unix/stream/connected.rs +++ b/kernel/src/net/socket/unix/stream/connected.rs @@ -72,10 +72,24 @@ impl Connected { } pub(super) fn try_read(&self, writer: &mut dyn MultiWrite) -> Result { + if writer.is_empty() { + if self.reader.is_empty() { + return_errno_with_message!(Errno::EAGAIN, "the channel is empty"); + } + return Ok(0); + } + self.reader.try_read(writer) } pub(super) fn try_write(&self, reader: &mut dyn MultiRead) -> Result { + if reader.is_empty() { + if self.writer.is_shutdown() { + return_errno_with_message!(Errno::EPIPE, "the channel is shut down"); + } + return Ok(0); + } + self.writer.try_write(reader) } diff --git a/test/apps/network/rtnl_err.c b/test/apps/network/rtnl_err.c index 87fa0fd93..c2a314888 100644 --- a/test/apps/network/rtnl_err.c +++ b/test/apps/network/rtnl_err.c @@ -83,9 +83,15 @@ FN_TEST(send) { char buf[1] = { 'z' }; - TEST_RES(send(sk_bound, buf, 1, 0), 1); + TEST_SUCC(send(sk_bound, buf, 1, 0)); + TEST_ERRNO(send(sk_bound, buf, 0, 0), ENODATA); + TEST_SUCC(write(sk_bound, buf, 1)); + TEST_ERRNO(write(sk_bound, buf, 0), ENODATA); TEST_ERRNO(send(sk_connected, buf, 1, 0), ECONNREFUSED); + TEST_ERRNO(send(sk_connected, buf, 0, 0), ENODATA); + TEST_ERRNO(write(sk_connected, buf, 1), ECONNREFUSED); + TEST_ERRNO(write(sk_connected, buf, 0), ENODATA); } END_TEST() @@ -94,10 +100,19 @@ FN_TEST(recv) char buf[1] = { 'z' }; TEST_ERRNO(recv(sk_unbound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_unbound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_unbound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_unbound, buf, 0)); TEST_ERRNO(recv(sk_bound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_bound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_bound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_bound, buf, 0)); TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_connected, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_connected, buf, 1), EAGAIN); + TEST_SUCC(read(sk_connected, buf, 0)); } END_TEST() diff --git a/test/apps/network/tcp_err.c b/test/apps/network/tcp_err.c index 772726c64..9d7ebb7f4 100644 --- a/test/apps/network/tcp_err.c +++ b/test/apps/network/tcp_err.c @@ -148,10 +148,19 @@ FN_TEST(send) char buf[1] = { 'z' }; TEST_ERRNO(send(sk_unbound, buf, 1, 0), EPIPE); + TEST_ERRNO(send(sk_unbound, buf, 0, 0), EPIPE); + TEST_ERRNO(write(sk_unbound, buf, 1), EPIPE); + TEST_ERRNO(write(sk_unbound, buf, 0), EPIPE); TEST_ERRNO(send(sk_bound, buf, 1, 0), EPIPE); + TEST_ERRNO(send(sk_bound, buf, 0, 0), EPIPE); + TEST_ERRNO(write(sk_bound, buf, 1), EPIPE); + TEST_ERRNO(write(sk_bound, buf, 0), EPIPE); TEST_ERRNO(send(sk_listen, buf, 1, 0), EPIPE); + TEST_ERRNO(send(sk_listen, buf, 0, 0), EPIPE); + TEST_ERRNO(write(sk_listen, buf, 1), EPIPE); + TEST_ERRNO(write(sk_listen, buf, 0), EPIPE); } END_TEST() @@ -160,10 +169,24 @@ FN_TEST(recv) char buf[1] = { 'z' }; TEST_ERRNO(recv(sk_unbound, buf, 1, 0), ENOTCONN); + TEST_ERRNO(recv(sk_unbound, buf, 0, 0), ENOTCONN); + TEST_ERRNO(read(sk_unbound, buf, 1), ENOTCONN); + TEST_SUCC(read(sk_unbound, buf, 0)); TEST_ERRNO(recv(sk_bound, buf, 1, 0), ENOTCONN); + TEST_ERRNO(recv(sk_bound, buf, 0, 0), ENOTCONN); + TEST_ERRNO(read(sk_bound, buf, 1), ENOTCONN); + TEST_SUCC(read(sk_bound, buf, 0)); TEST_ERRNO(recv(sk_listen, buf, 1, 0), ENOTCONN); + TEST_ERRNO(recv(sk_listen, buf, 0, 0), ENOTCONN); + TEST_ERRNO(read(sk_listen, buf, 1), ENOTCONN); + TEST_SUCC(read(sk_listen, buf, 0)); + + TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_connected, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_connected, buf, 1), EAGAIN); + TEST_SUCC(read(sk_connected, buf, 0)); } END_TEST() diff --git a/test/apps/network/udp_err.c b/test/apps/network/udp_err.c index c5ffe4d11..bf04bde09 100644 --- a/test/apps/network/udp_err.c +++ b/test/apps/network/udp_err.c @@ -93,8 +93,14 @@ FN_TEST(send) char buf[1] = { 'z' }; TEST_ERRNO(send(sk_unbound, buf, 1, 0), EDESTADDRREQ); + TEST_ERRNO(send(sk_unbound, buf, 0, 0), EDESTADDRREQ); + TEST_ERRNO(write(sk_unbound, buf, 1), EDESTADDRREQ); + TEST_ERRNO(write(sk_unbound, buf, 0), EDESTADDRREQ); TEST_ERRNO(send(sk_bound, buf, 1, 0), EDESTADDRREQ); + TEST_ERRNO(send(sk_bound, buf, 0, 0), EDESTADDRREQ); + TEST_ERRNO(write(sk_bound, buf, 1), EDESTADDRREQ); + TEST_ERRNO(write(sk_bound, buf, 0), EDESTADDRREQ); } END_TEST() @@ -103,10 +109,19 @@ FN_TEST(recv) char buf[1] = { 'z' }; TEST_ERRNO(recv(sk_unbound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_unbound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_unbound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_unbound, buf, 0)); TEST_ERRNO(recv(sk_bound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_bound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_bound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_bound, buf, 0)); TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_connected, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_connected, buf, 1), EAGAIN); + TEST_SUCC(read(sk_connected, buf, 0)); } END_TEST() diff --git a/test/apps/network/uevent_err.c b/test/apps/network/uevent_err.c index 28dd89c56..6d7d3eb65 100644 --- a/test/apps/network/uevent_err.c +++ b/test/apps/network/uevent_err.c @@ -84,9 +84,15 @@ FN_TEST(send) { char buf[1] = { 'z' }; - TEST_RES(send(sk_bound, buf, 1, 0), 1); + TEST_SUCC(send(sk_bound, buf, 1, 0)); + TEST_ERRNO(send(sk_bound, buf, 0, 0), ENODATA); + TEST_SUCC(write(sk_bound, buf, 1)); + TEST_ERRNO(write(sk_bound, buf, 0), ENODATA); TEST_ERRNO(send(sk_connected, buf, 1, 0), ECONNREFUSED); + TEST_ERRNO(send(sk_connected, buf, 0, 0), ENODATA); + TEST_ERRNO(write(sk_connected, buf, 1), ECONNREFUSED); + TEST_ERRNO(write(sk_connected, buf, 0), ENODATA); } END_TEST() @@ -95,10 +101,19 @@ FN_TEST(recv) char buf[1] = { 'z' }; TEST_ERRNO(recv(sk_unbound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_unbound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_unbound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_unbound, buf, 0)); TEST_ERRNO(recv(sk_bound, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_bound, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_bound, buf, 1), EAGAIN); + TEST_SUCC(read(sk_bound, buf, 0)); TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_connected, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_connected, buf, 1), EAGAIN); + TEST_SUCC(read(sk_connected, buf, 0)); } END_TEST() diff --git a/test/apps/network/unix_err.c b/test/apps/network/unix_err.c index 80035b374..7c3fe06f1 100644 --- a/test/apps/network/unix_err.c +++ b/test/apps/network/unix_err.c @@ -13,6 +13,12 @@ #include "test.h" +FN_SETUP(general) +{ + signal(SIGPIPE, SIG_IGN); +} +END_SETUP() + #define PATH_OFFSET offsetof(struct sockaddr_un, sun_path) FN_TEST(socket_addresses) @@ -114,13 +120,13 @@ static int sk_accepted; FN_SETUP(unbound) { - sk_unbound = CHECK(socket(PF_UNIX, SOCK_STREAM, 0)); + sk_unbound = CHECK(socket(PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)); } END_SETUP() FN_SETUP(bound) { - sk_bound = CHECK(socket(PF_UNIX, SOCK_STREAM, 0)); + sk_bound = CHECK(socket(PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)); CHECK(bind(sk_bound, (struct sockaddr *)&BOUND_ADDR, BOUND_ADDRLEN)); } @@ -128,7 +134,7 @@ END_SETUP() FN_SETUP(listen) { - sk_listen = CHECK(socket(PF_UNIX, SOCK_STREAM, 0)); + sk_listen = CHECK(socket(PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)); CHECK(bind(sk_listen, (struct sockaddr *)&LISTEN_ADDR, LISTEN_ADDRLEN)); @@ -138,7 +144,7 @@ END_SETUP() FN_SETUP(connected) { - sk_connected = CHECK(socket(PF_UNIX, SOCK_STREAM, 0)); + sk_connected = CHECK(socket(PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)); CHECK(connect(sk_connected, (struct sockaddr *)&LISTEN_ADDR2, LISTEN_ADDRLEN2)); @@ -321,10 +327,19 @@ FN_TEST(send) char buf[1] = { 'z' }; TEST_ERRNO(send(sk_unbound, buf, 1, 0), ENOTCONN); + TEST_ERRNO(send(sk_unbound, buf, 0, 0), ENOTCONN); + TEST_ERRNO(write(sk_unbound, buf, 1), ENOTCONN); + TEST_ERRNO(write(sk_unbound, buf, 0), ENOTCONN); TEST_ERRNO(send(sk_bound, buf, 1, 0), ENOTCONN); + TEST_ERRNO(send(sk_bound, buf, 0, 0), ENOTCONN); + TEST_ERRNO(write(sk_bound, buf, 1), ENOTCONN); + TEST_ERRNO(write(sk_bound, buf, 0), ENOTCONN); TEST_ERRNO(send(sk_listen, buf, 1, 0), ENOTCONN); + TEST_ERRNO(send(sk_listen, buf, 0, 0), ENOTCONN); + TEST_ERRNO(write(sk_listen, buf, 1), ENOTCONN); + TEST_ERRNO(write(sk_listen, buf, 0), ENOTCONN); } END_TEST() @@ -333,10 +348,24 @@ FN_TEST(recv) char buf[1] = { 'z' }; TEST_ERRNO(recv(sk_unbound, buf, 1, 0), EINVAL); + TEST_ERRNO(recv(sk_unbound, buf, 0, 0), EINVAL); + TEST_ERRNO(read(sk_unbound, buf, 1), EINVAL); + TEST_SUCC(read(sk_unbound, buf, 0)); TEST_ERRNO(recv(sk_bound, buf, 1, 0), EINVAL); + TEST_ERRNO(recv(sk_bound, buf, 0, 0), EINVAL); + TEST_ERRNO(read(sk_bound, buf, 1), EINVAL); + TEST_SUCC(read(sk_bound, buf, 0)); TEST_ERRNO(recv(sk_listen, buf, 1, 0), EINVAL); + TEST_ERRNO(recv(sk_listen, buf, 0, 0), EINVAL); + TEST_ERRNO(read(sk_listen, buf, 1), EINVAL); + TEST_SUCC(read(sk_listen, buf, 0)); + + TEST_ERRNO(recv(sk_connected, buf, 1, 0), EAGAIN); + TEST_ERRNO(recv(sk_connected, buf, 0, 0), EAGAIN); + TEST_ERRNO(read(sk_connected, buf, 1), EAGAIN); + TEST_SUCC(read(sk_connected, buf, 0)); } END_TEST() @@ -655,3 +684,38 @@ FN_SETUP(cleanup) CHECK(unlink(LISTEN_ADDR.sun_path)); } END_SETUP() + +// See also `zero_reads_always_succeed` in `pipe_err.c` +FN_TEST(zero_recvs_may_fail) +{ + int fildes[2]; + char buf[1] = { 'z' }; + + TEST_SUCC(socketpair(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0, fildes)); + + TEST_ERRNO(recv(fildes[0], buf, 0, 0), EAGAIN); + + TEST_RES(send(fildes[1], buf, 1, 0), _ret == 1); + TEST_SUCC(recv(fildes[0], buf, 0, 0)); + + TEST_SUCC(close(fildes[0])); + TEST_SUCC(close(fildes[1])); +} +END_TEST() + +// See also `zero_writes_always_succeed` in `pipe_err.c` +FN_TEST(zero_sends_may_fail) +{ + int fildes[2]; + char buf[1] = { 'z' }; + + TEST_SUCC(socketpair(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0, fildes)); + + TEST_SUCC(send(fildes[1], buf, 0, 0)); + + TEST_SUCC(close(fildes[0])); + TEST_ERRNO(send(fildes[1], buf, 0, 0), EPIPE); + + TEST_SUCC(close(fildes[1])); +} +END_TEST() diff --git a/test/apps/pipe/pipe_err.c b/test/apps/pipe/pipe_err.c index 49fd6d20e..2460cb3a9 100644 --- a/test/apps/pipe/pipe_err.c +++ b/test/apps/pipe/pipe_err.c @@ -176,3 +176,37 @@ FN_TEST(close_second_then_poll) TEST_SUCC(close(fildes[0])); } END_TEST() + +// See also `zero_recvs_may_fail` in `unix_err.c` +FN_TEST(zero_reads_always_succeed) +{ + int fildes[2]; + char buf[1] = { 'z' }; + + CHECK(pipe(fildes)); + + TEST_SUCC(read(fildes[0], buf, 0)); + + TEST_RES(write(fildes[1], buf, 1), _ret == 1); + TEST_SUCC(read(fildes[0], buf, 0)); + + TEST_SUCC(close(fildes[0])); +} +END_TEST() + +// See also `zero_sends_may_fail` in `unix_err.c` +FN_TEST(zero_writes_always_succeed) +{ + int fildes[2]; + char buf[1] = { 'z' }; + + CHECK(pipe(fildes)); + + TEST_SUCC(write(fildes[1], buf, 0)); + + TEST_SUCC(close(fildes[0])); + TEST_SUCC(write(fildes[1], buf, 0)); + + TEST_SUCC(close(fildes[1])); +} +END_TEST()