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