Re: [coLinux-users] change in slirp behavior from 0.7.1 to 0.7.3 with udp forwarded ports

[Henry N. <hen...@ar...>]

Henry Nestler wrote:
> Mike Frysinger wrote:
>> On Mon, Jun 23, 2008 at 4:27 PM, Henry Nestler wrote:
>>> With rev 479/480 you pointed to a bigger set of changes.
>>>
>>> By the while I found the place where it goes wrong. Please see the
>>> source snip or:
>>> http://colinux.svn.sourceforge.net/viewvc/colinux/branches/devel/src/colinux/user/slirp/udp.c?revision=677&view=markup
>>>
>>> file udp.c, line 323, function udp_output()
>>>
>>> In the line 323 the source address saddr.sin_addr (192.168.x.x) is
>>> overwritten with 10.0.2.2
>>> One line before was checked the listen address (so->so_faddr).
>>> I'm afraid, there should check the source address instead? But why there
>>> is a check for broadcast? Is that a hack for a hack? It sems me, that it
>>> would better work without the lines from 322 to 326 there?
>>>
>>> Background:
>>> special_addr = 10.0.2.2 (const)
>>>
>>> All UDP sockets are listen on address 0.0.0.0, that address is set as
>>> faked address 10.0.2.2 in line 383:
>>> "so->so_faddr = alias_addr;"
>>>
>>> -----------------------------------
>>> devel/src/colinux/user/slirp/udp.c
>>>  315 int udp_output(struct socket *so, struct mbuf *m,
>>>  316                struct sockaddr_in *addr)
>>>  317
>>>  318 {
>>>  319     struct sockaddr_in saddr, daddr;
>>>  320
>>>  321     saddr = *addr;
>>>  322     if ((so->so_faddr.s_addr & htonl(0xffffff00)) ==
>>> special_addr.s_addr) {
>>>  323         saddr.sin_addr.s_addr = so->so_faddr.s_addr;
>>>  324         if ((so->so_faddr.s_addr & htonl(0x000000ff)) == htonl(0xff))
>>>  325             saddr.sin_addr.s_addr = alias_addr.s_addr;
>>>  326     }
>>>  327     daddr.sin_addr = so->so_laddr;
>>>  328     daddr.sin_port = so->so_lport;
>>>  329
>>>  330     return udp_output2(so, m, &saddr, &daddr, so->so_iptos);
>>>  331 }
>> i'd like to assist in analysis, but i'm not familiar with any of the
>> colinux source base, so i cant really even make an educated guess; i'd
>> be shooting blind.  i'll gladly test any proposed changes though.
>>
>> a comment block here in the source explaining what it's supposed to be
>> doing would have been useful :/.
> 
> Yes, I know. But is not from me. It was copied from some others.
> As I understand, after deep debugging with you helpful simple netcat and
> tcp example I would say, that the if condition wand to check if the
> source is from the host (localhost or the host's self ipaddress) and
> then it replaced the source from sender with faked 10.0.2.2
> 
> But after the change SVN rev 479/480 the address is fix as localhost or
> bind all times to all (0.0.0.0). In that case, this can not be use for
> that test any more.
> 
> Have fixed. Here is the new executable. Your Netcat + tcpcopy example
> was working:
> http://www.henrynestler.com/colinux/testing/stable-0.7.3/20080623-slirp/colinux-slirp-net-daemon-0.7.3-2.zip
> 
> I don't know, what the broadcast address (10.0.2.255) should do there.
> Have removed. This is the new code snip:
> 
> int udp_output(struct socket *so, struct mbuf *m,
>                struct sockaddr_in *addr)
> 
> {
>     struct sockaddr_in saddr, daddr;
> 
>     saddr = *addr;
> 
>     /* Translate connections from localhost to the alias hostname */
>     if(is_localhost(saddr.sin_addr)) {
>         saddr.sin_addr.s_addr = so->so_faddr.s_addr;
>     }
> 
>     daddr.sin_addr = so->so_laddr;
>     daddr.sin_port = so->so_lport;
> 
>     return udp_output2(so, m, &saddr, &daddr, so->so_iptos);
> }
> 

I'm afraid, we are not the only, that tries to fix this. See the last 3
changes 4402, 4267 and 4259 in Qemu:
http://svn.savannah.gnu.org/viewvc/trunk/slirp/udp.c?root=qemu&view=log

-- 
Henry N.