Azure / azure-uamqp-c

AMQP library for C
Other
60 stars 63 forks source link

Avoid double-free if session_send_transfer fails and internally destroys the async operation (in link_transfer_async) #468

Closed ewertons closed 2 months ago

ewertons commented 2 months ago

If session_send_transfer fails internally, depending on the point where it fails remove_all_pending_deliveries might be called, causing the ASYNC_OPERATION_HANDLE/DELIVERY_INSTANCE variable result to be removed from link->pending_deliveries and destroyed. Then when session_send_transfer returns, in such cases singlylinkedlist_remove will fail and async_operation_destroy should not be called (to avoid a double-free).

Callstack of the current double-free in such scenario (without this fix):

==4910== Invalid free() / delete / delete[] / realloc() ==4910== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==4910== by 0x15E0FD: async_operation_destroy (async_operation.c:63) ==4910== by 0x169B32: link_transfer_async (link.c:1509) ==4910== by 0x16FAE0: send_one_message (message_sender.c:566) ==4910== by 0x170810: messagesender_send_async (message_sender.c:962) ==4910== by 0x122E0C: send_batched_message_and_reset_state (iothubtransport_amqp_telemetry_messenger.c:1120) ==4910== by 0x1234B9: send_pending_events (iothubtransport_amqp_telemetry_messenger.c:1258) ==4910== by 0x124A05: telemetry_messenger_do_work (iothubtransport_amqp_telemetry_messenger.c:1809) ==4910== by 0x11BF83: amqp_device_do_work (iothubtransport_amqp_device.c:1137) ==4910== by 0x115666: IoTHubTransport_AMQP_Common_Device_DoWork (iothubtransport_amqp_common.c:1121) ==4910== by 0x1164A9: IoTHubTransport_AMQP_Common_DoWork (iothubtransport_amqp_common.c:1485) ==4910== by 0x112FD3: IoTHubTransportAMQP_DoWork (iothubtransportamqp.c:56) ==4910== Address 0x852ae30 is 0 bytes inside a block of size 56 free'd ==4910== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==4910== by 0x15E0FD: async_operation_destroy (async_operation.c:63) ==4910== by 0x1661D3: remove_all_pending_deliveries (link.c:105) ==4910== by 0x167B5B: on_session_state_changed (link.c:674) ==4910== by 0x1746D4: session_set_state (session.c:143) ==4910== by 0x174C73: on_connection_state_changed (session.c:386) ==4910== by 0x15FB7B: connection_set_state (connection.c:114) ==4910== by 0x160222: on_bytes_encoded (connection.c:276) ==4910== by 0x166093: frame_codec_encode_frame (frame_codec.c:706) ==4910== by 0x1777BF: amqp_frame_codec_encode_frame (amqp_frame_codec.c:314) ==4910== by 0x1648D8: connection_encode_frame (connection.c:2084) ==4910== by 0x176CD4: session_send_transfer (session.c:1655)