etl
Fix numeric overflow in bip buffer's get_write_reserve
#1088
Merged

Fix numeric overflow in bip buffer's get_write_reserve #1088

jedrzejboczar
jedrzejboczar20 days ago

The bug happens because psize is set to numeric_limits<size_type>::max() so the code path in get_write_reserve has numeric overflow and won't update psize.

The following sequence reproduces the issue:

#include <format>
#include <iostream>

#include <etl/bip_buffer_spsc_atomic.h>

int main()
{
    etl::bip_buffer_spsc_atomic<uint8_t, 256> buf;

    auto b1 = buf.write_reserve_optimal();
    assert(b1.size() == 256);
    buf.write_commit(b1.first(256));

    auto b2 = buf.read_reserve();
    assert(b2.size() == 256);

    auto b3 = buf.write_reserve_optimal();
    assert(b3.size() == 0);

    buf.read_commit(b2.first(17));

    auto b4 = buf.read_reserve();
    assert(b4.size() == 239);

    auto b5 = buf.write_reserve_optimal();
    assert(b5.size() == 16);
    buf.write_commit(b5.first(16));

    buf.read_commit(b4.first(10));

    auto b6 = buf.read_reserve();
    assert(b6.size() == 229);

    auto b7 = buf.write_reserve_optimal(); // will cause numeric overflow and return huge span
    std::cout << std::format("b7.size() = {}\n", b7.size());
    assert(b7.size() <= 256);

    return 0;
}
benedekkupper
benedekkupper13 days ago

Good point! Wouldn't it be a clearer change to change if ((write_index + *psize) >= read_index) to if (*psize >= (read_index - write_index)) though? Since at that point read_index > write_index, no over/underflows will occur.

jedrzejboczar Fix numeric overflow in bip buffer's get_write_reserve
72838fa7
jedrzejboczar jedrzejboczar force pushed from 2eda21d7 to 72838fa7 10 days ago
jedrzejboczar
jedrzejboczar10 days ago👍 1

Yeah, that makes sense and is much cleaner. Updated.

jwellbelove jwellbelove changed the base branch from master to development 9 days ago
jwellbelove jwellbelove merged 95a4b107 into development 9 days ago

Login to write a write a comment.

Login via GitHub

Reviewers
No reviews
Assignees
No one assigned
Labels
Milestone