[Qemu-devel] Anyone familiar with the slirp code?

[Qemu-devel] Anyone familiar with the slirp code?

[Paul LeoNerd Evans]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

Hi all again,

I'm looking over the slirp code, trying to work out the exact nature of
this AMD64 bug. So far I've determined that some 32bit integer fields
are being used to store struct pointers in - very bad on AMD64 because
pointers are 64 bit. From what I can tell, I can't see how the code
would ever work on any 64 bit machine - can anyone here confirm if this
is indeed the case? What I observe is an immediate segfault of Qemu
itself when the guest OS closes a TCP socket.

What is further confusing me is that I can't find parts of code that
write many of the values; only bits that read them. For example, a
search for anything on the ti_next field gives only the following hits:

./slirp/tcp_input.c:137:            q = (struct tcpiphdr *)q->ti_next)
./slirp/tcp_input.c:170:                q = (struct tcpiphdr *)(q->ti_next);
./slirp/tcp_input.c:190:                q = (struct tcpiphdr *)q->ti_next;
./slirp/tcp_input.c:218:                ti = (struct tcpiphdr *)ti->ti_next;
./slirp/tcp_input.c:305:        ti->ti_next = ti->ti_prev = 0;
./slirp/tcp_subr.c:87:  n->ti_next = n->ti_prev = 0;
./slirp/tcp_subr.c:170: ti->ti_next = ti->ti_prev = 0;
./slirp/tcp_subr.c:288:         t = (struct tcpiphdr *)t->ti_next;
./slirp/tcpip.h:47:#define      ti_next         ti_i.ih_next

There are no hits for ih_next directly on its own, other than this macro
define, and some unrelated hits in the udp.c file. I can't see anywhere
where these values are initialised, which leads me to think there must
be something odd going on with different types of struct, and pointers
to them specifically cast about between the types, and members accessed
this way. It seems to me quite a fragile way to work....

Is anyone here familiar with the way the code is intended to run..? I
would like to get to the bottom of the issue, and fix it in a proper
robust way...

-- 
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk
ICQ# 4135350       |  Registered Linux# 179460
http://www.leonerd.org.uk/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Anyone familiar with the slirp code?

[John R. Hogerhuis]

I don't think the original author anticipated or cared about slirp being
ported to a 64-bit processor. I won't speak for the quality of the code
in general, but on a 32-bit machine the pointer size is 32-bit. It's
perfectly safe on that platform to use any 32-bit spot as a hidey hole
for your cookies.

Things like this is why porting from 32-bit to 64-bit is hard. Frankly I
wonder at the reason for increasingly higher word sizes on machines. Is
the bulk of our data these days really 64-bits long? But I digress...

Just go for it. The slirp code was imported into qemu. At this point
you're probably as much an expert as anyone. There is no upstream
maintainer for the code either, I looked and found and asked the last
sucker that had maintained it for a bit, and he just wanted to unload
it.

If you fix it though, be prepared for the fact that you will be the new
expert ;-)

One thing I'd like to see long term is to completely remove the NAT code
and replace it with something more modern and robust like netfilter.
That would give us a lot of nice application level gateways (nat
modules) for important protocols, and some tweakable firewall settings
for user-net.

While I'm wishing, in fact it would be a nice feature in general for
QEMU to have a built in firewall pointed at each host with fairly
minimal permissions by default. A windows machine on your network is a
windows machine on your network, virtual or not :-)

-- John.

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Paul LeoNerd Evans]

[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]

On Fri, Jul 22, 2005 at 09:47:13AM -0700, John R. Hogerhuis wrote:
> I don't think the original author anticipated or cared about slirp being
> ported to a 64-bit processor. I won't speak for the quality of the code
> in general, but on a 32-bit machine the pointer size is 32-bit. It's
> perfectly safe on that platform to use any 32-bit spot as a hidey hole
> for your cookies.

Well put.. :)

> Just go for it. The slirp code was imported into qemu. At this point
> you're probably as much an expert as anyone. There is no upstream
> maintainer for the code either, I looked and found and asked the last
> sucker that had maintained it for a bit, and he just wanted to unload
> it.

Well, in this case, it is almost worth pondering if a complete
from-the-top rewrite is required... It might be easiest to look at what
the code is meant to do, from a high level, and re-implement from
scratch. This way, all the required features can be put in from the
beginning, no extra stuff that's not required, and so on... I expect
large amounts of the code can be kept and modified, and the general
layout will remain, but a lot of the data structures will have to
go...

> If you fix it though, be prepared for the fact that you will be the new
> expert ;-)

Hhhmmmm.... I have numerous projects I'm already working on; adding one
more always gets tricky... But we'll see how things pan out...

> One thing I'd like to see long term is to completely remove the NAT code
> and replace it with something more modern and robust like netfilter.
> That would give us a lot of nice application level gateways (nat
> modules) for important protocols, and some tweakable firewall settings
> for user-net.
> 
> While I'm wishing, in fact it would be a nice feature in general for
> QEMU to have a built in firewall pointed at each host with fairly
> minimal permissions by default. A windows machine on your network is a
> windows machine on your network, virtual or not :-)

Well, as I said above, this sort of thing could be implemented with a
re-write....

Perhaps we could borrow some of the "iptables" code from the Linux
kernel, and use that..? It would be quite cool, I think, to have
something similar to iptables built into qemu...

-- 
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk
ICQ# 4135350       |  Registered Linux# 179460
http://www.leonerd.org.uk/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Gwenole Beauchesne]

Hi,

> From what I can tell, I can't see how the code
> would ever work on any 64 bit machine - can anyone here confirm if this
> is indeed the case?

AFAICS, slirp code in qemu cvs and other projects works on x86_64. 
Really try CVS instead of 0.7.0. ;-)

Bye,
Gwenolé.

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Jim C. Brown]

On Fri, Jul 22, 2005 at 08:45:16PM +0200, Gwenole Beauchesne wrote:
> AFAICS, slirp code in qemu cvs and other projects works on x86_64. 
> 
> Bye,
> Gwenol?.
> 

Nope.

-- 
Infinite complexity begets infinite beauty.
Infinite precision begets infinite perfection.

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Gwenole Beauchesne]

Le samedi, 23 jul 2005, à 00:05 Europe/Paris, Jim C. Brown a écrit :

> On Fri, Jul 22, 2005 at 08:45:16PM +0200, Gwenole Beauchesne wrote:
>> AFAICS, slirp code in qemu cvs and other projects works on x86_64.
>
> Nope.

Then please provide a clear testcase. My testing limited to 32-bit 
Ubuntu Live as guest on 64-bit Mandriva Linux 2005 and downloading 
files from FTP performed very well. The original author of this thread 
was using 0.7.0 and the patch he posted is not enough for 0.7.0 
(SIZEOF_CHAR_P was not defined so sizeof(structure) were bogus).

The current implementation in CVS will work as is on x86_64 because 
dynamically allocated data through malloc() and global data fit under 
32-bit. The former is true because malloc()'ed data is small enough to 
not use mmap() and reside in the data segment. Now, you will get 
crashes if you explicitly set an M_MMAP_THRESHOLD < sizeof(structures 
used in the slirp code).

A more complete solution than used in current CVS can use 
qemu_{malloc,realloc,*}() instead with special provisions to returning 
memory with addresses that fit under 32-bit. qemu already does this but 
only for (deprecated) qemu-fast at this time.

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Jim C. Brown]

On Sat, Jul 23, 2005 at 12:53:55AM +0200, Gwenole Beauchesne wrote:
> Le samedi, 23 jul 2005, ? 00:05 Europe/Paris, Jim C. Brown a ?crit :
> 
> >On Fri, Jul 22, 2005 at 08:45:16PM +0200, Gwenole Beauchesne wrote:
> >>AFAICS, slirp code in qemu cvs and other projects works on x86_64.
> >
> >Nope.
> My testing limited to 32-bit 
> Ubuntu Live as guest on 64-bit Mandriva Linux 2005 and downloading 
> files from FTP performed very well.

Well, I remember someone having issues with slirp and WinXP on an AMD64 host
a few months ago, but you're right that seems to be fixed now.

> 
> Then please provide a clear testcase.

Well, x86_64 on x86_64. But, rereading the emails, that doesn't cover this case,
which is x86 on x86_64.

> A more complete solution than used in current CVS can use 
> qemu_{malloc,realloc,*}() instead with special provisions to returning 
> memory with addresses that fit under 32-bit. qemu already does this but 
> only for (deprecated) qemu-fast at this time.
> 

Still not a very good substitute for fixing the slirp code to handle 64bit
values properly imho.

-- 
Infinite complexity begets infinite beauty.
Infinite precision begets infinite perfection.

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Adrian Smarzewski]

Jim C. Brown wrote:
> Well, I remember someone having issues with slirp and WinXP on an AMD64 host
> a few months ago, but you're right that seems to be fixed now.

It was me I think - winxp guest on linux host. I recompiled Qemu with
patch that was send by someone on the list and it's working now. I
didn't try cvs, but I can do it if you are not sure that cvs version is
correct.

-- 
Pozdrowienia,
Adrian Smarzewski

Re: [Qemu-devel] Anyone familiar with the slirp code?

[Josh Metzler]

On Friday 22 July 2005 11:51 am, Paul LeoNerd Evans wrote:
> Hi all again,
>
> I'm looking over the slirp code, trying to work out the exact nature of
> this AMD64 bug.
...
> Is anyone here familiar with the way the code is intended to run..? I
> would like to get to the bottom of the issue, and fix it in a proper
> robust way...

You might want to ask the colinux developers, too, as I believe they have 
been doing some work on slirp for their project.

Josh