c++ - 如何修复“在一个大小为 64 的块内的大小为 8 - 40 字节的无效读取”

Question

m_PhyToBtMap 中的 SPacket 中的 shared_ptr 似乎导致“在大小为 64 的块内的大小为 8 - 40 字节的无效读取已释放。” 注意：在 valgrind（下面的日志）发出此错误消息之前，这运行了近 22 小时，有数百万条消息，但我也在 EraseAcknowledgedPackets（下面）中收到 SIGSEGV 崩溃，并怀疑这是原因。我使用的是 Boost 1.63，因为交叉编译器不支持 shared_ptr。SendMessageToBt（大小为 8 的无效读取）和 EraseAcknowledgedPackets（大小为 64 的块内的 40 个字节）在 valgrind 日志中被调用。

我使用 shared_ptr 或 make_shared 不当？
在执行 map.erase 或 insert 之前是否需要添加任何检查？
我是否正确理解了 valgrind 日志？

SPacket 和 m_PhyToBtMap

typedef struct SPacket
{
    boost::shared_ptr<uint8_t[]> data;
    size_t size;
} SPacket;
map<uint16_t, SPacket> m_PhyToBtMap;

SendMessageToBt

void RadioManager::SendMessageToBt(uint8_t * response, size_t responseSize)
{
    CSrProtected ThreadSafe(m_LockPhyToBt);
    SPacket sentPacket;
    sentPacket.size = MESSAGE_HEADER_OFFSET + responseSize + CRC32_SIZE;
    sentPacket.data = boost::make_shared<uint8_t[]>(sentPacket.size);
    assert(sentPacket.data.get());
    memcpy(&sentPacket.data.get()[MESSAGE_HEADER_OFFSET], response, responseSize);
    m_AcknowledgementNumberSent = m_NextReceiveSequenceNumber;
    m_SequenceNumberSent = m_NextSendSequenceNumber;
    ++m_NextSendSequenceNumber;
    if (0 == m_NextSendSequenceNumber) m_NextSendSequenceNumber = SEQUENCE_NUMBER_MIN;
    m_PhyToBtMap[m_SequenceNumberSent] = sentPacket; // RadioManager.cpp:246
    sentPacket.data.get()[DATAGRAM_ACKNOWLEDGEMENT_LSB] = m_AcknowledgementNumberSent;
    sentPacket.data.get()[DATAGRAM_ACKNOWLEDGEMENT_MSB] = m_AcknowledgementNumberSent >> 8;
    sentPacket.data.get()[DATAGRAM_HEADER_SIZE + SEQUENCE_NUMBER_LIST_SIZE_LSB] = m_SequenceNumberSent;
    sentPacket.data.get()[DATAGRAM_HEADER_SIZE + SEQUENCE_NUMBER_LIST_SIZE_MSB] = m_SequenceNumberSent >> 8;
    SetCrc32(sentPacket.data.get(), sentPacket.size - CRC32_SIZE);
    m_Socket->SendTo(sentPacket.data.get(), sentPacket.size);
}

擦除确认包

void RadioManager::EraseAcknowledgedPackets(uint16_t acknowledgementNumber)
{
    CSrProtected ThreadSafe(m_LockPhyToBt);
    if (!m_PhyToBtMap.empty())
    {
        map<uint16_t, SPacket>::iterator it = m_PhyToBtMap.upper_bound(acknowledgementNumber);
        int begin = m_PhyToBtMap.begin()->first;
        if (begin > acknowledgementNumber) m_PhyToBtMap.erase(it, m_PhyToBtMap.end()); // acknowledgementNumber rollover
        else m_PhyToBtMap.erase(m_PhyToBtMap.begin(), it); // RadioManager.cpp:1113
    }
}

valgrind 日志

 Invalid read of size 8
    at 0x474F84: void std::swap<unsigned char*>(unsigned char*&, unsigned char*&) (move.h:176)
    by 0x47430A: boost::shared_ptr<unsigned char []>::swap(boost::shared_ptr<unsigned char []>&) (shared_ptr.hpp:743)
    by 0x473042: boost::shared_ptr<unsigned char []>::operator=(boost::shared_ptr<unsigned char []> const&) (shared_ptr.hpp:526)
    by 0x4729B8: SPacket::operator=(SPacket const&) (RadioManager.h:32)
    by 0x467C8E: RadioManager::SendMessageToBt(unsigned char*, unsigned long) (RadioManager.cpp:246)
    by 0x46B64C: RadioManager::TransmitFecBlockResponseEvent(unsigned char*, unsigned long) (RadioManager.cpp:683)
    by 0x4A5FDD: BtToRadioInterfaceClient::ProcessMessage(unsigned char*, unsigned long) (BtToRadioInterface.cpp:174)
    by 0x4AB6AD: SrZmqPubSubInterface::ReadThread(void*) (SrZmqPubSubInterface.cpp:220)
    by 0x4AC258: SingleParamObjCallback<SrZmqPubSubInterface, void*>::operator()(void*) (CallbackFunctors.h:161)
    by 0x4AC1D4: CSrClassThread<SrZmqPubSubInterface, void*>::ThreadProcedure(void*) (SrClassThread.h:21)
    by 0x46360A: SrThreadProcedure(void*) (SrThread.cpp:41)
    by 0x50BCDC4: start_thread (pthread_create.c:308)
    by 0x610273C: clone (clone.S:113)
  Address 0xb93c638 is 40 bytes inside a block of size 64 free'd
    at 0x4C29131: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x476841: __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<unsigned short const, SPacket> > >::deallocate(std::_Rb_tree_node<std::pair<unsigned short const, SPacket> >*, unsigned long) (new_allocator.h:110)
    by 0x47562D: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::_M_put_node(std::_Rb_tree_node<std::pair<unsigned short const, SPacket> >*) (stl_tree.h:374)
    by 0x4748BC: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::_M_destroy_node(std::_Rb_tree_node<std::pair<unsigned short const, SPacket> >*) (stl_tree.h:396)
    by 0x473AB8: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::_M_erase(std::_Rb_tree_node<std::pair<unsigned short const, SPacket> >*) (stl_tree.h:1127)
    by 0x473C8C: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::clear() (stl_tree.h:860)
    by 0x4754F1: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::_M_erase_aux(std::_Rb_tree_const_iterator<std::pair<unsigned short const, SPacket> >, std::_Rb_tree_const_iterator<std::pair<unsigned short const, SPacket> >) (stl_tree.h:1757)
    by 0x474706: std::_Rb_tree<unsigned short, std::pair<unsigned short const, SPacket>, std::_Select1st<std::pair<unsigned short const, SPacket> >, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::erase(std::_Rb_tree_iterator<std::pair<unsigned short const, SPacket> >, std::_Rb_tree_iterator<std::pair<unsigned short const, SPacket> >) (stl_tree.h:848)
    by 0x47345E: std::map<unsigned short, SPacket, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, SPacket> > >::erase(std::_Rb_tree_iterator<std::pair<unsigned short const, SPacket> >, std::_Rb_tree_iterator<std::pair<unsigned short const, SPacket> >) (stl_map.h:763)
    by 0x46E7A3: RadioManager::EraseAcknowledgedPackets(unsigned short) (RadioManager.cpp:1113)
    by 0x46D785: RadioManager::Decode(unsigned char*, unsigned long) (RadioManager.cpp:935)
    by 0x4655DE: MsgDispatcher::Decode(UdpState*) (MsgDispatcher.cpp:20)
    by 0x465976: SingleParamObjCallback<MsgDispatcher, UdpState*>::operator()(UdpState*) (CallbackFunctors.h:161)
    by 0x464CBC: IsimUdpSocket::ReceiveHandler(void*) (IsimUdpSocket.cpp:41)
    by 0x465410: SingleParamObjCallback<IsimUdpSocket, void*>::operator()(void*) (CallbackFunctors.h:161)
    by 0x46538C: CSrClassThread<IsimUdpSocket, void*>::ThreadProcedure(void*) (SrClassThread.h:21)
    by 0x46360A: SrThreadProcedure(void*) (SrThread.cpp:41)

score 4 · Accepted Answer

我创建了这个 SSCCE Live On Coliru来实现我所做的任何假设：

#include <boost/make_shared.hpp>
#include <boost/thread.hpp>

constexpr unsigned SEQUENCE_NUMBER_MIN = 100u;

struct SPacket {
    boost::shared_ptr<uint8_t[]> data;
    size_t size;
};

struct RadioManager {
    void SendMessageToBt(uint8_t const* response, size_t responseSize);
    void SetCrc32(uint8_t * buffer, size_t offset) {
        buffer[offset++] = 0; // TODO
        buffer[offset++] = 0;
        buffer[offset++] = 0;
        buffer[offset++] = 0;
    }

    void EraseAcknowledgedPackets(uint16_t acknowledgementNumber);

  private:
    mutable boost::mutex m_LockPhyToBt;
    using CSrProtected = boost::mutex::scoped_lock;

    std::map<uint16_t, SPacket> m_PhyToBtMap;

    uint16_t m_NextReceiveSequenceNumber = SEQUENCE_NUMBER_MIN;
    uint16_t m_AcknowledgementNumberSent;
    uint16_t m_NextSendSequenceNumber = SEQUENCE_NUMBER_MIN;
    uint16_t m_SequenceNumberSent;
    enum {
        DATAGRAM_ACKNOWLEDGEMENT_LSB = 0,
        DATAGRAM_ACKNOWLEDGEMENT_MSB = 1,
        DATAGRAM_HEADER_SIZE = 8,
        SEQUENCE_NUMBER_LIST_SIZE_LSB = 0,
        SEQUENCE_NUMBER_LIST_SIZE_MSB = 1,
        MESSAGE_HEADER_OFFSET = 10,
        //
        CRC32_SIZE = 4
    };

};

void RadioManager::EraseAcknowledgedPackets(uint16_t acknowledgementNumber) {
    CSrProtected ThreadSafe(m_LockPhyToBt);
    auto it = m_PhyToBtMap.upper_bound(acknowledgementNumber);

    int begin = m_PhyToBtMap.begin()->first;

    if (begin > acknowledgementNumber)
        m_PhyToBtMap.erase(it, m_PhyToBtMap.end()); // acknowledgementNumber rollover
    else
        m_PhyToBtMap.erase(m_PhyToBtMap.begin(), it); // RadioManager.cpp:1113
}

void RadioManager::SendMessageToBt(uint8_t const* response, size_t responseSize)
{
    CSrProtected ThreadSafe(m_LockPhyToBt);
    SPacket sentPacket;
    sentPacket.size = MESSAGE_HEADER_OFFSET + responseSize + CRC32_SIZE;
    sentPacket.data = boost::make_shared<uint8_t[]>(sentPacket.size);

    auto data = sentPacket.data.get();
    assert(data);

    memcpy(data + MESSAGE_HEADER_OFFSET, response, responseSize);

    m_AcknowledgementNumberSent = m_NextReceiveSequenceNumber;
    m_SequenceNumberSent        = m_NextSendSequenceNumber;
    if (0 == ++m_NextSendSequenceNumber)
        m_NextSendSequenceNumber = SEQUENCE_NUMBER_MIN;

    m_PhyToBtMap[m_SequenceNumberSent] = sentPacket; // RadioManager.cpp:246

    data[DATAGRAM_ACKNOWLEDGEMENT_LSB] = m_AcknowledgementNumberSent;
    data[DATAGRAM_ACKNOWLEDGEMENT_MSB] = m_AcknowledgementNumberSent >> 8;

    data[DATAGRAM_HEADER_SIZE + SEQUENCE_NUMBER_LIST_SIZE_LSB] = m_SequenceNumberSent;
    data[DATAGRAM_HEADER_SIZE + SEQUENCE_NUMBER_LIST_SIZE_MSB] = m_SequenceNumberSent >> 8;

    SetCrc32(data, sentPacket.size - CRC32_SIZE);
    //m_Socket->SendTo(data, sentPacket.size);
}

int main() {
    RadioManager rm;
    uint8_t message[] = "HELLO WORLD";
    rm.SendMessageToBt(message, sizeof(message));
    rm.SendMessageToBt(message, sizeof(message));
    rm.EraseAcknowledgedPackets(SEQUENCE_NUMBER_MIN);
    rm.SendMessageToBt(message, sizeof(message));
    rm.SendMessageToBt(message, sizeof(message));
}

久而久之，我只能看到

显示的代码没有问题（假设这CSrProtected确实是“关键部分”的范围锁，所以喜欢std::lock_guard<std::mutex>）。
为了让里面SendMessageToBt的那条线读取触发，它实际上应该被误报越界。换句话说，该消息说它在分配的data成员时尝试读取 8 个字节SPacket。

它还报告该地址是由 shared-ptr 先前分配的 64 字节块中的 40 字节（因此某些东西在分配给另一个共享指针的内部）。char[]

由于我们知道sentPacket完全在堆栈上，“某物”不可能是shared_ptr<>对象本身，并且由于在uint8_t[]复制共享指针时没有复制数据，所以我们知道它必须是控制块。

然而这没有任何意义，因为shared_ptr保证控制块不会移动或消失，直到最后一个shared_ptr<>实例消失。我们在堆栈上至少有 1 个，所以...

所有这一切只会导致 1 个可能的结果：其他地方存在未定义的行为（可能会破坏SPacket堆栈上的实例？）。UB 的一个不太可能的来源确实是可能缺乏线程同步。

检查它CSrProtected是否符合您的想法
检查对成员数据的所有操作是否在正确的关键部分同步

其他注意事项：

分开有点奇怪size。现在，的生命周期data是共享的，但大小不是。为什么不换成你这样 SPacket的东西shared_ptr<vector<uint8_t> >
- 没有那种奇怪的分裂
- 可以使用vector::at而不是未经检查的operator[]索引。这将保护您免受越界（如果DATAGRAM_HEADER_SIZE + SEQUENCE_NUMBER_LIST_SIZE_LSB碰巧是 >MESSAGE_HEADER_OFFSET等）。

你已经在使用 Valgrind。也许 ASan/UBSan 编译器开关会有所帮助。

c++ - 如何修复“在一个大小为 64 的块内的大小为 8 - 40 字节的无效读取”

1 回答 1

Related

Reference