public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 1/4] udp: Fix inorrect use of IPv6 mh buffers in IPv4 path
Date: Thu, 24 Nov 2022 19:54:18 +1100	[thread overview]
Message-ID: <20221124085421.3027886-2-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20221124085421.3027886-1-david@gibson.dropbear.id.au>

udp_sock_handler() incorrectly uses udp6_l2_mh_tap[] on the IPv4 path.  In
fact this is harmless because this assignment is redundant (the 0th entry
msg_hdr will always point to the 0th iov entry for both IPv4 and IPv6 and
won't change).

There is also an incorrect usage of udp6_l2_mh_tap[] in
udp_sock_fill_data_v4.  This one can cause real problems, because we'll
use stale iov_len values if we send multiple messages to the qemu socket.
Most of the time that will be relatively harmless - we're likely to either
drop UDP packets, or send duplicates.  However, if the stale iov_len we
use ends up referencing an uninitialized buffer we could desynchronize the
qemu stream socket.

Correct both these bugs.  The UDP6 path appears to be correct, but it does
have some comments that incorrectly reference the IPv4 versions, so fix
those as well.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/udp.c b/udp.c
index ee5c2c5..99f9374 100644
--- a/udp.c
+++ b/udp.c
@@ -643,7 +643,7 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
 				  const struct timespec *now)
 {
-	struct msghdr *mh = &udp6_l2_mh_tap[*msg_idx].msg_hdr;
+	struct msghdr *mh = &udp4_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
 	size_t ip_len, buf_len;
 	in_port_t src_port;
@@ -717,9 +717,9 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 }
 
 /**
- * udp_sock_fill_data_v4() - Fill and queue one buffer. In pasta mode, write it
+ * udp_sock_fill_data_v6() - Fill and queue one buffer. In pasta mode, write it
  * @c:		Execution context
- * @n:		Index of buffer in udp4_l2_buf pool
+ * @n:		Index of buffer in udp6_l2_buf pool
  * @ref:	epoll reference from socket
  * @msg_idx:	Index within message being prepared (spans multiple buffers)
  * @msg_len:	Length of current message being prepared for sending
@@ -865,7 +865,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		if (n <= 0)
 			return;
 
-		udp6_l2_mh_tap[0].msg_hdr.msg_iov = &udp6_l2_iov_tap[0];
+		udp4_l2_mh_tap[0].msg_hdr.msg_iov = &udp4_l2_iov_tap[0];
 
 		for (i = 0; i < (unsigned)n; i++) {
 			udp_sock_fill_data_v4(c, i, ref,
-- 
@@ -643,7 +643,7 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 				  int *msg_idx, int *msg_bufs, ssize_t *msg_len,
 				  const struct timespec *now)
 {
-	struct msghdr *mh = &udp6_l2_mh_tap[*msg_idx].msg_hdr;
+	struct msghdr *mh = &udp4_l2_mh_tap[*msg_idx].msg_hdr;
 	struct udp4_l2_buf_t *b = &udp4_l2_buf[n];
 	size_t ip_len, buf_len;
 	in_port_t src_port;
@@ -717,9 +717,9 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
 }
 
 /**
- * udp_sock_fill_data_v4() - Fill and queue one buffer. In pasta mode, write it
+ * udp_sock_fill_data_v6() - Fill and queue one buffer. In pasta mode, write it
  * @c:		Execution context
- * @n:		Index of buffer in udp4_l2_buf pool
+ * @n:		Index of buffer in udp6_l2_buf pool
  * @ref:	epoll reference from socket
  * @msg_idx:	Index within message being prepared (spans multiple buffers)
  * @msg_len:	Length of current message being prepared for sending
@@ -865,7 +865,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,
 		if (n <= 0)
 			return;
 
-		udp6_l2_mh_tap[0].msg_hdr.msg_iov = &udp6_l2_iov_tap[0];
+		udp4_l2_mh_tap[0].msg_hdr.msg_iov = &udp4_l2_iov_tap[0];
 
 		for (i = 0; i < (unsigned)n; i++) {
 			udp_sock_fill_data_v4(c, i, ref,
-- 
2.38.1


  reply	other threads:[~2022-11-24  8:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-24  8:54 [PATCH 0/4] udp: Fix some confusion of IPv4 and IPv6 control structures David Gibson
2022-11-24  8:54 ` David Gibson [this message]
2022-11-24  8:54 ` [PATCH 2/4] udp: Better factor IPv4 and IPv6 paths in udp_sock_handler() David Gibson
2022-11-24  8:54 ` [PATCH 3/4] udp: Preadjust udp[46]_l2_iov_tap[].iov_base for pasta mode David Gibson
2022-11-24  8:54 ` [PATCH 4/4] udp: Factor out control structure management from udp_sock_fill_data_v[46] David Gibson
2022-11-25  2:01 ` [PATCH 0/4] udp: Fix some confusion of IPv4 and IPv6 control structures Stefano Brivio
2022-11-25  7:11   ` David Gibson
2022-12-06  6:47 ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221124085421.3027886-2-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).