Package: squid3
Version: 3.4.8-6+deb8u1
Followup-For: Bug #728144

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


I see that there is some confusion about whether or not this bug is fixed,
and which of many pinger sigsegv bugs this is all about.  So to be clear:

- - the additional info I provide here concerns the version currently in
  Debian jessie (stable), which is 3.4.8-6+deb8u1
- - the crash I see is the "IcmpPinger.cc:190 (debugFinish)" one, but the
  same bug is likely to trigger a similar crash anywhere "debugs" is used
- - this bug is fixed upstream, but it is *not* fixed in the  Debian jessie
  squid version, which is what this report is about.

The bug trivial to reproduce:
1) run the Debian jessie squid3 (version 3.4.8-6+deb8u1) on a dual stack
   system with default pinger config
2) stop squid.

That's all. The "debugs" statement attempting to log "Pinger exiting." at
IcmpPinger.cc:190 will crash.

Sample backtrace:

(gdb) bt full
#0  0x00007f46f854bb05 in malloc_consolidate (av=av@entry=0x7f46f8876620 
<main_arena>) at malloc.c:4165
        fb = <optimized out>
        maxfb = 0x7f46f8876670 <main_arena+80>
        p = 0x7f46fb825110
        nextp = 0x0
        unsorted_bin = 0x7f46f8876678 <main_arena+88>
        first_unsorted = <optimized out>
        nextchunk = 0x7f46fb825130
        size = 336
        nextsize = 304
        prevsize = <optimized out>
        nextinuse = 0
        bck = 0x23e7f57b0081
        fwd = 0x23e7f57c0080
        __func__ = "malloc_consolidate"
#1  0x00007f46f854c691 in _int_free (av=0x7f46f8876620 <main_arena>, 
p=<optimized out>, have_lock=0) at malloc.c:4057
        size = 132400
        fb = <optimized out>
        nextchunk = 0x7f46fb825d00
        nextsize = 131840
        nextinuse = <optimized out>
        prevsize = <optimized out>
        bck = <optimized out>
        fwd = <optimized out>
        errstr = 0x0
        locked = 1
        __func__ = "_int_free"
#2  0x00007f46f8e2e604 in std::basic_ostringstream<char, 
std::char_traits<char>, std::allocator<char> >::~basic_ostringstream() ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#3  0x00007f46fa135f27 in Debug::finishDebug () at ../../src/debug.cc:762
No locals.
#4  0x00007f46fa132ea4 in IcmpPinger::Recv (this=0x7f46fa344660 <control>) at 
IcmpPinger.cc:190
        _dbo = @0x7f46fb825970: <incomplete type>
        pecho = {to = {mSocketAddr_ = {sin6_family = 0, sin6_port = 0, 
sin6_flowinfo = 0, sin6_addr = {__in6_u = {
                  __u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 
0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}, 
              sin6_scope_id = 0}, static STRLEN_IP4A = 16, static STRLEN_IP4R = 
28, static STRLEN_IP4S = 21, 
            static MAX_IP4_STRLEN = 28, static STRLEN_IP6A = 42, static 
STRLEN_IP6R = 75, static STRLEN_IP6S = 48, 
            static MAX_IP6_STRLEN = 75, static v4_localhost = {__in6_u = {
- ---Type <return> to continue, or q <return> to quit---
                __u6_addr8 = 
"\000\000\000\000\000\000\000\000\000\000\377\377\177\000\000\001", __u6_addr16 
= {0, 0, 0, 0, 0, 
                  65535, 127, 256}, __u6_addr32 = {0, 0, 4294901760, 
16777343}}}, static v4_anyaddr = {__in6_u = {
                __u6_addr8 = 
"\000\000\000\000\000\000\000\000\000\000\377\377\000\000\000", __u6_addr16 = 
{0, 0, 0, 0, 0, 65535, 
                  0, 0}, __u6_addr32 = {0, 0, 4294901760, 0}}}, static 
v4_noaddr = {__in6_u = {
                __u6_addr8 = 
"\000\000\000\000\000\000\000\000\000\000\377\377\377\377\377\377", __u6_addr16 
= {0, 0, 0, 0, 0, 
                  65535, 65535, 65535}, __u6_addr32 = {0, 0, 4294901760, 
4294967295}}}, static v6_noaddr = {__in6_u = {
                __u6_addr8 = '\377' <repeats 16 times>, __u6_addr16 = {65535, 
65535, 65535, 65535, 65535, 65535, 65535, 65535}, 
                __u6_addr32 = {4294967295, 4294967295, 4294967295, 
4294967295}}}}, opcode = 0 '\000', psize = 0, 
          payload = '\000' <repeats 8191 times>}
        n = <optimized out>
        guess_size = <optimized out>
        __FUNCTION__ = "Recv"
#5  0x00007f46fa1318c3 in main (argc=<optimized out>, argv=<optimized out>) at 
pinger.cc:223
        t = <optimized out>


I took a quick look at upstream and noticed this commit, which appears to have
resolved the issue once and for all:

- ----
- From 41b2578a9ee6b692869b9d962197cf6e3773898e Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squ...@treenet.co.nz>
Date: Fri, 19 Dec 2014 08:26:44 -0800
Subject: [PATCH] MemPool the debug output stream buffers

The CurrentDebug output stream controller for cache.log was
defined as a std::ostringstream object and allocated with
new/delete on each call to debugs().

The std::ostringstream is defined as a templates output stream
which uses the std::allocator<char> built into libc when its
new()'d. Since this is all internal to the STL library
definitions it links against the libc global-scope allocator.

However, there is no matching deallocator definition and when
the object is delete()'d the standard C++ operator overloading
rules make the global-scope SquidNew.h definition of
::operator delete() be the method of deallocation. That uses
free() internally.

To resolve the mismatch of new()/free() we must define a
wrapper class with explicit class-scope new/delete operators
instead of relying on weak linkages to overloaded global scope
operators.

As a result the memory is new()'d and free()'d. As detected by
Valgrind
- ---
 src/Debug.h             | 14 +++++++++++++-
 src/debug.cc            |  6 +++---
 src/tests/stub_debug.cc |  6 +++---
 3 files changed, 19 insertions(+), 7 deletions(-)
- ----

I've confirmed that applying that patch on top of the jessie
squid3 version fixes the reported crash. The patch is attached
for convenience/completeness.  


Bjørn

- -- System Information:
Debian Release: 8.1
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'stable-updates')
Architecture: amd64 (x86_64)

Kernel: Linux 3.16.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages squid3 depends on:
ii  adduser                  3.113+nmu3
ii  libc6                    2.19-18
ii  libcap2                  1:2.24-8
ii  libcomerr2               1.42.12-1.1
ii  libdb5.3                 5.3.28-9
ii  libecap2                 0.2.0-3
ii  libexpat1                2.1.0-6+deb8u1
ii  libgcc1                  1:4.9.2-10
ii  libgssapi-krb5-2         1.12.1+dfsg-19
ii  libk5crypto3             1.12.1+dfsg-19
ii  libkrb5-3                1.12.1+dfsg-19
ii  libldap-2.4-2            2.4.40+dfsg-1
ii  libltdl7                 2.4.2-1.11
ii  libnetfilter-conntrack3  1.0.4-1
ii  libnettle4               2.7.1-5
ii  libpam0g                 1.1.8-3.1
ii  libsasl2-2               2.1.26.dfsg1-13
ii  libstdc++6               4.9.2-10
ii  libxml2                  2.9.1+dfsg1-5
ii  logrotate                3.8.7-1+b1
ii  lsb-base                 4.1+Debian13+nmu1
ii  netbase                  5.3
ii  squid3-common            3.4.8-6+deb8u1

squid3 recommends no packages.

Versions of packages squid3 suggests:
pn  resolvconf   <none>
ii  smbclient    2:4.1.17+dfsg-2
ii  squid-cgi    3.4.8-6+deb8u1
pn  squid-purge  <none>
pn  squidclient  <none>
pn  ufw          <none>
pn  winbindd     <none>

- -- Configuration Files:
/etc/squid3/squid.conf changed [not included]

- -- no debconf information

-----BEGIN PGP SIGNATURE-----

iEYEARECAAYFAlXMfogACgkQ10rqkowbIsnrlQCdE3/3yTvlzQwlcgU1nAobC6hF
/64AnAj1CYwAXh1u5GWW2hwkHDKzwREb
=Cfht
-----END PGP SIGNATURE-----
>From 41b2578a9ee6b692869b9d962197cf6e3773898e Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squ...@treenet.co.nz>
Date: Fri, 19 Dec 2014 08:26:44 -0800
Subject: [PATCH] MemPool the debug output stream buffers

The CurrentDebug output stream controller for cache.log was
defined as a std::ostringstream object and allocated with
new/delete on each call to debugs().

The std::ostringstream is defined as a templates output stream
which uses the std::allocator<char> built into libc when its
new()'d. Since this is all internal to the STL library
definitions it links against the libc global-scope allocator.

However, there is no matching deallocator definition and when
the object is delete()'d the standard C++ operator overloading
rules make the global-scope SquidNew.h definition of
::operator delete() be the method of deallocation. That uses
free() internally.

To resolve the mismatch of new()/free() we must define a
wrapper class with explicit class-scope new/delete operators
instead of relying on weak linkages to overloaded global scope
operators.

As a result the memory is new()'d and free()'d. As detected by
Valgrind
---
 src/Debug.h             | 14 +++++++++++++-
 src/debug.cc            |  6 +++---
 src/tests/stub_debug.cc |  6 +++---
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/Debug.h b/src/Debug.h
index 7d0509a..41ea2fe 100644
--- a/src/Debug.h
+++ b/src/Debug.h
@@ -67,7 +67,19 @@ private:
     // Hack: replaces global ::xassert() to debug debugging assertions
     static void xassert(const char *msg, const char *file, int line);
 
-    static std::ostringstream *CurrentDebug;
+    /// Wrapper class to prevent SquidNew.h overrides getting confused
+    /// with the libc++6 std::ostringstream definitions
+    class OutStream : public std::ostringstream
+    {
+        // XXX: use MEMPROXY_CLASS() once that no longer pulls in typedefs.h and enums.h and globals.h
+    public:
+        void *operator new(size_t size) throw(std::bad_alloc) {return xmalloc(size);}
+        void operator delete(void *address) throw() {xfree(address);}
+        void *operator new[] (size_t size) throw(std::bad_alloc) ; //{return xmalloc(size);}
+        void operator delete[] (void *address) throw() ; // {xfree(address);}
+    };
+
+    static OutStream *CurrentDebug;
     static int TheDepth; // level of nested debugging calls
 };
 
diff --git a/src/debug.cc b/src/debug.cc
index 869dd37..1d63351 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -709,6 +709,8 @@ ctx_get_descr(Ctx ctx)
 
 int Debug::TheDepth = 0;
 
+Debug::OutStream *Debug::CurrentDebug(NULL);
+
 std::ostream &
 Debug::getDebugOut()
 {
@@ -719,7 +721,7 @@ Debug::getDebugOut()
         *CurrentDebug << std::endl << "reentrant debuging " << TheDepth << "-{";
     } else {
         assert(!CurrentDebug);
-        CurrentDebug = new std::ostringstream();
+        CurrentDebug = new Debug::OutStream;
         // set default formatting flags
         CurrentDebug->setf(std::ios::fixed);
         CurrentDebug->precision(2);
@@ -756,8 +758,6 @@ Debug::xassert(const char *msg, const char *file, int line)
     abort();
 }
 
-std::ostringstream (*Debug::CurrentDebug)(NULL);
-
 size_t
 BuildPrefixInit()
 {
diff --git a/src/tests/stub_debug.cc b/src/tests/stub_debug.cc
index 7412640..dea7c3c 100644
--- a/src/tests/stub_debug.cc
+++ b/src/tests/stub_debug.cc
@@ -87,6 +87,8 @@ _db_print_stderr(const char *format, va_list args)
     vfprintf(stderr, format, args);
 }
 
+Debug::OutStream *Debug::CurrentDebug(NULL);
+
 std::ostream &
 Debug::getDebugOut()
 {
@@ -97,7 +99,7 @@ Debug::getDebugOut()
         *CurrentDebug << std::endl << "reentrant debuging " << TheDepth << "-{";
     } else {
         assert(!CurrentDebug);
-        CurrentDebug = new std::ostringstream();
+        CurrentDebug = new Debug::OutStream;
         // set default formatting flags
         CurrentDebug->setf(std::ios::fixed);
         CurrentDebug->precision(2);
@@ -138,8 +140,6 @@ Debug::xassert(const char *msg, const char *file, int line)
     abort();
 }
 
-std::ostringstream *Debug::CurrentDebug (NULL);
-
 const char*
 SkipBuildPrefix(const char* path)
 {
-- 
2.1.4

Reply via email to