Bug 1450495

Description of problem:
When qpidd user can't access/read /etc/pki/katello/nssdb/nss_db_password-file file, an attempt to (re)start qpidd broker results in segfault.

Usually, this situation shall not happen since the file is managed by puppet in installer, but it happened at a customer during 6.1->6.2 upgrade.

Backtrace and initial analysis: see additional info.

Reproducer is very trivial, (monkey) codefix should be simple (bug shall be in upstream as well - investigation ongoing).


Version-Release number of selected component (if applicable):
qpid-cpp-server 0.30-* and also 0.34-24


How reproducible:
100%


Steps to Reproduce:
1. chown root:root /etc/pki/katello/nssdb/nss_db_password-file 
2. service qpidd restart
3. check /var/log/messages  / service qpidd status


Actual results:
qpidd segfaults.


Expected results:
No segfault but error log - a similar one when other nssdb files are not accessible


Additional info:
Curiously, when making inaccessible other nssdb files, broker can run filing some error and not listening on 5671. Just the password file causes this problem.

Backtrace:

(gdb) bt
#0  0x00007f3010a4f704 in qpid::sys::SocketAddress::nextAddress (this=this@entry=0x7ffe36dc0570) at /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/posix/SocketAddress.cpp:321
#1  0x00007f301113ec17 in qpid::sys::SocketAcceptor::listen (this=this@entry=0x29bf500, interfaces=..., port=port@entry=5671, backlog=backlog@entry=10, factory=...)
    at /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/SocketTransport.cpp:150
#2  0x00007f3010fdfdbb in qpid::sys::SslPlugin::initialize (this=0x7f3011407180 <qpid::sys::sslPlugin>, target=...) at /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/SslPlugin.cpp:126
#3  0x00007f3010a876af in operator() (a1=..., p=<optimized out>, this=<synthetic pointer>) at /usr/include/boost/bind/mem_fn_template.hpp:165
#4  operator()<boost::_mfi::mf1<void, qpid::Plugin, qpid::Plugin::Target&>, boost::_bi::list1<qpid::Plugin* const&> > (a=<synthetic pointer>, f=<synthetic pointer>, 
    this=<synthetic pointer>) at /usr/include/boost/bind/bind.hpp:313
#5  operator()<qpid::Plugin*> (a1=@0x2488ce0: 0x7f3011407180 <qpid::sys::sslPlugin>, this=<synthetic pointer>) at /usr/include/boost/bind/bind_template.hpp:47
#6  for_each<__gnu_cxx::__normal_iterator<qpid::Plugin* const*, std::vector<qpid::Plugin*> >, boost::_bi::bind_t<void, boost::_mfi::mf1<void, qpid::Plugin, qpid::Plugin::Target&>, boost::_bi::list2<boost::arg<1>, boost::reference_wrapper<qpid::Plugin::Target> > > > (__f=..., __last=..., __first=<qpid::sys::sslPlugin>) at /usr/include/c++/4.8.2/bits/stl_algo.h:4417
#7  qpid::(anonymous namespace)::each_plugin<boost::_bi::bind_t<void, boost::_mfi::mf1<void, qpid::Plugin, qpid::Plugin::Target&>, boost::_bi::list2<boost::arg<1>, boost::reference_wrapper<qpid::Plugin::Target> > > > (f=...) at /usr/src/debug/qpid-cpp-0.34/src/qpid/Plugin.cpp:73
#8  0x00007f3010a877a2 in qpid::Plugin::initializeAll (t=...) at /usr/src/debug/qpid-cpp-0.34/src/qpid/Plugin.cpp:91
#9  0x00007f3010ffc99a in qpid::broker::Broker::Broker (this=0x249bae0, conf=...) at /usr/src/debug/qpid-cpp-0.34/src/qpid/broker/Broker.cpp:376
#10 0x0000000000405c82 in qpid::broker::QpiddBroker::execute (this=this@entry=0x7ffe36dc284e, options=0x24909a0) at /usr/src/debug/qpid-cpp-0.34/src/posix/QpiddBroker.cpp:229
#11 0x0000000000409d04 in qpid::broker::run_broker (argc=3, argv=0x7ffe36dc2be8, hidden=<optimized out>) at /usr/src/debug/qpid-cpp-0.34/src/qpidd.cpp:108
#12 0x00007f300fb0db35 in __libc_start_main (main=0x404ce0 <main(int, char**)>, argc=3, ubp_av=0x7ffe36dc2be8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7ffe36dc2bd8) at ../csu/libc-start.c:274
#13 0x0000000000404f51 in _start ()
(gdb) list
316	        (void) getAddrInfo(*this);
317	    }
318	}
319	
320	bool SocketAddress::nextAddress() const {
321	    bool r = currentAddrInfo->ai_next != 0;
322	    if (r)
323	        currentAddrInfo = currentAddrInfo->ai_next;
324	    return r;
325	}
(gdb) p currentAddrInfo
$2 = (addrinfo *) 0x0
(gdb) 


Cause: In /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/SocketTransport.cpp, method SocketAcceptor::listen :

    for (unsigned i = 0; i<addresses.size(); ++i) {
        QPID_LOG(debug, "Using interface: " << addresses[i]);
        SocketAddress sa(addresses[i], sport);

        do {
        try {
            ..
            std::auto_ptr<Socket> s(factory());
            uint16_t lport = s->listen(sa, backlog);
            ..
        } catch (std::exception& e) {
            QPID_LOG(warning, "Couldn't listen to: " << sa.asString() << ": " << e.what());
        }
        } while (sa.nextAddress());
    }

The "SocketAddress sa(addresses[i], sport);" sets currentAddrInfo to null. If an exception is raised during "s(factory());" call (when setting up SSL stuff), "s->listen(sa, backlog)" isnt called and sa.getAddrInfo does not initiate currentAddrInfo. Iterating the above cycle, "sa.nextAddress()" fails.

So a patch might be:

--- /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/posix/SocketAddress.cpp	2014-10-02 15:55:36.000000000 +0200
+++ /usr/src/debug/qpid-cpp-0.34/src/qpid/sys/posix/SocketAddress.cpp.new	2017-05-12 20:50:12.926907398 +0200
@@ -318,7 +318,7 @@ void SocketAddress::firstAddress() const
 }
 
 bool SocketAddress::nextAddress() const {
-    bool r = currentAddrInfo->ai_next != 0;
+    bool r = (currentAddrInfo) && (currentAddrInfo->ai_next != 0);
     if (r)
         currentAddrInfo = currentAddrInfo->ai_next;
     return r;

that will 1) prevent nullpointer dereference, and 2) exit the do {} while cycle above.