From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.3 (2019-12-06) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=0.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RDNS_NONE,SPF_HELO_PASS,WEIRD_PORT autolearn=unavailable autolearn_force=no version=3.4.3 Received: by atuin.qyliss.net (Postfix, from userid 496) id 2C5ED1825D; Mon, 6 Jul 2020 16:01:50 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 9D1EB18166; Mon, 6 Jul 2020 16:01:44 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 496) id D96881815D; Mon, 6 Jul 2020 16:01:42 +0000 (UTC) Received: from out1-smtp.messagingengine.com (unknown [66.111.4.25]) by atuin.qyliss.net (Postfix) with ESMTPS id 22B951823F for ; Mon, 6 Jul 2020 16:01:39 +0000 (UTC) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 358175C006B; Mon, 6 Jul 2020 12:01:23 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 06 Jul 2020 12:01:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h= from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; s=fm3; bh=jO0nPONi65dwabbROyg9U4fsMQ qksUyAdFWrLqj8kqo=; b=Xd3NH6VYYhGrQbUQtb5ealU0HfDoqcPJs1OJLqolmA wCCQfv1XYCZ7N3PT1md6il6py80/T+ZXAlYm3BQHIBfDx6KRPlhN2dBPdR7tosFb xHSm4/EAW2PpubB2ZqKdldssY1hxdLnpu2LvzbxQQY2PRqJFzZQ6dhF4RayUkwSJ cMc2XDeLYGDYWBOl6VPg6reboJM7VgA7b8l70VEYQ9j5uIDxhIFEwnLKzw41/OTY tSEFAX0iBDCyXFi5CDwNItIwvn/VJUQoeN5VdM3jyi7q5O4cMKrtTpGeB0iPMggf BtQkSf6EhJeJEuMNCrA0/I6dW/op22kYJ4RZ7P9R0gBw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :message-id:mime-version:subject:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=jO0nPONi65dwabbRO yg9U4fsMQqksUyAdFWrLqj8kqo=; b=JTPgLUlZxqsCPvD7uTRYaiQFodgmAMZD4 TzHJDUF6bCn11M8uajAC1km4sxxhularHDm2zEQ8afX0HU2FvFsi8/4/zzk9zD4V rkyHCnr1UVm08b+XX5/y8zm9fmEXBc7+804U6GQa1rHXtYTVTi+2/Nc+BHXCpx8s S8pRYF67+h9jcckOzhVCVC9SFAjuNsxQEhoet4qGjpyrMZarUV6y4EewoNzb9olE XU7lbM608N6vSqFpr7HQlLa2R+Ltx4HJwxA+64YXIx4eHgpb1dNmncV6nDN/cjJO w71NsZEPgQFxQFrVCK/gz3Zr7tIP5xXQTMQbUpsmhUDkNvZyNft0w== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrudefgdelkecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkofgggfestdekredtredttdenucfhrhhomheptehlhihsshgrucft ohhsshcuoehhihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhepueegfeehff eutdfgjeeuveeiiedufffgleejvefhudeivdeuvddtuddvheeiudejnecuffhomhgrihhn pehsphgvtghtrhhumhdqohhsrdhorhhgnecukfhppeejledrvdefhedruddviedrudejle enucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehqhihl ihhsshesgidvvddtrdhqhihlihhsshdrnhgvth X-ME-Proxy: Received: from x220.qyliss.net (p4feb7eb3.dip0.t-ipconnect.de [79.235.126.179]) by mail.messagingengine.com (Postfix) with ESMTPA id 20488328006A; Mon, 6 Jul 2020 12:01:22 -0400 (EDT) Received: by x220.qyliss.net (Postfix, from userid 1000) id 9E656483; Mon, 6 Jul 2020 16:01:20 +0000 (UTC) From: Alyssa Ross To: devel@spectrum-os.org Subject: [PATCH crosvm 1/2] sys_util: make UnixSeqpacket::recv use libc::recv Date: Mon, 6 Jul 2020 15:59:13 +0000 Message-Id: <20200706155913.18065-1-hi@alyssa.is> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 4OB2I5ZFYUPXES7FCOKFIUKA5XZGJKAS X-Message-ID-Hash: 4OB2I5ZFYUPXES7FCOKFIUKA5XZGJKAS X-MailFrom: qyliss@x220.qyliss.net X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: Cole Helbling X-Mailman-Version: 3.3.1 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: It's very surprising that a method called "recv" would end up calling read(). When UnixSeqpacket was introduced in 1d44223f9, the method was called "read", so this behaviour made more sense. It was renamed to "recv" in b7196e2a1, but the implementation was not changed to call libc::recv. In most cases, according to recv(2), the difference between read() and recv() doesn't matter: > The only difference between recv() and read(2) is the presence of > flags. With a zero flags argument, recv() is generally equivalent > to read(2) (but see NOTES). But the NOTES section explains that > If a zero-length datagram is pending, read(2) and recv() with a > flags argument of zero provide different behavior. In this > circumstance, read(2) has no effect (the datagram remains pending), > while recv() consumes the pending datagram. This means that, while we might expect UnixSeqpacket::recv(&mut []) to mirror the behaviour of recv() and discard a message despite the empty buffer, it would actually leave it there. This became relevant when I implemented the startup message queue in 5bd2e7f60, because I used an empty recv to drop the ready message from the socket, since it was only used for polling and I didn't care about the content. But, it turns out that my UnixSeqpacket::recv call did nothing, and left the message on the socket. This wasn't immediately noticeable, but it meant that when a message came through on the control socket later, it would actually get the ready message from UnixSeqpacket::recv, rather than the expected message on the control socket. For example, trying to use crosvm disk resize would print a message like the following: > [ERROR:vm_control/src/lib.rs:649] unexpected disk socket result: Ready > [ERROR:devices/src/virtio/block.rs:334] Attempted to resize read-only b= lock device We can see that it does try to process the received message, but first it tries to do something with the Ready message, which should have been discarded by now. With this fix applied, we can see that it now receives and tries to process the correct message (but then fails because the VM I tested didn't support disk resizing!): > [ERROR:devices/src/virtio/block.rs:334] Attempted to resize read-only b= lock device Fixes: b7196e2a1c1eb7123e7eace5418b7eb4a3e24dbe Fixes: 5bd2e7f606b726b598fdc26fe26a9b4029fab6d3 Cc: Cole Helbling --- This is the fix I described in This Week in Spectrum[1]. [1]: https://spectrum-os.org/lists/archives/spectrum-discuss/8736654ij2.f= sf@alyssa.is/ sys_util/src/net.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sys_util/src/net.rs b/sys_util/src/net.rs index 71ab3eeb5..82b4f5bb0 100644 --- a/sys_util/src/net.rs +++ b/sys_util/src/net.rs @@ -197,7 +197,7 @@ impl UnixSeqpacket { } } =20 - /// Read data from the socket fd to a given buffer + /// Receive data from the socket fd to a given buffer. /// /// # Arguments /// * `buf` - A mut reference to the data buffer. @@ -206,11 +206,11 @@ impl UnixSeqpacket { /// * `usize` - The size of bytes read to the buffer. /// /// # Errors - /// Returns error when `libc::read` failed. + /// Returns error when `libc::recv` failed. pub fn recv(&self, buf: &mut [u8]) -> io::Result { // Safe since we make sure the input `count` =3D=3D `buf.len()` = and handle the returned error. unsafe { - let ret =3D libc::read(self.fd, buf.as_mut_ptr() as *mut _, = buf.len()); + let ret =3D libc::recv(self.fd, buf.as_mut_ptr() as *mut _, = buf.len(), 0); if ret < 0 { Err(io::Error::last_os_error()) } else { --=20 2.26.2