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: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
|