CESNET / libnetconf

C NETCONF library
Other
113 stars 83 forks source link

segmentation fault in ncntf_dispatch_send #232

Closed mrmouro closed 7 years ago

mrmouro commented 7 years ago

Hi, we have several Netconf sessions subscribed to notifications. Whenever we disconnect all sessions in bulk (for instance using mobaxterm multi-execution mode), the application crashes with a segmentation fault.

I assume this is related to a race condition : a session that was deleted is receiving a notification.

This is very similar to what was reported in #201, and it was fixed in the meantime. But now we've enabled NACM and we have the same erroneous behavior but with a different root cause.

Follows the backtrace:

1. /usr/local/lib/libnetconf.so.0(+0x359fc) [0xf86f9fc]
 2. /usr/local/lib/libnetconf.so.0(ncntf_dispatch_send+0x850) [0xf86626c]
 3. /usr/local/lib/libnetc.so(NetconfSessionNotification::run()+0x60) [0xfdc7560]
 4. /usr/local/lib/libcli.so(std::thread::_Impl<std::_Bind_result<void, std::_Bind<std::_Mem_fn<void (ThreadBase::*)()> (ThreadBase*)> ()> >::_M_run()+0x68) [0xfe8e618]
 5. /usr/lib/libstdc++.so.6(+0xf9686dc) [0x4837c6dc]
 6. /lib/libpthread.so.0(+0xfe0784c) [0x4821384c]
 7. /lib/libc.so.6(clone+0x84) [0x485f96f4]

After going through our application logs, I've noticed the following entry: nacm_rpc_struct: invalid session to use, so i did a quick lookup at the nacm_check_notification and added the following check:

Index: nacm.c
===================================================================
--- nacm.c      (revision 763)
+++ nacm.c      (working copy)
@@ -1353,7 +1353,8 @@
        int retval;
        NCNTF_EVENT event;

-       if (ntf == NULL || session == NULL) {
+       if (ntf == NULL || session == NULL ||
+        (session->status != NC_SESSION_STATUS_WORKING && session->status != NC_SESSION_STATUS_DUMMY)) {
                /* invalid input parameter */
                return (-1);
        }

This seems to correct the problem.

Thanks and best regards, Joao

rkrejci commented 7 years ago

Thanks Joao! Next time you can provide the fix as pull request ;)