9ae3a8
From 5e805decf566e71cd315243de9a037d99becb074 Mon Sep 17 00:00:00 2001
9ae3a8
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com>
9ae3a8
Date: Mon, 20 May 2019 16:43:15 +0200
9ae3a8
Subject: [PATCH 4/4] slirp: don't manipulate so_rcv in tcp_emu()
9ae3a8
MIME-Version: 1.0
9ae3a8
Content-Type: text/plain; charset=UTF-8
9ae3a8
Content-Transfer-Encoding: 8bit
9ae3a8
9ae3a8
RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>
9ae3a8
Message-id: <20190520164315.22140-4-marcandre.lureau@redhat.com>
9ae3a8
Patchwork-id: 88095
9ae3a8
O-Subject: [RHEL-7.7 qemu-kvm PATCH 3/3] slirp: don't manipulate so_rcv in tcp_emu()
9ae3a8
Bugzilla: 1669068
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
9ae3a8
RH-Acked-by: Thomas Huth <thuth@redhat.com>
9ae3a8
9ae3a8
For some reason, EMU_IDENT is not like other "emulated" protocols and
9ae3a8
tries to reconstitute the original buffer, if it came in multiple
9ae3a8
packets. Unfortunately, it does so wrongly, as it doesn't respect the
9ae3a8
sbuf circular buffer appending rules, nor does it maintain some of the
9ae3a8
invariants (rptr is incremented without bounds, etc): this leads to
9ae3a8
further memory corruption revealed by ASAN or various malloc
9ae3a8
errors. Furthermore, the so_rcv buffer is regularly flushed, so there
9ae3a8
is no guarantee that buffer reconstruction will do what is expected.
9ae3a8
9ae3a8
Instead, do what the function comment says: "XXX Assumes the whole
9ae3a8
command came in one packet", and don't touch so_rcv.
9ae3a8
9ae3a8
Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1664205
9ae3a8
9ae3a8
Cc: Prasad J Pandit <pjp@fedoraproject.org>
9ae3a8
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
9ae3a8
9ae3a8
(cherry picked from libslirp commit
9ae3a8
9da0da837780f825b5db31db6620492f8b7cd5d6)
9ae3a8
9ae3a8
[ MA - backported with style conflicts, and without qemu commit
9ae3a8
a7104eda7dab99d0cdbd3595c211864cba415905 which is unnecessary with
9ae3a8
this patch ]
9ae3a8
9ae3a8
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 slirp/tcp_subr.c | 63 +++++++++++++++++++++++++++-----------------------------
9ae3a8
 1 file changed, 30 insertions(+), 33 deletions(-)
9ae3a8
9ae3a8
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
9ae3a8
index 578b204..d49a366 100644
9ae3a8
--- a/slirp/tcp_subr.c
9ae3a8
+++ b/slirp/tcp_subr.c
9ae3a8
@@ -581,42 +581,39 @@ tcp_emu(struct socket *so, struct mbuf *m)
9ae3a8
 			struct socket *tmpso;
9ae3a8
 			struct sockaddr_in addr;
9ae3a8
 			socklen_t addrlen = sizeof(struct sockaddr_in);
9ae3a8
-			struct sbuf *so_rcv = &so->so_rcv;
9ae3a8
-
9ae3a8
-			memcpy(so_rcv->sb_wptr, m->m_data, m->m_len);
9ae3a8
-			so_rcv->sb_wptr += m->m_len;
9ae3a8
-			so_rcv->sb_rptr += m->m_len;
9ae3a8
-			m_inc(m, m->m_len + 1);
9ae3a8
-			m->m_data[m->m_len] = 0; /* NULL terminate */
9ae3a8
-			if (strchr(m->m_data, '\r') || strchr(m->m_data, '\n')) {
9ae3a8
-				if (sscanf(so_rcv->sb_data, "%u%*[ ,]%u", &n1, &n2) == 2) {
9ae3a8
-					HTONS(n1);
9ae3a8
-					HTONS(n2);
9ae3a8
-					/* n2 is the one on our host */
9ae3a8
-					for (tmpso = slirp->tcb.so_next;
9ae3a8
-					     tmpso != &slirp->tcb;
9ae3a8
-					     tmpso = tmpso->so_next) {
9ae3a8
-						if (tmpso->so_laddr.s_addr == so->so_laddr.s_addr &&
9ae3a8
-						    tmpso->so_lport == n2 &&
9ae3a8
-						    tmpso->so_faddr.s_addr == so->so_faddr.s_addr &&
9ae3a8
-						    tmpso->so_fport == n1) {
9ae3a8
-							if (getsockname(tmpso->s,
9ae3a8
-								(struct sockaddr *)&addr, &addrlen) == 0)
9ae3a8
-							   n2 = addr.sin_port;
9ae3a8
-							break;
9ae3a8
-						}
9ae3a8
+			char *eol = g_strstr_len(m->m_data, m->m_len, "\r\n");
9ae3a8
+
9ae3a8
+			if (!eol) {
9ae3a8
+				return 1;
9ae3a8
+			}
9ae3a8
+
9ae3a8
+			*eol = '\0';
9ae3a8
+			if (sscanf(m->m_data, "%u%*[ ,]%u", &n1, &n2) == 2) {
9ae3a8
+				HTONS(n1);
9ae3a8
+				HTONS(n2);
9ae3a8
+				/* n2 is the one on our host */
9ae3a8
+				for (tmpso = slirp->tcb.so_next; tmpso != &slirp->tcb;
9ae3a8
+					 tmpso = tmpso->so_next) {
9ae3a8
+					if (tmpso->so_laddr.s_addr == so->so_laddr.s_addr &&
9ae3a8
+						tmpso->so_lport == n2 &&
9ae3a8
+						tmpso->so_faddr.s_addr == so->so_faddr.s_addr &&
9ae3a8
+						tmpso->so_fport == n1) {
9ae3a8
+						if (getsockname(tmpso->s, (struct sockaddr *)&addr,
9ae3a8
+										&addrlen) == 0)
9ae3a8
+							n2 = addr.sin_port;
9ae3a8
+						break;
9ae3a8
 					}
9ae3a8
-					NTOHS(n1);
9ae3a8
-					NTOHS(n2);
9ae3a8
-					so_rcv->sb_cc = snprintf(so_rcv->sb_data,
9ae3a8
-								 so_rcv->sb_datalen,
9ae3a8
-								 "%d,%d\r\n", n1, n2);
9ae3a8
-					so_rcv->sb_rptr = so_rcv->sb_data;
9ae3a8
-					so_rcv->sb_wptr = so_rcv->sb_data + so_rcv->sb_cc;
9ae3a8
 				}
9ae3a8
+				NTOHS(n1);
9ae3a8
+				NTOHS(n2);
9ae3a8
+				m_inc(m, snprintf(NULL, 0, "%d,%d\r\n", n1, n2) + 1);
9ae3a8
+				m->m_len = snprintf(m->m_data, M_ROOM(m), "%d,%d\r\n", n1, n2);
9ae3a8
+				assert(m->m_len < M_ROOM(m));
9ae3a8
+			} else {
9ae3a8
+				*eol = '\r';
9ae3a8
 			}
9ae3a8
-			m_free(m);
9ae3a8
-			return 0;
9ae3a8
+
9ae3a8
+			return 1;
9ae3a8
 		}
9ae3a8
 
9ae3a8
         case EMU_FTP: /* ftp */
9ae3a8
-- 
9ae3a8
1.8.3.1
9ae3a8