Azure / azure-c-shared-utility

Azure C SDKs common code
Other
111 stars 203 forks source link

fix mem leak in tlsio_schannel.c #540

Closed yunhaoling closed 3 years ago

yunhaoling commented 3 years ago

hey team, proposing a PR to fix some memory leak issues in the tlsio_schannel.c, not sure if it's the best fix, but after the fix, there's no memory leak in my test.


https://github.com/Azure/azure-c-shared-utility/blob/master/adapters/tlsio_schannel.c#L341-L347

    status = AcquireCredentialsHandle(NULL, UNISP_NAME, SECPKG_CRED_OUTBOUND, NULL,
        &auth_data, NULL, NULL, &tls_io_instance->credential_handle, NULL);
    if (status != SEC_E_OK)
    {
        tls_io_instance->tlsio_state = TLSIO_STATE_ERROR;
        indicate_error(tls_io_instance);
    }

https://github.com/Azure/azure-c-shared-utility/blob/master/adapters/tlsio_schannel.c#L363-L367

        flags = ISC_REQ_EXTENDED_ERROR | ISC_REQ_STREAM | ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_USE_SUPPLIED_CREDS;
        status = InitializeSecurityContext(&tls_io_instance->credential_handle,
            NULL, tls_io_instance->host_name, ISC_REQ_EXTENDED_ERROR | ISC_REQ_STREAM | ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_USE_SUPPLIED_CREDS, 0, 0, NULL, 0,
            &tls_io_instance->security_context, &security_buffers_desc,
            &context_attributes, NULL);

https://github.com/Azure/azure-c-shared-utility/blob/master/adapters/tlsio_schannel.c#L656-L660

                flags = ISC_REQ_EXTENDED_ERROR | ISC_REQ_STREAM | ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_USE_SUPPLIED_CREDS;
                status = InitializeSecurityContext(&tls_io_instance->credential_handle,
                    &tls_io_instance->security_context, tls_io_instance->host_name, flags, 0, 0, &input_buffers_desc, 0,
                    &tls_io_instance->security_context, &output_buffers_desc,
                    &context_attributes, NULL);

https://github.com/Azure/azure-c-shared-utility/blob/master/adapters/tlsio_schannel.c#L938-L942

                    flags = ISC_REQ_EXTENDED_ERROR | ISC_REQ_STREAM | ISC_REQ_ALLOCATE_MEMORY | ISC_REQ_USE_SUPPLIED_CREDS;
                    status = InitializeSecurityContext(&tls_io_instance->credential_handle,
                        &tls_io_instance->security_context, tls_io_instance->host_name, flags, 0, 0, &input_buffers_desc, 0,
                        &tls_io_instance->security_context, &output_buffers_desc,
                        &context_attributes, NULL);
yunhaoling commented 3 years ago

hey @ewertons @ericwol-msft, could you help review my PR?

yunhaoling commented 3 years ago

hey @jspaith @ewertons , do you have time to take a look at the PR?

danewalton commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).