Skip to content

QuicOperationFree uses CXPLAT_FREE for pool-allocated QUIC_RECV_CHUNK in STRM_PROVIDE_RECV_BUFFERS cleanup #5841

@dunalduck0

Description

@dunalduck0

Describe the bug

In src/core/operation.c:122-130, QuicOperationFree cleans up leftover QUIC_RECV_CHUNK entries from a STRM_PROVIDE_RECV_BUFFERS operation using CXPLAT_FREE:

} else if (ApiCtx->Type == QUIC_API_TYPE_STRM_PROVIDE_RECV_BUFFERS) {
    while (!CxPlatListIsEmpty(&ApiCtx->STRM_PROVIDE_RECV_BUFFERS.Chunks)) {
        CXPLAT_FREE(
            CXPLAT_CONTAINING_RECORD(
                CxPlatListRemoveHead(&ApiCtx->STRM_PROVIDE_RECV_BUFFERS.Chunks),
                QUIC_RECV_CHUNK,
                Link),
            QUIC_POOL_RECVBUF);
    }

These chunks are allocated via CxPlatPoolAlloc in MsQuicStreamProvideReceiveBuffers (api.c:1514) and initialized with AllocatedFromPool = TRUE (api.c:1524). CxPlatPoolAlloc returns an interior pointer (malloc_ptr + sizeof(CXPLAT_POOL_HEADER)), so calling CXPLAT_FREEfree() on that pointer is invalid — it frees an address that was not returned by malloc.

The correct free function is QuicRecvChunkFree() (recv_buffer.c:181-195), which checks Chunk->AllocatedFromPool and calls CxPlatPoolFree for pool-allocated chunks or CXPLAT_FREE for directly-allocated ones.

Affected OS

  • Windows
  • Linux
  • macOS
  • Other (specify below)

Additional OS information

No response

MsQuic version

main

Steps taken to reproduce bug

Test code to verify the bug

//
// Regression test for a bug in QuicOperationFree (operation.c:122-130)
// where CXPLAT_FREE is used on pool-allocated QUIC_RECV_CHUNK entries in the
// STRM_PROVIDE_RECV_BUFFERS operation cleanup. These chunks are allocated via
// CxPlatPoolAlloc (which returns an interior pointer), so calling CXPLAT_FREE
// (free()) on them is invalid. The correct function is QuicRecvChunkFree().
//
// Trigger: provide buffers whose total length exceeds UINT32_MAX so that
// QuicRecvBufferProvideChunks returns QUIC_STATUS_INVALID_PARAMETER without
// consuming the chunks. The chunks remain in the operation and are cleaned up
// by QuicOperationFree after QuicConnFatalError tears down the connection.
//
void
QuicTestOperationFreeChunkCleanup(
    )
{
    MsQuicRegistration Registration(true);
    TEST_QUIC_SUCCEEDED(Registration.GetInitStatus());

    MsQuicConfiguration ServerConfiguration(
        Registration, "MsQuicTest",
        MsQuicSettings().SetPeerBidiStreamCount(1),
        ServerSelfSignedCredConfig);
    TEST_QUIC_SUCCEEDED(ServerConfiguration.GetInitStatus());

    MsQuicConfiguration ClientConfiguration(
        Registration, "MsQuicTest", MsQuicCredentialConfig());
    TEST_QUIC_SUCCEEDED(ClientConfiguration.GetInitStatus());

    CxPlatWatchdog Watchdog(5000);

    NthAllocFailTestContext RecvContext {};
    MsQuicAutoAcceptListener Listener(
        Registration, ServerConfiguration,
        NthAllocFailTestContext::ConnCallback, &RecvContext);
    TEST_QUIC_SUCCEEDED(Listener.GetInitStatus());
    TEST_QUIC_SUCCEEDED(Listener.Start("MsQuicTest"));
    QuicAddr ServerLocalAddr;
    TEST_QUIC_SUCCEEDED(Listener.GetLocalAddr(ServerLocalAddr));

    MsQuicConnection Connection(Registration);
    TEST_QUIC_SUCCEEDED(Connection.GetInitStatus());
    TEST_QUIC_SUCCEEDED(Connection.Start(
        ClientConfiguration,
        ServerLocalAddr.GetFamily(),
        QUIC_TEST_LOOPBACK_FOR_AF(ServerLocalAddr.GetFamily()),
        ServerLocalAddr.GetPort()));

    TEST_TRUE(Connection.HandshakeCompleteEvent.WaitTimeout(TestWaitTimeout));

    MsQuicStream Stream(
        Connection,
        QUIC_STREAM_OPEN_FLAG_APP_OWNED_BUFFERS,
        CleanUpManual);
    TEST_QUIC_SUCCEEDED(Stream.GetInitStatus());

    //
    // First call: provide a small buffer so the recv buffer is initialized
    // with a valid chunk and the worker processes it normally.
    //
    uint8_t SmallBuf[64];
    QUIC_BUFFER InitBuffer[1] = {
        { sizeof(SmallBuf), SmallBuf }
    };
    Stream.ProvideReceiveBuffers(1, InitBuffer);
    CxPlatSleep(50);

    //
    // Second call: provide buffers whose total AllocLength (when added to
    // existing buffer) overflows UINT32_MAX. QuicRecvBufferProvideChunks
    // returns QUIC_STATUS_INVALID_PARAMETER without consuming the chunks.
    // The chunks remain in the operation's Chunks list.
    //
    // After the failure, QuicConnFatalError aborts the connection. The
    // subsequent teardown calls QuicOperationFree, which uses CXPLAT_FREE
    // on the pool-allocated chunks — an invalid free detectable by ASAN.
    //
    uint8_t Buf1[64], Buf2[64];
    QUIC_BUFFER OverflowBuffers[2] = {
        { UINT32_MAX, Buf1 },
        { 1024, Buf2 }
    };
    Stream.ProvideReceiveBuffers(2, OverflowBuffers);

    //
    // Wait for the worker to process the operation, hit the error,
    // call QuicConnFatalError, and tear down the connection.
    //
    CxPlatSleep(200);
}

Expected behavior

Test should pass without error.

Actual outcome

Under ASAN, this reports attempting free on address which was not malloc()-ed.

Additional details

No response

Metadata

Metadata

Type

Projects

Status

No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions