Uploaded image for project: 'Qpid Proton'
  1. Qpid Proton
  2. PROTON-1832

[c] duplicate link names cause invalid read in pn_transport_unbind_handles

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: proton-c-0.22.0
    • Fix Version/s: proton-c-0.23.0
    • Component/s: proton-c
    • Labels:
      None

      Description

      Proton has an invalid read error in pn_transport_unbind_handles under specific circumstances where the same name is used to create 2 different incoming links on the same session.

      This patch demonstrates

      1 file changed, 43 insertions(+), 2 deletions(-)
      c/tests/connection_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
      
      modified   c/tests/connection_driver.c
      @@ -48,6 +48,24 @@ static pn_event_type_t open_handler(test_handler_t *th, pn_event_t *e) {
         return PN_EVENT_NONE;
       }
       
      +/* Like open_handler but also reply to REMOTE_CLOSE */
      +static pn_event_type_t open_close_handler(test_handler_t *th, pn_event_t *e) {
      +  switch (pn_event_type(e)) {
      +   case PN_CONNECTION_REMOTE_CLOSE:
      +    pn_connection_open(pn_event_connection(e));
      +    break;
      +   case PN_SESSION_REMOTE_CLOSE:
      +    pn_session_open(pn_event_session(e));
      +    break;
      +   case PN_LINK_REMOTE_CLOSE:
      +    pn_link_close(pn_event_link(e));
      +    break;
      +   default:
      +    return open_handler(th, e);
      +  }
      +  return PN_EVENT_NONE;
      +}
      +
       /* Handler that returns control on PN_DELIVERY and stores the delivery on the handler */
       static pn_event_type_t delivery_handler(test_handler_t *th, pn_event_t *e) {
         switch (pn_event_type(e)) {
      @@ -442,6 +458,30 @@ static void test_session_flow_control(test_t *t) {
         test_connection_drivers_destroy(&client, &server);
       }
       
      +/* Regression test for crash caused by using the same name for multiple links on same session. */
      +static void test_duplicate_link(test_t *t) {
      +  /* Set up the initial link */
      +  test_connection_driver_t client, server;
      +  test_connection_drivers_init(t, &client, open_close_handler, &server, open_close_handler);
      +  pn_connection_open(client.driver.connection);
      +  pn_session_t *ssn = pn_session(client.driver.connection);
      +  pn_session_open(ssn);
      +  pn_link_t *x = pn_sender(ssn, "x");
      +  pn_link_open(x);
      +  /* Close the link and open a new link with same name.
      +     NOTE: close and duplicate open must be in the same batch of events
      +  */
      +  pn_link_close(x);
      +  pn_link_open(pn_sender(ssn, "x"));
      +  test_connection_drivers_run(&client, &server);
      +  /* At this point the server has 2 handle map entries for the same link.
      +     Freeing the link is correct, refcounting will keep it in memory,
      +     but the extra handle map entry will cause a core dump in pn_transport_unbind_handles
      +  */
      +  pn_link_free(server.handler.link);
      +  test_connection_drivers_destroy(&client, &server);
      +}
      +
       int main(int argc, char **argv) {
         int failed = 0;
         RUN_ARGV_TEST(failed, t, test_message_transfer(&t));
      @@ -449,5 +489,6 @@ int main(int argc, char **argv) {
         RUN_ARGV_TEST(failed, t, test_message_abort(&t));
         RUN_ARGV_TEST(failed, t, test_message_abort_mixed(&t));
         RUN_ARGV_TEST(failed, t, test_session_flow_control(&t));
      +  RUN_ARGV_TEST(failed, t, test_duplicate_link(&t));
         return failed;
       }
      
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                aconway Alan Conway
                Reporter:
                aconway Alan Conway
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: