Skip to content

Conversation

@casperisfine
Copy link

Fix: #342
Ref: #341

Reseting the memory in _msgpack_buffer_alloc_new_chunk was pointless because the previous tail is immediately copied into it, and it's the tail that is then used by the caller. So it's the tail we should have zeroed-out.

I think what is happening is in _msgpack_buffer_expand:

    if(b->tail.mapped_string != NO_MAPPED_STRING || capacity <= MSGPACK_RMEM_PAGE_SIZE) {
        /* allocate new chunk */
        _msgpack_buffer_add_new_chunk(b);

        char* mem = _msgpack_buffer_chunk_malloc(b, &b->tail, length, &capacity);

        char* last = mem;
        if(data != NULL) {
            memcpy(mem, data, length);
            last += length;
        }

        /* rebuild tail chunk */
        b->tail.first = mem;
        b->tail.last = last;
        b->tail.mapped_string = NO_MAPPED_STRING;
        b->tail_buffer_end = mem + capacity;

_msgpack_buffer_add_new_chunk potentially leave b->tail.mapped_string as a garbage reference, then _msgpack_buffer_chunk_malloc may trigger a GC.

b->tail.mapped_string is only reset later.

I'm still trying to craft a repro for such error case.

Fix: msgpack#342

Reseting the memory in _msgpack_buffer_alloc_new_chunk was pointless
because the previous `tail` is immediately copied into it, and it's
the `tail` that is then used by the caller. So it's the `tail` we
should zero-out.
@byroot byroot closed this Sep 25, 2025
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.

2 participants