Bug 1450495 - qpidd segfault during startup when nss_db_password-file inaccessible
Summary: qpidd segfault during startup when nss_db_password-file inaccessible
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Qpid
Version: 6.2.10
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: Unspecified
Assignee: Pavel Moravec
QA Contact: Katello QA List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-05-12 18:52 UTC by Pavel Moravec
Modified: 2020-12-14 08:39 UTC (History)
5 users (show)

Fixed In Version: qpid-cpp-0.34-25
Doc Type: Bug Fix
Doc Text:
During an upgrade, the `qpidd` user could not access or read the `/etc/pki/katello/nssdb/nss_db_password-file` file. The qpidd broker attempted to restart, which caused a segmentation fault.
Clone Of:
Environment:
Last Closed: 2018-02-21 12:57:54 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Apache JIRA QPID-7786 0 None None None 2017-05-18 10:20:15 UTC
Red Hat Product Errata RHBA-2018:0338 0 normal SHIPPED_LIVE Red Hat Satellite 6.3 Tools 2018-02-21 19:30:02 UTC

Description Pavel Moravec 2017-05-12 18:52:30 UTC
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.

Comment 2 Pavel Moravec 2017-05-15 06:44:27 UTC
Very trivial reproducer: use wrong SSL certificate name (change ssl-cert-name=.. in /etc/qpid/qpidd.conf) and restart qpidd service.

Comment 3 Pavel Moravec 2017-05-16 12:39:47 UTC
Hi Alan,
could you pls. review the patch in #c0 ?

Expected behaviour should be broker stop during the startup (but without the segfault), maybe it should log better error than now?

Comment 4 Alan Conway 2017-05-16 18:22:11 UTC
(In reply to Pavel Moravec from comment #3)
> Hi Alan,
> could you pls. review the patch in #c0 ?
> 
> Expected behaviour should be broker stop during the startup (but without the
> segfault), maybe it should log better error than now?

Fix looks good. I don't think you can improve the error message at this level, you'd need to go inside the factory() call to provide a more informative exception message, if the one that it's providing now isn't helpful.

Comment 8 errata-xmlrpc 2018-02-21 12:57:54 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:0338


Note You need to log in before you can comment on or make changes to this bug.