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