In qemu-0.8.0.20060327, there are three problems with sending large
packets from guest to host:
(1) the code in slirp's ip_reass() reads a next pointer out an mbuf
after freeing it via m_cat().
(2) the code in slirp's m_inc() calls realloc() on a large mbuf, but
fails to adjust m_data to point to the new allocation (see
http://lists.gnu.org/archive/html/qemu-devel/2005-05/msg00228.html).
(3) there are many places within ip_input(), ip_reass(),
udp_input(), etc., that treat ip_len and ip_off as though they were
declared unsigned, when in fact they have been declared signed.
Patches fixing these problems are attached. I hope they can be
applied. Please let me know what I can do to make the patches more
likely to be accepted.
Thanks,
-Ken
diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c
qemu-snapshot-2006-03-27_23/slirp/ip_input.c
--- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-22
00:10:47.000000000 +0000
+++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06
06:02:52.000000000 +0000
@@ -344,8 +344,8 @@
while (q != (struct ipasfrag *)fp) {
struct mbuf *t;
t = dtom(q);
- m_cat(m, t);
q = (struct ipasfrag *) q->ipf_next;
+ m_cat(m, t);
}
/*
diff -BurN qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c
qemu-snapshot-2006-03-27_23/slirp/mbuf.c
--- qemu-snapshot-2006-03-27_23.orig/slirp/mbuf.c 2004-04-22
00:10:47.000000000 +0000
+++ qemu-snapshot-2006-03-27_23/slirp/mbuf.c 2006-04-05 13:03:03.000000000
+0000
@@ -146,18 +146,19 @@
struct mbuf *m;
int size;
{
+ int datasize;
+
/* some compiles throw up on gotos. This one we can fake. */
if(m->m_size>size) return;
if (m->m_flags & M_EXT) {
- /* datasize = m->m_data - m->m_ext; */
+ datasize = m->m_data - m->m_ext;
m->m_ext = (char *)realloc(m->m_ext,size);
/* if (m->m_ext == NULL)
* return (struct mbuf *)NULL;
*/
- /* m->m_data = m->m_ext + datasize; */
+ m->m_data = m->m_ext + datasize;
} else {
- int datasize;
char *dat;
datasize = m->m_data - m->m_dat;
dat = (char *)malloc(size);
diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip.h
qemu-snapshot-2006-03-27_23/slirp/ip.h
--- qemu-snapshot-2006-03-27_23.orig/slirp/ip.h 2004-04-21 17:10:47.000000000
-0700
+++ qemu-snapshot-2006-03-27_23/slirp/ip.h 2006-04-06 00:28:49.000000000
-0700
@@ -79,6 +79,11 @@
* We declare ip_len and ip_off to be short, rather than u_short
* pragmatically since otherwise unsigned comparisons can result
* against negative integers quite easily, and fail in subtle ways.
+ *
+ * The only problem with the above theory is that these quantities
+ * are in fact unsigned, and sorting fragments by a signed version
+ * of ip_off doesn't work very well, nor does length checks on
+ * ip packets with a signed version of their length!
*/
struct ip {
#ifdef WORDS_BIGENDIAN
@@ -101,6 +106,9 @@
struct in_addr ip_src,ip_dst; /* source and dest address */
};
+#define IP_OFF(ip) (*(u_int16_t *)&((ip)->ip_off))
+#define IP_LEN(ip) (*(u_int16_t *)&((ip)->ip_len))
+
#define IP_MAXPACKET 65535 /* maximum packet size */
/*
diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c
qemu-snapshot-2006-03-27_23/slirp/ip_input.c
--- qemu-snapshot-2006-03-27_23.orig/slirp/ip_input.c 2004-04-21
17:10:47.000000000 -0700
+++ qemu-snapshot-2006-03-27_23/slirp/ip_input.c 2006-04-06
00:32:19.000000000 -0700
@@ -111,7 +111,7 @@
* Convert fields to host representation.
*/
NTOHS(ip->ip_len);
- if (ip->ip_len < hlen) {
+ if (IP_LEN(ip) < hlen) {
ipstat.ips_badlen++;
goto bad;
}
@@ -124,13 +124,13 @@
* Trim mbufs if longer than we expect.
* Drop packet if shorter than we expect.
*/
- if (m->m_len < ip->ip_len) {
+ if (m->m_len < IP_LEN(ip)) {
ipstat.ips_tooshort++;
goto bad;
}
/* Should drop packet if mbuf too long? hmmm... */
- if (m->m_len > ip->ip_len)
- m_adj(m, ip->ip_len - m->m_len);
+ if (m->m_len > IP_LEN(ip))
+ m_adj(m, IP_LEN(ip) - m->m_len);
/* check ip_ttl for a correct ICMP reply */
if(ip->ip_ttl==0 || ip->ip_ttl==1) {
@@ -191,7 +191,7 @@
* or if this is not the first fragment,
* attempt reassembly; if it succeeds, proceed.
*/
- if (((struct ipasfrag *)ip)->ipf_mff & 1 || ip->ip_off) {
+ if (((struct ipasfrag *)ip)->ipf_mff & 1 || IP_OFF(ip)) {
ipstat.ips_fragments++;
ip = ip_reass((struct ipasfrag *)ip, fp);
if (ip == 0)
@@ -281,7 +281,7 @@
*/
for (q = (struct ipasfrag *)fp->ipq_next; q != (struct ipasfrag *)fp;
q = (struct ipasfrag *)q->ipf_next)
- if (q->ip_off > ip->ip_off)
+ if (IP_OFF(q) > IP_OFF(ip))
break;
/*
@@ -290,10 +290,10 @@
* segment. If it provides all of our data, drop us.
*/
if (q->ipf_prev != (ipasfragp_32)fp) {
- i = ((struct ipasfrag *)(q->ipf_prev))->ip_off +
- ((struct ipasfrag *)(q->ipf_prev))->ip_len - ip->ip_off;
+ i = IP_OFF((struct ipasfrag *)(q->ipf_prev)) +
+ IP_LEN((struct ipasfrag *)(q->ipf_prev)) - IP_OFF(ip);
if (i > 0) {
- if (i >= ip->ip_len)
+ if (i >= IP_LEN(ip))
goto dropfrag;
m_adj(dtom(ip), i);
ip->ip_off += i;
@@ -305,9 +305,9 @@
* While we overlap succeeding segments trim them or,
* if they are completely covered, dequeue them.
*/
- while (q != (struct ipasfrag *)fp && ip->ip_off + ip->ip_len >
q->ip_off) {
- i = (ip->ip_off + ip->ip_len) - q->ip_off;
- if (i < q->ip_len) {
+ while (q != (struct ipasfrag *)fp && IP_OFF(ip) + IP_LEN(ip) >
IP_OFF(q)) {
+ i = (IP_OFF(ip) + IP_LEN(ip)) - IP_OFF(q);
+ if (i < IP_LEN(q)) {
q->ip_len -= i;
q->ip_off += i;
m_adj(dtom(q), i);
@@ -327,9 +327,9 @@
next = 0;
for (q = (struct ipasfrag *) fp->ipq_next; q != (struct ipasfrag *)fp;
q = (struct ipasfrag *) q->ipf_next) {
- if (q->ip_off != next)
+ if (IP_OFF(q) != next)
return (0);
- next += q->ip_len;
+ next += IP_LEN(q);
}
if (((struct ipasfrag *)(q->ipf_prev))->ipf_mff & 1)
return (0);
diff -burN qemu-snapshot-2006-03-27_23.orig/slirp/udp.c
qemu-snapshot-2006-03-27_23/slirp/udp.c
--- qemu-snapshot-2006-03-27_23.orig/slirp/udp.c 2006-04-06
00:24:30.000000000 -0700
+++ qemu-snapshot-2006-03-27_23/slirp/udp.c 2006-04-06 00:32:55.000000000
-0700
@@ -111,12 +111,12 @@
*/
len = ntohs((u_int16_t)uh->uh_ulen);
- if (ip->ip_len != len) {
- if (len > ip->ip_len) {
+ if (IP_LEN(ip) != len) {
+ if (len > IP_LEN(ip)) {
udpstat.udps_badlen++;
goto bad;
}
- m_adj(m, len - ip->ip_len);
+ m_adj(m, len - IP_LEN(ip));
ip->ip_len = len;
}
_______________________________________________
Qemu-devel mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/qemu-devel
