Сервер сокетов с epoll дает неизвестные байты при отключении

У меня есть сервер сокетов на C++, и я использую epoll. Я отправляю на сервер символ, содержащий HeaderPacket и NormalPacket. Сначала я читаю HeaderPacket, а после этого я читаю NormalPacket.

И теперь проблема: когда я закрываю клиент (я пытался использовать close и shutdown - тот же вывод), я получаю какие-то странные байты в первом recv (тот, который читает пакет заголовка) и после этой ошибки сегментации.

Кроме того, когда я изменяю размер своего символа содержимого с HeaderPacket на другое значение, например 120, я не получаю ошибку сегментации, но когда я устанавливаю его на 40 или другое значение, я получаю ошибку сегментации.

#define BUFFERSIZE 256
#define CHARSIZE 40

Вот функция, которую я использую для чтения:

void PacketHandler::ReadBytes(int fd, struct HeaderPacket &hp, char buffer[])

{

    int reading = 0;

    ssize_t hpCount, cpCount;

    char hpBuffer[6];



    hpCount = recv(fd, hpBuffer, 6, 0);

    if(hpCount <= 0)

    {

       reading = 1;

    } else {  

        this->UnserializeHeaderPacket(hpBuffer, hp);

        print(DEBUG, Helpers::IntegerToString(hp.length)); 

        cpCount = hp.length;

        char cpBuffer[cpCount];

        memset(cpBuffer, 0, sizeof(cpBuffer));

        char* iterator = cpBuffer;

        int bytesLeft = sizeof(cpBuffer) - sizeof(char);

        print(DEBUG, Helpers::IntegerToString(bytesLeft)); 

        if(bytesLeft < 0)

        {

            reading = 1;

        }

        while(bytesLeft > 0)

        {

            ssize_t curr;

            curr = recv(fd, iterator, bytesLeft, 0);



            if(curr == -1)

            {

                if(errno != EAGAIN)

                {

                    reading = 1;

                    print(WARNING, "reading error at content packet");

                }

                break;

            } else if(curr == 0) {

                reading = 1;

                break;

            } 



            iterator += curr;

            bytesLeft -= curr;                 





        }



            memcpy(buffer, cpBuffer, sizeof cpBuffer);     

    }



    if(reading)

    {

        print(NOTICE, "Closed connection with the descriptor " + Helpers::IntegerToString(fd));

        close(fd);

    }

}

Вот функция для epoll

void EventHandler::RunningLoop(int fd)

{

    while(1)

    {

        int availableEvents, i;



        availableEvents = epoll_wait(this->efd, this->events, MAXEVENTS, -1);

        for(i = 0; i < availableEvents; i++)

        {

            if(this->events[i].data.fd == fd)

            {

                // Accepting new connection

                this->AcceptClient(fd);

                continue;

            }

            else if((this->events[i].events & EPOLLERR) || (this->events[i].events & EPOLLHUP) || (!(this->events[i].events & EPOLLIN)))

            {

                print(WARNING, "epoll error on reading from fd");

                close (this->events[i].data.fd);

                continue;

            } 

            else if(this->events[i].events & EPOLLRDHUP) 

            {

                print(WARNING, "intern close socket");

                close (this->events[i].data.fd);



            } else {

                // Reading packets

                this->run->InitializePacket(this->events[i].data.fd); // cals the read function

            }

        }

    }



    free(this->events);

    close(fd);

}

Мои пакеты:

struct HeaderPacket

{

    uint16_t opcode;

    uint32_t length;

};



struct HelloWorldPacket

{

    uint16_t byteOrder;

    char content[CHARSIZE];

};

Функция сериализации:

void PacketHandler::SerializeHeaderPacket(HeaderPacket packet, char buffer[])

{

    uint16_t u16;

    uint32_t u32;



    u16 = htons(packet.opcode);

    memcpy(buffer+0, &u16, 2);

    u32 = htonl(packet.length);

    memcpy(buffer+2, &u32, 4);

}



void PacketHandler::UnserializeHeaderPacket(char buffer[], HeaderPacket &packet)

{

    uint16_t u16;

    uint32_t u32;



    memcpy(&u16, buffer+0, 2);

    packet.opcode = ntohs(u16);

    memcpy(&u32, buffer+2, 4);

    packet.length = ntohl(u32);

}



void PacketHandler::SerializeHelloWorldPacket(HelloWorldPacket packet, char buffer[])

{

    uint16_t u16;



    u16 = htons(packet.byteOrder);

    memcpy(buffer+0, &u16, 2);

    memcpy(buffer+2, &packet.content, sizeof packet.content);

}



void PacketHandler::UnserializeHelloWorldPacket(char buffer[], HelloWorldPacket &packet)

{

    uint16_t u16;



    memcpy(&u16, buffer+0, 2);

    packet.byteOrder = ntohs(u16);



    strcpy(packet.content, buffer+2);



}

И вот как я отправляю данные на сервер:

int EventHandler::SendHelloWorld(int fd)

{

    HeaderPacket hp;

    HelloWorldPacket hc;

    char buffer[256];

    int sendResult;

    char message[] = "hello_first_message\r\n";





    hp.opcode = HELLOWORLD;

    hp.length = sizeof message;

    memcpy(hc.content, message, sizeof message);





    packets->SerializeHeaderPacket(hp, buffer);

    packets->SerializeHelloWorldPacket(hc, buffer+6);



    sendResult = write(fd, buffer, sizeof buffer);







    return sendResult;



}

Спасибо за вашу помощь.


person cemycc    schedule 24.01.2012    source источник
comment
компилируется без ошибок и предупреждений?   -  person stefanB    schedule 24.01.2012
comment
Когда соединение закрыто (recv возвращает 0), вы удаляете сокет из набора epoll?   -  person Some programmer dude    schedule 24.01.2012
comment
@Joachim PileBorg Я использую close (fd) недостаточно? Спасибо.   -  person cemycc    schedule 24.01.2012
comment
@stefanB Да, компилируется без предупреждений и ошибок.   -  person cemycc    schedule 24.01.2012
comment
Не обращайте внимания на мой комментарий. Если вы не дублируете дескриптор сокета (например, с dup и т. д.), то он должен автоматически удаляться из набора при закрытии.   -  person Some programmer dude    schedule 24.01.2012
comment
Куда вы звоните PacketHandler::ReadBytes? И предложение после вызова recv регистрировать возвращаемое значение, чтобы убедиться, что вы получаете все данные, которые хотите получить в вызове.   -  person Some programmer dude    schedule 24.01.2012
comment
@JoachimPileborg эта функция this-›run-›InitializePacket(this-›events[i].data.fd); // вызывает функцию чтения, которая содержит вызов ReadBytes и ничего больше. В функции PacketHandler::ReadBytes я проверяю возвращаемое значение из recv на -1 или 0 .   -  person cemycc    schedule 24.01.2012
comment
давайте продолжим это обсуждение в чате   -  person cemycc    schedule 24.01.2012


Ответы (1)


Я нашел проблему. Когда я пишу на сервер, я использую sizeof buffer, буфер имеет длину 256, и я не читаю 256 байт.

Спасибо за вашу помощь.

person cemycc    schedule 24.01.2012
comment
Вы имеете в виду, что клиент может сломать ваш сервер? Я надеюсь, что вы намерены исправить это и на стороне сервера. Ваша функция unserializeHelloWorld изначально небезопасна. Нужно ли уточнять, почему? Кроме того, valgrind — очень полезный инструмент для диагностики загадочных сбоев, подобных этому. Скорее всего, это пометило бы переполнение буфера в определенном месте кода перед сбоем. - person selbie; 25.01.2012
comment
@selbie, не могли бы вы объяснить, почему это небезопасно? Спасибо, попробую валгринд. - person cemycc; 25.01.2012
comment
Ошибка №1: функция ReadBytes. Он считывает 6 байтов и передает их в UnserializeHeaderPacket. Плохая покупка (со своим собственным клиентским кодом) подключается к вашему серверу и отправляет эти 6 байтов в виде потока байтов заголовочного пакета: {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}. UnserializeHeaderPacket инициализирует член hp.length равным 4294967295 (это 4 ГБ). Затем вы выделите этот объем памяти с помощью вызова char cpBuffer[cpCount]; Я почти уверен, что это приведет к сбою вашего сервера. И если это не так, оператор memcpy(buffer, cpBuffer, sizeof cpBuffer) в нижней части функции наверняка это сделает. - person selbie; 25.01.2012
comment
Ошибка № 1.1: Зная, что размер вашего буфера равен 256, я могу отправить следующий поток байтов {0xff, 0xff, 0x00, 0x00, 0xff, 0xff}. Это всего 64 КБ для объявления cpBuffer. Теперь мой клиентский поток отправляет 64 КБ данных, большинство из которых являются кодами операций x86. Когда вы делаете этот оператор memcpy в конце ReadBytes, я могу вызвать переполнение буфера, и теперь ваш сервер выполняет мой программный код. Более новые компиляторы могут вставлять средства защиты, чтобы предотвратить это, но в любом случае произойдет сбой. Это называется переполнением буфера. Читайте об этом здесь: en.wikipedia.org/wiki/Buffer_overflow - person selbie; 25.01.2012
comment
Ошибка № 2: UnserializeHelloWorldPacket вызывает strcpy для копирования сетевого буфера в буфер package.content (который составляет всего 40 байт). Что, если отправитель отправил ровно 40 байт, но не завершил свое сообщение нулем? Даже если вы заранее установили memset(cpBuffer), если я отправлю вам ровно столько байтов без нулевого символа, ваш strcpy будет скопирован в недопустимую память. И если я отправил более 40 байтов, я воспользовался еще одним переполнением буфера в вашем коде. - person selbie; 25.01.2012
comment
Вывод: когда вы пишете код для сокетов и сетей, вы должны кодировать так, как будто пир, отправляющий вам данные, пытается вас использовать. Размеры сообщений и структуры пакетов должны быть проверены перед копированием буфера. Код синтаксического анализа должен быть написан очень осторожно, и предпочтительнее пройти нечеткое тестирование. Надеюсь это поможет! - person selbie; 25.01.2012
comment
@selbie не знал об этих проблемах, я постараюсь их исправить, но проблема в том, что если я не отправлю длину своего пакета, я не знаю, сколько читать. Я постараюсь найти решение после прочтения о переполнении буфера. В любом случае спасибо за помощь. Теперь я знаю, что я сделал не так. - person cemycc; 25.01.2012
comment
Совершенно нормально указывать длину данных в заголовке. Но ваш сервер должен применять ограничения по размеру. То есть, если он увидит заголовок пакета, в котором объявлена ​​длина, превышающая размер вашего буфера, просто отбросьте сообщение и закройте соединение. - person selbie; 25.01.2012
comment
Еще одна ошибка. Я не уверен, переводит ли ваш AcceptConnection сокет удаленного клиента в неблокирующее состояние. В случае блокирующего сокета клиент может отправить заголовок, но после этого никаких данных, и просто оставить ваш сервер зависшим на вызове recv(). Если клиентский сокет был сделан неблокирующим, то есть еще одна ошибка, которая повлияет на законных клиентов. Клиентский TCP-поток может быть фрагментирован, а пакеты могут быть задержаны на мгновение, и recv() вернет -1 (errno==EGAIN), даже если входящих данных может быть больше. Ваш код разорвет законное соединение.... - person selbie; 25.01.2012
comment
... хорошей идеей было бы выделить объект контекста для каждого нового соединения, которое включает в себя буфер пакетов и ваши собственные внутренние переменные состояния для соединения. Объект контекста можно прикрепить к элементу epoll.data.ptr и ссылаться на него в последующих уведомлениях epoll. Когда я пишу код TCP, я пишу его так, как будто recv() будет возвращать только 1 байт за раз. - person selbie; 25.01.2012
comment
@selbie Моя функция AcceptConnection использует int SocketServer::ChangeSocketType(int tfd) в этой функции. Я устанавливаю клиентский сокет в неблокирующее состояние, используя fcntl. Не могли бы вы подсказать, пожалуйста, как исправить последнюю ошибку? Потому что я пишу этот сервер для совместного редактирования в реальном времени и не могу задерживать какие-либо данные. - person cemycc; 25.01.2012
comment
@selbie Я понял, когда клиент подключится, я установлю буфер на объект ptr из epoll, и все данные о доходах будут вставлены туда, а когда клиент перестанет отправлять данные, буфер будет уничтожен, верно? В очередной раз благодарим за помощь. - person cemycc; 25.01.2012
comment
давайте продолжим это обсуждение в чате - person selbie; 25.01.2012