Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-24 Thread Amos Jeffries

Bernhard Schmidt wrote:

Hi,


Oh drat. I mis-configured my new test setup. Sorry.
Revision 8903 has had a more careful set of tests done and fixes that 
problem.


Confirmed. At least I think so, there was quite some trial and error 
involved to force the dreaded bzr to sync the trees again (after 
altering it locally by reversing your commit). But I think I finally did 
it and the resulting binary works fine.


Hmm, I noticed that too yesterday. I think this is a cause to ask the 
experts for a recipe. I've started a thread 'bzr revert' to see if we 
can get any help on that.


Amos
--
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.


Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-24 Thread Alex Rousskov
On Mon, 2008-03-24 at 13:57 +0100, Henrik Nordstrom wrote:
> On Sun, 2008-03-23 at 23:20 +1200, Amos Jeffries wrote:
> > Fix memory leak in Linux builds.

bb:comment

Just testing whether voting works now. Please ignore.

Alex.




Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-24 Thread Bernhard Schmidt

Hi,


Oh drat. I mis-configured my new test setup. Sorry.
Revision 8903 has had a more careful set of tests done and fixes that 
problem.


Confirmed. At least I think so, there was quite some trial and error 
involved to force the dreaded bzr to sync the trees again (after 
altering it locally by reversing your commit). But I think I finally did 
it and the resulting binary works fine.


Bernhard


Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-24 Thread Henrik Nordstrom
On Sun, 2008-03-23 at 23:20 +1200, Amos Jeffries wrote:
> Fix memory leak in Linux builds.

bb:approve




Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-24 Thread Amos Jeffries

Bernhard Schmidt wrote:

Amos Jeffries <[EMAIL PROTECTED]> wrote:


Fix memory leak in Linux builds.


This one seems to break outgoing IPv4 connects (on a dualstacked Squid)

bzr trunk revision 8902 fails with 


| While trying to retrieve the URL: http://www.heise.de/
| 
| The following error was encountered:
| 
| * Write Error 
| 
| The system returned:
| 
| (32) Broken pipe
| 
| An error condition occurred while writing to the network. Please

| retry your request.

to all IPv4 hosts. IPv6 hosts work fine. Reverting this changeset with

$ bzr merge . --revision 8901..8900

fixes the issue.

/configure --prefix=/opt/squid3-ipv6 --enable-ipv6 --disable-carp
--disable-wccp --disable-wccpv2 --enable-epoll --disable-ident-lookups
--with-localhost-ipv6 --enable-async-io --enable-storeio=aufs

on Linux Debian unstable.

Bernhard



Oh drat. I mis-configured my new test setup. Sorry.
Revision 8903 has had a more careful set of tests done and fixes that 
problem.


Amos
--
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.


Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-23 Thread Bernhard Schmidt
Amos Jeffries <[EMAIL PROTECTED]> wrote:

> Fix memory leak in Linux builds.

This one seems to break outgoing IPv4 connects (on a dualstacked Squid)

bzr trunk revision 8902 fails with 

| While trying to retrieve the URL: http://www.heise.de/
| 
| The following error was encountered:
| 
| * Write Error 
| 
| The system returned:
| 
| (32) Broken pipe
| 
| An error condition occurred while writing to the network. Please
| retry your request.

to all IPv4 hosts. IPv6 hosts work fine. Reverting this changeset with

$ bzr merge . --revision 8901..8900

fixes the issue.

/configure --prefix=/opt/squid3-ipv6 --enable-ipv6 --disable-carp
--disable-wccp --disable-wccpv2 --enable-epoll --disable-ident-lookups
--with-localhost-ipv6 --enable-async-io --enable-storeio=aufs

on Linux Debian unstable.

Bernhard



Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-23 Thread Amos Jeffries

Amos Jeffries wrote:

Fix memory leak in Linux builds.



bb:approve



Re: [MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-23 Thread Bundle Buggy

Bundle Buggy has detected this merge request.

For details, see: 
http://squid-cache.org/bundlebuggy//request/%3C20080323112051.7AD57E6FE9%40treenet.co.nz%3E


[MERGE] Replace leaking addrinfo in comm_connect_addr() with sockaddr_storage

2008-03-23 Thread Amos Jeffries
Fix memory leak in Linux builds.
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: [EMAIL PROTECTED]
# target_branch: file:///src/squid/bzr/trunk/
# testament_sha1: 17d43699f1813e00a034e95f491c5dbb84d791c3
# timestamp: 2008-03-23 23:19:37 +1200
# base_revision_id: [EMAIL PROTECTED]
#   bsyr7gm4440gm4s1
# 
# Begin patch
=== modified file 'include/IPAddress.h'
--- include/IPAddress.h	2008-03-20 11:30:19 +
+++ include/IPAddress.h	2008-03-22 23:26:24 +
@@ -123,17 +123,13 @@
 /[EMAIL PROTECTED]/
 IPAddress& operator =(const IPAddress &s);
 IPAddress& operator =(IPAddress *s);
-
 IPAddress& operator =(struct sockaddr_in const &s);
-
+IPAddress& operator =(struct sockaddr_storage const &s);
 IPAddress& operator =(struct in_addr const &s);
 #if USE_IPV6
-
 IPAddress& operator =(struct in6_addr const &s);
-
 IPAddress& operator =(struct sockaddr_in6 const &s);
 #endif
-
 bool operator =(const struct hostent &s);
 bool operator =(const struct addrinfo &s);
 bool operator =(const char *s);
@@ -359,6 +355,8 @@
  * these functiosn WILL NOT be in the final public API after transition.
  */
 
+void GetSockAddr(struct sockaddr_storage &addr, const int family) const;
+
 /// \deprecated Deprecated for public use. Use IPAddress::GetAddrInfo()
 void GetSockAddr(struct sockaddr_in &) const;
 

=== modified file 'lib/IPAddress.cc'
--- lib/IPAddress.cc	2008-02-15 16:45:57 +
+++ lib/IPAddress.cc	2008-03-22 23:26:24 +
@@ -557,6 +557,24 @@
 return *this;
 };
 
+IPAddress& IPAddress::operator =(const sockaddr_storage &s)
+{
+#if USE_IPV6
+/* some AF_* magic to tell socket types apart and what we need to do */
+if(s.ss_family == AF_INET6) {
+memcpy(&m_SocketAddr, &s, sizeof(struct sockaddr_in));
+}
+else { // convert it to our storage mapping.
+struct sockaddr_in *sin = (struct sockaddr_in*)&s;
+m_SocketAddr.sin6_port = sin->sin_port;
+Map4to6( sin->sin_addr, m_SocketAddr.sin6_addr);
+}
+#else
+memcpy(&m_SocketAddr, &s, sizeof(struct sockaddr_in));
+#endif
+return *this;
+};
+
 void IPAddress::check4Mapped()
 {
   // obsolete.
@@ -1065,6 +1083,36 @@
 return buf;
 }
 
+void IPAddress::GetSockAddr(struct sockaddr_storage &addr, const int family) const
+{
+if( family == AF_INET && !IsIPv4()) {
+// FIXME INET6: caller using the wrong socket type!
+debugs(14, DBG_CRITICAL, HERE << "IPAddress::GetSockAddr : Cannot convert non-IPv4 to IPv4. from " << *this);
+assert(false);
+}
+
+#if USE_IPV6
+if( IsIPv6() ) {
+memcpy(&addr, &m_SocketAddr, sizeof(struct sockaddr_in6));
+
+if(addr.ss_family == 0)
+addr.ss_family = AF_INET6;
+}
+else if( IsIPv4() ) {
+struct sockaddr_in *sin = (struct sockaddr_in*)&addr;
+addr.ss_family = AF_INET;
+sin->sin_port = m_SocketAddr.sin6_port;
+Map6to4( m_SocketAddr.sin6_addr, sin->sin_addr);
+}
+#else
+struct sockaddr_in *sa = (struct sockaddr_in*)&addr;
+memcpy(&addr, &m_SocketAddr, sizeof(struct sockaddr_in));
+
+if(addr.sa_family == 0)
+addr.sa_family = AF_INET;
+#endif
+}
+
 void IPAddress::GetSockAddr(struct sockaddr_in &buf) const
 {
 #if USE_IPV6

=== modified file 'src/comm.cc'
--- src/comm.cc	2008-02-27 04:49:32 +
+++ src/comm.cc	2008-03-23 10:39:43 +
@@ -1133,15 +1133,16 @@
 int x = 0;
 int err = 0;
 socklen_t errlen;
-struct addrinfo *AI = NULL;
+struct sockaddr_storage sas;
+socklen_t slen = sizeof(struct sockaddr_storage);
 PROF_start(comm_connect_addr);
 
 assert(address.GetPort() != 0);
 
 debugs(5, 9, "comm_connect_addr: connecting socket " << sock << " to " << address << " (want family: " << F->sock_family << ")");
 
-/* FIXME INET6 : Bug : when sock is an IPv4-only socket IPv6 traffic will crash. */
-address.GetAddrInfo(AI, F->sock_family);
+memset(&sas, NULL, slen);
+address.GetSockAddr(sas, F->sock_family);
 
 /* Establish connection. */
 errno = 0;
@@ -1151,7 +1152,7 @@
 F->flags.called_connect = 1;
 statCounter.syscalls.sock.connects++;
 
-x = connect(sock, AI->ai_addr, AI->ai_addrlen);
+x = connect(sock, (struct sockaddr*)&sas, slen);
 
 // XXX: ICAP code refuses callbacks during a pending comm_ call
 // Async calls development will fix this.
@@ -1163,12 +1164,8 @@
 if (x < 0)
 {
 debugs(5,5, "comm_connect_addr: sock=" << sock << ", addrinfo( " <<
- " flags=" << AI->ai_flags <<
- ", family=" << AI->ai_family <<
- ", socktype=" << AI->ai_socktype <<
- ", protocol=" << AI->ai_protocol <<
- ", &addr=" << AI->ai_addr <<
- ", addrlen=" << AI->ai_addrlen <<
+ ", family=" << sas.ss_family <<
+