Ken Giusti created DISPATCH-1544:
------------------------------------
Summary: Coverity false positive use-after-free error
Key: DISPATCH-1544
URL: https://issues.apache.org/jira/browse/DISPATCH-1544
Project: Qpid Dispatch
Issue Type: Improvement
Affects Versions: 1.11.0
Reporter: Ken Giusti
Assignee: Ken Giusti
Fix For: 1.11.0
Coverity static analysis has started reporting use-after-free errors in the
delivery state update logic. I've reviewed these errors and I believe these
are false positives. I suspect that Coverity doesn't have enough context to
understand the delivery refcount management done by this code to prevent
use-after-free of the reported deliveries.
I've created a patch that adds assert checking of the refcounts to enforce the
ownership state expected by the code. This appears to "fix" the coverity issue
in so much as when a CMAKE_BUILD_TYPE=Debug build is submitted for analysis
(the asserts are present in Debug) the errors are resolved. Non-Debug builds
still report the error however so I'm also going update the patch to include
annotations to ignore the errors.
I'm opening this Jira to solicit additional eyes on the problem in case I'm
missing something and these errors are legit.
-----------------------------------------------------------------
4 new defect(s) introduced to Apache Qpid dispatch-router found with Coverity
Scan.
New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)
*
** CID 353022: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 729 in
qdr_delivery_anycast_update_CT()
________________________________________________________________________________________________________
*
**
*** CID 353022: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 729 in
qdr_delivery_anycast_update_CT()
723 assert(sys_atomic_get(&dlv->ref_count) > 1);
724 assert(sys_atomic_get(&peer->ref_count) > 1);
725 qdr_delivery_unlink_peers_CT(core, dlv, peer);
726 }
727
728 if (dlink)
>>> CID 353022: (USE_AFTER_FREE)
>>> Calling "qdr_delivery_settled_CT" dereferences freed pointer "dlv".
729 dlv_moved = qdr_delivery_settled_CT(core, dlv);
730 }
731
732 //
733 // If the delivery's link has a core endpoint, notify the endpoint of the
update
734 //
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 729 in
qdr_delivery_anycast_update_CT()
723 assert(sys_atomic_get(&dlv->ref_count) > 1);
724 assert(sys_atomic_get(&peer->ref_count) > 1);
725 qdr_delivery_unlink_peers_CT(core, dlv, peer);
726 }
727
728 if (dlink)
>>> CID 353022: (USE_AFTER_FREE)
>>> Passing freed pointer "dlv" as an argument to "qdr_delivery_settled_CT".
729 dlv_moved = qdr_delivery_settled_CT(core, dlv);
730 }
731
732 //
733 // If the delivery's link has a core endpoint, notify the endpoint of the
update
734 //
*
** CID 353021: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 749 in
qdr_delivery_anycast_update_CT()
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 739 in
qdr_delivery_anycast_update_CT()
________________________________________________________________________________________________________
*
**
*** CID 353021: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 749 in
qdr_delivery_anycast_update_CT()
743 //
744 if (dlv_moved)
745 qdr_delivery_decref_CT(core, dlv, "qdr_delivery_anycast_update CT - dlv
removed from unsettled");
746 if (peer_moved)
747 qdr_delivery_decref_CT(core, peer, "qdr_delivery_anycast_update_CT - peer
removed from unsettled");
748 if (peer)
>>> CID 353021: (USE_AFTER_FREE)
>>> Passing freed pointer "peer" as an argument to "qdr_delivery_decref_CT".
749 qdr_delivery_decref_CT(core, peer, "qdr_delivery_anycast_update_CT - allow
free of peer");
750
751 return error_assigned;
752 }
753
754
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 739 in
qdr_delivery_anycast_update_CT()
733 // If the delivery's link has a core endpoint, notify the endpoint of the
update
734 //
735 if (dlink && dlink->core_endpoint)
736 qdrc_endpoint_do_update_CT(core, dlink->core_endpoint, dlv, settled);
737
738 if (push || peer_moved)
>>> CID 353021: (USE_AFTER_FREE)
>>> Passing freed pointer "peer" as an argument to "qdr_delivery_push_CT".
739 qdr_delivery_push_CT(core, peer);
740
741 //
742 // Release the unsettled references if the deliveries were moved/settled
743 //
744 if (dlv_moved)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 749 in
qdr_delivery_anycast_update_CT()
743 //
744 if (dlv_moved)
745 qdr_delivery_decref_CT(core, dlv, "qdr_delivery_anycast_update CT - dlv
removed from unsettled");
746 if (peer_moved)
747 qdr_delivery_decref_CT(core, peer, "qdr_delivery_anycast_update_CT - peer
removed from unsettled");
748 if (peer)
>>> CID 353021: (USE_AFTER_FREE)
>>> Calling "qdr_delivery_decref_CT" dereferences freed pointer "peer".
749 qdr_delivery_decref_CT(core, peer, "qdr_delivery_anycast_update_CT - allow
free of peer");
750
751 return error_assigned;
752 }
753
754
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 739 in
qdr_delivery_anycast_update_CT()
733 // If the delivery's link has a core endpoint, notify the endpoint of the
update
734 //
735 if (dlink && dlink->core_endpoint)
736 qdrc_endpoint_do_update_CT(core, dlink->core_endpoint, dlv, settled);
737
738 if (push || peer_moved)
>>> CID 353021: (USE_AFTER_FREE)
>>> Calling "qdr_delivery_push_CT" dereferences freed pointer "peer".
739 qdr_delivery_push_CT(core, peer);
740
741 //
742 // Release the unsettled references if the deliveries were moved/settled
743 //
744 if (dlv_moved)
*
** CID 353020: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 1005 in
qdr_delivery_mcast_outbound_update_CT()
________________________________________________________________________________________________________
*
**
*** CID 353020: (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 1005 in
qdr_delivery_mcast_outbound_update_CT()
999 // Note: qdr_delivery_mcast_outbound_settled_CT may free out_dlv!
1000 if (settled && qdr_delivery_mcast_outbound_settled_CT(core, in_dlv,
out_dlv, &dlv_moved)) \{ 1001 push_dlv = true; 1002 }
1003
1004 if (push_dlv || dlv_moved) \{ >>> CID 353020: (USE_AFTER_FREE) >>>
Passing freed pointer "in_dlv" as an argument to "qdr_delivery_push_CT". 1005
qdr_delivery_push_CT(core, in_dlv); 1006 }
1007 if (dlv_moved) \{ 1008 qdr_delivery_decref_CT(core, in_dlv,
"qdr_delivery_mcast_outbound_update_CT - removed mcast peer from unsettled");
1009 }
1010 qdr_delivery_decref_CT(core, in_dlv,
"qdr_delivery_mcast_outbound_update_CT - allow mcast peer free");
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 1005 in
qdr_delivery_mcast_outbound_update_CT()
999 // Note: qdr_delivery_mcast_outbound_settled_CT may free out_dlv!
1000 if (settled && qdr_delivery_mcast_outbound_settled_CT(core, in_dlv,
out_dlv, &dlv_moved)) \{1001 push_dlv = true;1002 }
1003
1004 if (push_dlv || dlv_moved)
{ >>> CID 353020: (USE_AFTER_FREE) >>> Calling "qdr_delivery_push_CT"
dereferences freed pointer "in_dlv". 1005 qdr_delivery_push_CT(core, in_dlv);
1006 }
1007 if (dlv_moved)
{ 1008 qdr_delivery_decref_CT(core, in_dlv,
"qdr_delivery_mcast_outbound_update_CT - removed mcast peer from unsettled");
1009 }
1010 qdr_delivery_decref_CT(core, in_dlv,
"qdr_delivery_mcast_outbound_update_CT - allow mcast peer free");
*
** CID 353019: Memory - illegal accesses (USE_AFTER_FREE)
________________________________________________________________________________________________________
*
**
*** CID 353019: Memory - illegal accesses (USE_AFTER_FREE)
/home/kgiusti/work/dispatch/qpid-dispatch/src/router_core/delivery.c: 826 in
qdr_delivery_mcast_inbound_update_CT()
820 if (unlink) \{ 821 // expect: in_dlv should not be freed here as caller
must hold reference: 822 assert(sys_atomic_get(&in_dlv->ref_count) > 1); 823
qdr_delivery_unlink_peers_CT(core, in_dlv, out_peer); // may free out_peer! 824
}
825
>>> CID 353019: Memory - illegal accesses (USE_AFTER_FREE)
>>> Calling "qdr_delivery_next_peer_CT" dereferences freed pointer "in_dlv".
826 out_peer = qdr_delivery_next_peer_CT(in_dlv);
827 }
828
829 if (settled) {
830 assert(qdr_delivery_peer_count_CT(in_dlv) == 0);
831 in_dlv->settled = true;
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
[https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZSbhom32dlDl11LWEm9nX1-2FDm2ydKRp2jKIMEChnF9qYjWDV40qhnoFf9KqJJs5gJkRt3r-2Bll2jeD6T5JeFcgC_GXVbCiBVihoVsavRYoBomC138NwPo21bY7ULJx-2Blbc6-2Bm7jghPWf9shMv58rQnTBWmoqpA4yGkw7Irka8-2F1mapS-2BlqITfMO7qBYycZX0CRsDtVVUgPaJZVLqhO3lDGP1Z3a7gDo-2F83Lwh4ic672c8spVZ1Z02NbeFL0d1BNBO8Nz7mQcucmnfcHUmlPy5YeUCE-2FKg3bEl0e3-2FIlMhTxoxCArT8Ei5dLOZ2dyqMlCZjo-3D]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
