pthread_mutex_unlock doesn't check whether arguments is NULL for windows, this may cause dos

[474172261]

description

according to https://github.com/coapp-packages/pthreads/blob/master/pthread_mutex_unlock.c, it doesn't check whether the *mutex is NULL, and access `mutex directly. This will cause crash because of invalid memory access.

The use of pthread_mutex_unlock in this project doesn't check it. I compiled it in windows, and get a crash in d2c_messaging.c:DefaultIoTHubSendReportedStateCompletedCallback while message_processing_context->mutex is NULL.

[jw-msft]

What real runtime scenario (without modifying code) are you seeing the message_processing_context->mutex is NULL?

AFAICT, it should be impossible for message_processing_context->mutex to be NULL because it would have caused the call to ADUC_D2C_Messaging_Init() to fail and the AducIotAgent process would then exit here: https://github.com/Azure/iot-hub-device-update/blob/b0dee1b96b313020670f1a0c4d614dcc1d7e4a7d/src/agent/src/main.c#L702C1-L706C1

See the code in ADUC_D2C_Messaging_Init() here that causes it to return an error if the res returned from pthread_mutex_init(&s_messageProcessingContext[i].mutex, NULL) fails.

https://github.com/Azure/iot-hub-device-update/blob/b0dee1b96b313020670f1a0c4d614dcc1d7e4a7d/src/utils/d2c_messaging/src/d2c_messaging.c#L433C1-L438C14

I suppose one could check if the mutex is NULL before trying to lock, but what do you propose it should do if it is NULL? continuing without locking is not an option.

The best I can think of is to exit the process because the invariant condition is that the mutex should have been initialized during the ADUC_D2C_Messaging_Init(), so a crash is a good thing to make it discoverable with a stack trace in a crash dump--it would mean the initialization was not called, which is bad, or something else set the mutex to NULL (also bad).

[474172261]

    pthreadVC3d.dll!00007ffb382a58d0()  未知
    AducIotAgent.exe!DefaultIoTHubSendReportedStateCompletedCallback(int http_status_code=0x00000000, void * context=0x00007ff610013dd0) 行 316  C
    AducIotAgent.exe!IoTHubClientCore_LL_ReportedStateComplete(unsigned int item_id=0x00000002, int status_code=0x00000000, void * ctx=0x000002c6c0c0be20) 行 568    C
    AducIotAgent.exe!on_device_send_twin_update_complete_callback(DEVICE_TWIN_UPDATE_RESULT_TAG result=DEVICE_TWIN_UPDATE_RESULT_ERROR, int status_code=0x00000000, void * context=0x000002c6c3a1df90) 行 530    C
    AducIotAgent.exe!on_report_state_complete_callback(TWIN_REPORT_STATE_RESULT_TAG result=TWIN_REPORT_STATE_RESULT_CANCELLED, TWIN_REPORT_STATE_REASON_TAG reason=TWIN_REPORT_STATE_REASON_MESSENGER_DESTROYED, int status_code=0x00000000, const void * context=0x000002c6c3a1ffe0) 行 283 C
    AducIotAgent.exe!cancel_all_pending_twin_operations(const void * item=0x000002c6c3a3dfc0, const void * match_context=0x000002c6c0c4ff70, bool * continue_processing=0x000000f7cff3f274) 行 1258  C
    AducIotAgent.exe!singlylinkedlist_remove_if(SINGLYLINKEDLIST_INSTANCE_TAG * list=0x000002c6c0c57fe0, bool(*)(const void *, const void *, bool *) condition_function=0x00007ff60fe354a0, const void * match_context=0x000002c6c0c4ff70) 行 296    C
    AducIotAgent.exe!internal_twin_messenger_destroy(TWIN_MESSENGER_INSTANCE_TAG * twin_msgr=0x000002c6c0c4ff70) 行 1317 C
    AducIotAgent.exe!twin_messenger_destroy(TWIN_MESSENGER_INSTANCE * twin_msgr_handle=0x000002c6c0c4ff70) 行 2057   C
    AducIotAgent.exe!internal_destroy_device(DEVICE_INSTANCE_TAG * instance=0x000002c6c0c39f40) 行 552   C
    AducIotAgent.exe!amqp_device_destroy(AMQP_DEVICE_INSTANCE * handle=0x000002c6c0c39f40) 行 1163   C
    AducIotAgent.exe!internal_destroy_amqp_device_instance(AMQP_TRANSPORT_DEVICE_INSTANCE_TAG * trdev_inst=0x000002c6c0c33f50) 行 235    C
    AducIotAgent.exe!IoTHubTransport_AMQP_Common_Unregister(void * deviceHandle=0x000002c6c0c33f50) 行 2201  C
    AducIotAgent.exe!IoTHubTransportAMQP_Unregister(void * deviceHandle=0x000002c6c0c33f50) 行 117   C
    AducIotAgent.exe!IoTHubClientCore_LL_Destroy(IOTHUB_CLIENT_CORE_LL_HANDLE_DATA_TAG * iotHubClientHandle=0x000002c6c0c0be20) 行 1751  C
    AducIotAgent.exe!IoTHubDeviceClient_LL_Destroy(IOTHUB_CLIENT_CORE_LL_HANDLE_DATA_TAG * iotHubClientHandle=0x000002c6c0c0be20) 行 52  C
    AducIotAgent.exe!ClientHandle_Destroy(void * iotHubClientHandle=0x000002c6c0c0be20) 行 391   C
    AducIotAgent.exe!IoTHub_CommunicationManager_Deinit(...) 行 170  C
>   AducIotAgent.exe!ShutdownAgent(...) 行 822   C
    AducIotAgent.exe!main(int argc=0x00000001, char * * argv=0x000002c6bc801fa0) 行 1108 C
    [外部代码]  

check

void ShutdownAgent()
{
    Log_Warn("Agent is shutting down.");
    ADUC_D2C_Messaging_Uninit();
#ifdef ADUC_COMMAND_HELPER_H
    UninitializeCommandListenerThread();
#endif
    ADUC_PnP_Components_Destroy();
    IoTHub_CommunicationManager_Deinit();
    DiagnosticsComponent_DestroyDeviceName();
    ADUC_Logging_Uninit();
    ExtensionManager_Uninit();
}

as you can see, ADUC_D2C_Messaging_Uninit was called before the DefaultIoTHubSendReportedStateCompletedCallback under IoTHub_CommunicationManager_Deinit. so it can be NULL

[jw-msft]

Thanks. It makes sense that this could happen during process shutdown given the order of uninit.