c++ linux send uninitialised sockets memory

Question

I'm new and I'm using debian 10 x64. Why is error in the line with SEND? Someone can sugest me how to fix that problem?

send(socket, (char*)m_MsgBuf+sendBytes, std::min(m_MsgSize-sendBytes+2, 1000),

I got that ERROR LOG:

==24819== Thread 3:
==24819== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==24819==    at 0x51442B4: __libc_send (send.c:28)
==24819==    by 0x51442B4: send (send.c:23)
==24819==    by 0x1FCFC9: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:117)
==24819==    by 0x231624: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==24819==    by 0x22ECF4: Protocol76::sendThingAppear(Thing const*) (protocol76.cpp:3039)
==24819==    by 0x214DE1: Player::onThingAppear(Thing const*) (player.cpp:2165)
==24819==    by 0x1829D0: Game::sendAddThing(Player*, Position const&, Thing const*) (game.cpp:7005)
==24819==    by 0x16789F: Game::placeCreature(Position&, Creature*, int*) (game.cpp:1260)
==24819==    by 0x223995: Protocol76::ConnectPlayer(int*) (protocol76.cpp:66)
==24819==    by 0x208C43: ConnectionHandler(void*) (otserv.cpp:575)
==24819==    by 0x5139F26: start_thread (pthread_create.c:479)
==24819==    by 0x55F12AE: clone (clone.S:95)
==24819==  Address 0x111137f1 is 289 bytes inside a block of size 16,912 alloc'd
==24819==    at 0x4836DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==24819==    by 0x2086ED: ConnectionHandler(void*) (otserv.cpp:510)
==24819==    by 0x5139F26: start_thread (pthread_create.c:479)
==24819==    by 0x55F12AE: clone (clone.S:95)
==24819== 

CODE:

bool NetworkMessage::WriteToSocket(SOCKET socket)
{
    if (m_MsgSize == 0)
        return true;

    m_MsgBuf[0] = (unsigned char)(m_MsgSize);
    m_MsgBuf[1] = (unsigned char)(m_MsgSize >> 8);

    bool ret = true;
    int32_t sendBytes = 0;
    int32_t flags = 0;
    int32_t retry = 0;

    flags = MSG_DONTWAIT; // 2 ?

    do
    {
        int32_t b = send(socket, (char*)m_MsgBuf+sendBytes, std::min(m_MsgSize-sendBytes+2, 1000), flags);

        if(b <= 0)
        {   
            int32_t errnum;

            if(errnum == EWOULDBLOCK) 
            {
                b = 0;
                OTSYS_SLEEP(10);
                retry++;

                if(retry == 10) 
                {
                    ret = false;
                    break;
                }
            }
            else
            {
                ret = false;
                break;
            }
        }
        sendBytes += b;
    }while(sendBytes < m_MsgSize+2);

    return ret;
}


==930== Thread 3:
==930== Conditional jump or move depends on uninitialised value(s)
==930==    at 0x1FCFE7: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:106)
==930==    by 0x208340: ConnectionHandler(void*) (otserv.cpp:427)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930==
==930== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==930==    at 0x51442B4: __libc_send (send.c:28)
==930==    by 0x51442B4: send (send.c:23)
==930==    by 0x1FCFD9: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:100)
==930==    by 0x231650: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==930==    by 0x22ED20: Protocol76::sendThingAppear(Thing const*) (protocol76.cpp:3039)
==930==    by 0x214E0D: Player::onThingAppear(Thing const*) (player.cpp:2165)
==930==    by 0x1829D0: Game::sendAddThing(Player*, Position const&, Thing const*) (game.cpp:7005)
==930==    by 0x16789F: Game::placeCreature(Position&, Creature*, int*) (game.cpp:1260)
==930==    by 0x2239C1: Protocol76::ConnectPlayer(int*) (protocol76.cpp:66)
==930==    by 0x208C6F: ConnectionHandler(void*) (otserv.cpp:575)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930==  Address 0x111198d1 is 289 bytes inside a block of size 16,912 alloc'd
==930==    at 0x4836DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==930==    by 0x208719: ConnectionHandler(void*) (otserv.cpp:510)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930==
==930== Conditional jump or move depends on uninitialised value(s)
==930==    at 0x1FCFE7: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:106)
==930==    by 0x231650: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==930==    by 0x22ED20: Protocol76::sendThingAppear(Thing const*) (protocol76.cpp:3039)
==930==    by 0x214E0D: Player::onThingAppear(Thing const*) (player.cpp:2165)
==930==    by 0x1829D0: Game::sendAddThing(Player*, Position const&, Thing const*) (game.cpp:7005)
==930==    by 0x16789F: Game::placeCreature(Position&, Creature*, int*) (game.cpp:1260)
==930==    by 0x2239C1: Protocol76::ConnectPlayer(int*) (protocol76.cpp:66)
==930==    by 0x208C6F: ConnectionHandler(void*) (otserv.cpp:575)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930== Thread 2:
==930== Conditional jump or move depends on uninitialised value(s)
==930==    at 0x1FCFE7: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:106)
==930==    by 0x231650: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==930==    by 0x214089: Player::flushMsg() (player.cpp:1867)
==930==    by 0x182338: Game::flushSendBuffers() (game.cpp:6879)
==930==    by 0x17AAB2: Game::checkCreature(unsigned int) (game.cpp:5461)
==930==    by 0x1A4B67: std::mem_fun1_t<void, Game, unsigned int>::operator()(Game*, unsigned int) const (stl_function.h:1284)
==930==    by 0x1A1668: std::binder2nd<std::mem_fun1_t<void, Game, unsigned int> >::operator()(Game* const&)const (binders.h:158)
==930==    by 0x19BCCD: boost::detail::function::void_function_obj_invoker1<std::binder2nd<std::mem_fun1_t<void, Game, unsigned int>>,void, Game*>::invoke(boost::detail::function::function_buffer&, Game*) (function_template.hpp:159)
==930==    by 0x23810D: boost::function1<void, Game*>::operator()(Game*) const (function_template.hpp:768)
==930==    by 0x237FBC: TSchedulerTask::operator()(Game*) (scheduler.h:63)
==930==    by 0x166D2B: Game::eventThread(void*) (game.cpp:1045)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==
==930== Thread 3:
==930== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==930==    at 0x51442B4: __libc_send (send.c:28)
==930==    by 0x51442B4: send (send.c:23)
==930==    by 0x1FCFD9: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:100)
==930==    by 0x231650: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==930==    by 0x214089: Player::flushMsg() (player.cpp:1867)
==930==    by 0x182338: Game::flushSendBuffers() (game.cpp:6879)
==930==    by 0x224573: Protocol76::parsePacket(NetworkMessage&) (protocol76.cpp:405)
==930==    by 0x223A3F: Protocol76::ReceiveLoop() (protocol76.cpp:86)
==930==    by 0x208E70: ConnectionHandler(void*) (otserv.cpp:611)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930==  Address 0x11119f5f is 1,967 bytes inside a block of size 16,912 alloc'd
==930==    at 0x4836DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==930==    by 0x208719: ConnectionHandler(void*) (otserv.cpp:510)
==930==    by 0x5139F26: start_thread (pthread_create.c:479)
==930==    by 0x55F12AE: clone (clone.S:95)
==930==

I use code like this:

bool NetworkMessage::WriteToSocket(SOCKET socket)
{
    if (m_MsgSize == 0)
        return true;

    m_MsgBuf[0] = (unsigned char)(m_MsgSize);
    m_MsgBuf[1] = (unsigned char)(m_MsgSize >> 8);

    bool ret = true;
    int32_t sendBytes = 0;
    int32_t flags = 0;
    int32_t retry = 0;

    flags = MSG_DONTWAIT; // 2 ?


    while (sendBytes != m_MsgSize) 
    { /* != rather than < */
        int32_t b = send(socket, (char*)m_MsgBuf+sendBytes, std::min(m_MsgSize-sendBytes+2, 1000), flags);

        if(b <= 0)
        {   
            int32_t errnum;

            if(errnum == EWOULDBLOCK) 
            {
                b = 0;
                OTSYS_SLEEP(10);
                retry++;

                if(retry == 10) 
                {
                    ret = false;
                    break;
                }
            }
            else
            {
                ret = false;
                break;
            }
        }
        sendBytes += b;
    }

    return ret;
}

EDIT

the last time when i had similar error I needed to use that code and now everything is working: sigemptyset(&sigh.sa_mask); OR memset(&sigh, 0, sizeof(sigh)); here i found all msgBuf and msg Size in my project files. Do U see something intresting here? m_MsgBuf wklejto.pl/828244 m_MsgSize wklejto.pl/828245

EDIT

so I did this, in file NetworkMessage.cpp I have added under AddString/AddU32/AddU16/AddByte this:

#ifdef CHECKVAL
    checkval(m_MsgBuf+m_ReadPos, stringlen);
#endif  

on the top this file I have added:

#include "player.h"

#ifdef CHECKVAL
    extern int checkval(unsigned char * buff, int len);
#endif

and to the end added this code to player.cpp

#ifdef CHECKVAL
int checkval(unsigned char * buff, int len)
{
   int r = 0;

   buff -= len;
   while (len--)
     r += *buff++;
   return r;
}
#endif

and also I have changed char to unsigned char - bcs I got error invalid conversion from ‘unsigned char*’ to ‘char*’

to END I added to compilator this -DCHECKVAL

after that I see same error

==15290== Thread 3:
==15290== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==15290==    at 0x51442B4: __libc_send (send.c:28)
==15290==    by 0x51442B4: send (send.c:23)
==15290==    by 0x1FCFDC: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:105)
==15290==    by 0x231728: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==15290==    by 0x22EDF8: Protocol76::sendThingAppear(Thing const*) (protocol76. cpp:3039)
==15290==    by 0x214E99: Player::onThingAppear(Thing const*) (player.cpp:2165)
==15290==    by 0x1829D0: Game::sendAddThing(Player*, Position const&, Thing const*) (game.cpp:7005)
==15290==    by 0x16789F: Game::placeCreature(Position&, Creature*, int*) (game.cpp:1260)
==15290==    by 0x223A99: Protocol76::ConnectPlayer(int*) (protocol76.cpp:66)
==15290==    by 0x208CFB: ConnectionHandler(void*) (otserv.cpp:575)
==15290==    by 0x5139F26: start_thread (pthread_create.c:479)
==15290==    by 0x55F12AE: clone (clone.S:95)
==15290==  Address 0x11116331 is 289 bytes inside a block of size 16,912 alloc'd
==15290==    at 0x4836DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==15290==    by 0x2087A5: ConnectionHandler(void*) (otserv.cpp:510)
==15290==    by 0x5139F26: start_thread (pthread_create.c:479)
==15290==    by 0x55F12AE: clone (clone.S:95)
==15290==
==15290== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==15290==    at 0x51442B4: __libc_send (send.c:28)
==15290==    by 0x51442B4: send (send.c:23)
==15290==    by 0x1FCFDC: NetworkMessage::WriteToSocket(int) (networkmessage.cpp:105)
==15290==    by 0x231728: Protocol76::flushOutputBuffer() (protocol76.cpp:3574)
==15290==    by 0x214115: Player::flushMsg() (player.cpp:1867)
==15290==    by 0x182338: Game::flushSendBuffers() (game.cpp:6879)
==15290==    by 0x22464B: Protocol76::parsePacket(NetworkMessage&) (protocol76.cpp:405)
==15290==    by 0x223B17: Protocol76::ReceiveLoop() (protocol76.cpp:86)
==15290==    by 0x208EFC: ConnectionHandler(void*) (otserv.cpp:611)
==15290==    by 0x5139F26: start_thread (pthread_create.c:479)
==15290==    by 0x55F12AE: clone (clone.S:95)
==15290==  Address 0x111169bf is 1,967 bytes inside a block of size 16,912 alloc'd
==15290==    at 0x4836DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==15290==    by 0x2087A5: ConnectionHandler(void*) (otserv.cpp:510)
==15290==    by 0x5139F26: start_thread (pthread_create.c:479)
==15290==    by 0x55F12AE: clone (clone.S:95)
==15290==

hmm.. but this function looks intresting ConnectionHandler in the line 510 is problem?

            Protocol76* protocol;
            protocol = new Protocol76(s);

and:

Protocol76::Protocol76(SOCKET s)
{
    OTSYS_THREAD_LOCKVARINIT(bufferLock);

    player = NULL;
    pendingLogout = false;

    windowTextID = 0;
    readItem = NULL;
    this->s = s;
}

full ConnectionHandler and password is 123

enter link description here

line 510 in the file is 152 how do u think?

something like this can be enough?

int checkval(unsigned char * buff, int len)
{
    buff -= len;

    ofstream bufile ("buff.txt");

    if (bufile.is_open())
    {
        while (len--)
        {
            bufile << "buff: " << *buff << "len: " << len;
        }
        bufile.close();
    }
}

Answer 1

Supposing you initialized m_MsgSize bytes in m_MsgBuf and you want to send them in a loop block by block whose size is

std::min(m_MsgSize-sendBytes+2, 1000)

because of the "+2" depending on the initial value of m_MsgSize you can send 1 or 2 extra bytes after the first m_MsgSize bytes, these extra bytes can be just non initialized or even out of m_MsgBuf with an undefined behavior

Notice also that if you do

  int32_t sendBytes = 0

  while (sendBytes != m_MsgSize) { /* != rather than < */
     int32_t b = send(socket, (char*)m_MsgBuf+sendBytes, std::min(m_MsgSize-sendBytes+2, 1000), ...);

     ... error management
     sendBytes += b;
  }

because of the error introduced by the +2 you can have sendBytes greater than m_MsgSize with probable bad effects.

Just remove that "+2"

---

[edit after you edit you question/you answer]

Your +2 is not a problem in case m_MsgSize do not count the two additional bytes to send the length of the next bytes

valgrind signal few errors and they concern bytes inside the buffer, not the 1 or 2 last ones, for me that means you copy in m_MsgBuf few bytes non initialized. Sometimes valgrind does not signal immediately when a value was not initialized, for instance you can have :

int i; // not initialized

f(i); // nothing signaled by valgrind even i not initialized


...

void f(int i)
{
   int j = i + 1; // here valgrind signal a non initialized value

I think you are in that case.

Note this may be a non error, imagine you do that :

// set some elements
char s[20];
...    
strcpy(s, "aze");
...
memcpy(m_MsgBuf + 2, /* save 2 bytes for the length */
       s, sizeof(s));
m_MsgSize = sizeof(s);
... may be some oher element added from m_MsgSize+2 and updating m_MsgSize
NetworkMessage::WriteToSocket(socket);

only 4 bytes was initialized in s but to have a constant size sent you send (at least) 20 bytes => 16 bytes in the buffer correspond to non initialized values => valgrind will signal them

To know if you are in a 'non error' case or if valgrind signals a real problem you have to analyze the setting of all the values you put in the buffer, but again warning valgrind can take time to signal the use of a non initialized value, that value may be was used by several intermediate location before to reach hte buffer you send

---

[edit from your remark giving piece of code]

The initialized value probably come from the caller of the methodes NetworkMessage::AddXXX, for instance NetworkMessage::AddByte receive in argument a non initialized byte.

The fact you have that class with dedicated method to add data in the buffer is a chance, you can modify their definitions with additional code to artificially use the bytes from the buffer to allow valgrind to detect a non initialized value. You can put the additional codes protected by #ifdef ... #endif to easily activate/deactivate it

For instance (use the type of m_MsgBuf to type the parameter buff of checkval, I used "char *" because I do not know the type of m_MsgBuf) :

#ifdef CHECKVAL
extern int checkval(char * buff, int len);
#endif

void NetworkMessage::AddByte(unsigned char value)
{
    if(!canAdd(1))
        return;
    m_MsgBuf[m_ReadPos++] = value;
    m_MsgSize++;
#ifdef CHECKVAL
    checkval(m_MsgBuf+m_ReadPos, 1);
#endif
}

void NetworkMessage::AddU16(uint16_t value)
{
    if(!canAdd(2))
        return;
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value);
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value >> 8);
    m_MsgSize += 2;
#ifdef CHECKVAL
    checkval(m_MsgBuf+m_ReadPos, 2);
#endif
}

void NetworkMessage::AddU32(uint32_t value)
{
    if(!canAdd(4))
        return;
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value);
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value >>  8);
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value >> 16);
    m_MsgBuf[m_ReadPos++] = (unsigned char)(value >> 24);
    m_MsgSize += 4;
#ifdef CHECKVAL
    checkval(m_MsgBuf+m_ReadPos, 4);
#endif
}

void NetworkMessage::AddString(const char* value)
{
    uint32_t stringlen = (uint32_t) strlen(value);
    if(!canAdd(stringlen+2) || stringlen > 8192)
        return;

#ifdef USING_VISUAL_2005
    strcpy_s((char*)m_MsgBuf + m_ReadPos, stringlen, value); //VISUAL
#else
    AddU16((uint16_t)stringlen);
    strcpy((char*)m_MsgBuf + m_ReadPos, value);
#endif //USING_VISUAL_2005
    m_ReadPos += stringlen;
    m_MsgSize += stringlen;
#ifdef CHECKVAL
    checkval(m_MsgBuf+m_ReadPos, stringlen);
#endif
}

checkval have to access to each byte, for instance :

#ifdef CHECKVAL
int checkval(char * buff, int len)
{
   int r = 0;

   buff -= len;
   while (len--)
     r += *buff++;
   return r;
}
#endif

and place checkval in an other file than where *NetworkMessage:Addxxx" are defined to be sure the compiler cannot detect it is useless to compute the sum of bytes from the buffer or useless to call checkval because the return value is never used and checkval has no side effect.

If to sum the byte is not enough to force valgrind to check if the bytes are initialized or not, change the definition for instance to save the bytes in a file etc

Of course compile defining CHECKVAL through compiler option, or just adding temporary

#define CHECKVAL

before the definition of checkval and before its declaration

When valgrind will detect a non initialized byte in checkval you will be able to know where the value come from looking at the stack of calls also produced by valgrind