Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 5, 2025

The ServerSendIp function in quicipserver.cpp was leaking memory in two error paths where allocated buffers were not being freed before returning.

Issue:
The function allocates memory using CXPLAT_ALLOC_PAGED but failed to free it when either StreamOpen or StreamStart operations failed:

auto SendBufferRaw = CXPLAT_ALLOC_PAGED(sizeof(QUIC_BUFFER) + sizeof(QUIC_ADDR), QUIC_POOL_TOOL);

// Error path 1: StreamOpen failure - no cleanup
if (QUIC_FAILED(Status = MsQuic->StreamOpen(...))) {
    printf("StreamOpen failed, 0x%x!\n", Status);
    return;  // Memory leak!
}

// Error path 2: StreamStart failure - no cleanup  
if (QUIC_FAILED(Status = MsQuic->StreamStart(...))) {
    printf("StreamStart failed, 0x%x!\n", Status);
    MsQuic->StreamClose(Stream);
    return;  // Memory leak!
}

Solution:
Added CXPLAT_FREE(SendBufferRaw, QUIC_POOL_TOOL); calls before returning in both error paths to ensure proper memory cleanup. This matches the pattern already used in other error paths within the same function and ensures every allocation has a corresponding deallocation.

The fix is minimal and surgical - only two lines added to prevent memory leaks without affecting any existing functionality.

Fixes #5255.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Potential Memory Leak Due to Missing CXPLAT_FREE() in Error Paths Fix memory leak in ServerSendIp error paths by adding missing CXPLAT_FREE calls Aug 5, 2025
@Copilot Copilot AI requested a review from guhetier August 5, 2025 00:24
Copilot finished work on behalf of guhetier August 5, 2025 00:24
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.37%. Comparing base (4aeadf8) to head (6e1021a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5324      +/-   ##
==========================================
- Coverage   86.58%   86.37%   -0.22%     
==========================================
  Files          59       59              
  Lines       18331    18331              
==========================================
- Hits        15872    15833      -39     
- Misses       2459     2498      +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential Memory Leak Due to Missing CXPLAT_FREE() in Error Paths
2 participants