qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ken Giusti (Jira)" <j...@apache.org>
Subject [jira] [Created] (DISPATCH-1544) Coverity false positive use-after-free error
Date Wed, 08 Jan 2020 15:17:00 GMT
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


Mime
View raw message