csocketsepoll

C - server causing a stack overflow when a second message is sent by a client


I am currently working on a IPv4 server using epoll() to handle the incoming connections. The way communication works between the server and client is the client sends a message starting with a one byte integer indicating the type, followed by randomly generated bytes. The server receives the message, and returns the message to the client with the client's IP address, and port number added to the front of the message between the type and random data.

The client sends a message that starts with a uint8_t "type" and then 32 bytes of random char values with a '\n' signifying the end of the message. The server is meant to send back a message with the same "type" at the start, the IP address of the sender of type uint32_t, the port of the sender of type uint16_t, and then the original message.

Currently, my server successfully communicates when the client sends only one message, but does not when I test the server with 2 or more messages. It just gives me a stack-buffer-overflow. I'm a bit lost as to where the error is occurring, but I believe it is due to the way I am manipulating the data before sending it back to the client as my first iteration of the server with just standard input from a client worked fine. I'm not exactly sure WHY, so it would be helpful if someone could point out what exactly is the issue. Thank you!

Here is my server code:

#include <arpa/inet.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/epoll.h>
#include <sys/socket.h>
#include <unistd.h>

#define LISTEN_BACKLOG 100
#define BUF_SIZE 100

#define handle_error(msg)                                                      \
  do {                                                                         \
    perror(msg);                                                               \
    exit(EXIT_FAILURE);                                                        \
  } while (0)

int main(int argc, char *argv[]) {
  int port = atoi(argv[1]);
  int num_clients = atoi(argv[2]);

  struct sockaddr_in addr, remote_addr;
  int sfd, cfd, epollfd, nfds;
  ssize_t num_read;
  socklen_t addrlen = sizeof(struct sockaddr_in);
  char buf[BUF_SIZE];
  struct epoll_event ev, events[10];

  sfd = socket(AF_INET, SOCK_STREAM, 0);
  if (sfd == -1) {
    handle_error("socket");
  }

  memset(&addr, 0, sizeof(struct sockaddr_in));
  addr.sin_family = AF_INET;
  addr.sin_port = htons(port);
  addr.sin_addr.s_addr = htonl(INADDR_ANY);

  if (bind(sfd, (struct sockaddr *)&addr, sizeof(struct sockaddr_in)) == -1) {
    handle_error("bind");
  }

  if (listen(sfd, LISTEN_BACKLOG) == -1) {
    handle_error("listen");
  }

  epollfd = epoll_create1(0);
  if (epollfd == -1) {
    handle_error("epoll_create1");
  }

  ev.events = EPOLLIN | EPOLLOUT;
  ev.data.fd = sfd;

  if (epoll_ctl(epollfd, EPOLL_CTL_ADD, sfd, &ev) == -1) {
    handle_error("epoll_ctl");
  }

  while (1) {
    nfds = epoll_wait(epollfd, events, num_clients, -1);
    if (nfds == -1) {
      handle_error("epoll_wait");
    }

    for (int i = 0; i < nfds; ++i) {
      if (events[i].data.fd == sfd) {
        memset(&remote_addr, 0, sizeof(struct sockaddr_in));
        cfd = accept(sfd, (struct sockaddr *)&remote_addr, &addrlen);
        if (cfd == -1) {
          handle_error("accept");
        }

        int flags = fcntl(cfd, F_GETFL, 0);
        flags |= O_NONBLOCK;

        if (fcntl(cfd, F_SETFL, flags) == -1) {
          handle_error("fcntl");
        }

        ev.events = EPOLLIN | EPOLLOUT;
        ev.data.fd = cfd;

        if (epoll_ctl(epollfd, EPOLL_CTL_ADD, cfd, &ev) == -1) {
          handle_error("epoll_ctl: conn_sock");
        }
      } else {
        struct sockaddr_in *client_info = (struct sockaddr_in *)&remote_addr;
        uint32_t ip_address = client_info->sin_addr.s_addr;
        uint16_t port = client_info->sin_port;
        uint8_t type = 0;

        while ((num_read =
                    recv(events[i].data.fd, buf, BUF_SIZE, MSG_DONTWAIT)) > 0) {
          char msg[num_read + 7];
          memmove(buf, buf + 1, sizeof(buf));
          memset(msg, 0, num_read + 7);
          memcpy(msg, &type, 1);
          memcpy(msg + 1, &ip_address, sizeof(ip_address));
          memcpy(msg + 5, &port, sizeof(port));
          memcpy(msg + 7, &buf, num_read);

          send(events[i].data.fd, msg, sizeof(msg), 0);
        }
      }
    }
  }
}

And here is the error I'm given when I test.

Test 2 (sending multiple type 0 messages and receiving them back correctly)
Connecting to 172.17.0.2:9000
172.17.0.2:43326
  Sending type 0
  DD91C03F8CCFEFC3B6FAAD696821A602
Sent: DD91C03F8CCFEFC3B6FAAD696821A602
Recv: DD91C03F8CCFEFC3B6FAAD696821A602
172.17.0.2:43326
  Sending type 0
  6AF880ED6A383F505A43AFCF03D07475
Sent: 6AF880ED6A383F505A43AFCF03D07475
=================================================================
==10053==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f9569800149 at pc 0x55fa1c150648 bp 0x7ffcc0e7c650 sp 0x7ffcc0e7bde0
READ of size 35 at 0x7f9569800149 thread T0
    #0 0x55fa1c150647 in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o
    #1 0x55fa1c1520bd in printf (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x540bd) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)
    #2 0x55fa1c205776 in recv_one_msg (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x107776) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)
    #3 0x55fa1c206ea2 in test2 (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x108ea2) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)
    #4 0x55fa1c20687b in main (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x10887b) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)
    #5 0x7f956b5a81c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #6 0x7f956b5a828a in __libc_start_main csu/../csu/libc-start.c:360:3
    #7 0x55fa1c12b444 in _start (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x2d444) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)

Address 0x7f9569800149 is located in stack of thread T0 at offset 73 in frame
    #0 0x55fa1c20540f in recv_one_msg (/home/cmpt201/units/04-long-assignments/a11-TGohar/bin/server-tester.amd64+0x10740f) (BuildId: 5c77824714b6de149f3c8c0a70e0670938e15bba)

  This frame has 1 object(s):
    [32, 73) 'recv_msg' <== Memory access at offset 73 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow asan_interceptors.cpp.o in printf_common(void*, char const*, __va_list_tag*)
Shadow bytes around the buggy address:
  0x7f95697ffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f95697fff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f95697fff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f9569800000: f1 f1 f1 f1 00 00 00 00 03 f3 f3 f3 f3 f3 f3 f3
  0x7f9569800080: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x7f9569800100: f1 f1 f1 f1 00 00 00 00 00[01]f3 f3 f3 f3 f3 f3
  0x7f9569800180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f9569800200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f9569800280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f9569800300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f9569800380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10053==ABORTING

Solution

  • I see some problems with your server code:

    With that said, try something more like the following (untested, but it houd give you some ideas. Adjust as needed...):

    #include <arpa/inet.h>
    #include <fcntl.h>
    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/epoll.h>
    #include <sys/socket.h>
    #include <unistd.h>
    
    #define LISTEN_BACKLOG 100
    #define BUF_SIZE 100
    #define CLI_MSG_SIZE 34
    
    #define fatal_error(msg) \
      do { \
        perror(msg); \
        exit(EXIT_FAILURE); \
      } while (0)
    
    typedef struct {
      char data[BUF_SIZE];
      size_t offset;
      size_t size;
    } buffer;
    
    typedef struct {
      int cfd;
      uint32_t ip_address;
      uint16_t port;
      buffer rbuf;
      buffer wbuf;
    } client_data;
    
    int sfd = -1, epollfd = -1, max_clients = 0, num_clients = 0;
    
    void start_server(int port)
    {
      sfd = socket(AF_INET, SOCK_STREAM, 0);
      if (sfd == -1) {
        fatal_error("socket");
      }
    
      struct sockaddr_in addr;
      memset(&addr, 0, sizeof(addr));
      addr.sin_family = AF_INET;
      addr.sin_port = htons(port);
      addr.sin_addr.s_addr = htonl(INADDR_ANY);
    
      if (bind(sfd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
        fatal_error("bind");
      }
    
      if (listen(sfd, LISTEN_BACKLOG) == -1) {
        fatal_error("listen");
      }
    
      epollfd = epoll_create1(0);
      if (epollfd == -1) {
        fatal_error("epoll_create1");
      }
    
      struct epoll_event ev;
      memset(&ev, 0, sizeof(ev));
      ev.events = EPOLLIN;
      ev.data.fd = sfd;
    
      if (epoll_ctl(epollfd, EPOLL_CTL_ADD, sfd, &ev) == -1) {
        fatal_error("epoll_ctl: add server");
      }
    }
    
    void close_client(client_data *cdata)
    {
      epoll_ctl(epollfd, EPOLL_CTL_DEL, cdata->cfd, NULL); 
      close(cdata->cfd);
      free(cdata);
      --num_clients;
    }
    
    void accept_new_client()
    {
      struct sockaddr_in remote_addr;
      socklen_t addrlen = sizeof(remote_addr);
    
      int cfd = accept(sfd, (struct sockaddr *)&remote_addr, &addrlen);
      if (cfd == -1) {
        perror("accept");
        return;
      }
    
      if (num_clients == max_clients) {
        printf("Max clients already connected!\n");
        close(cfd);
        return;
      }
    
      int flags = fcntl(cfd, F_GETFL, 0);
      flags |= O_NONBLOCK;
    
      if (fcntl(cfd, F_SETFL, flags) == -1) {
        perror("fcntl");
        close(cfd);
        return;
      }
    
      client_data *cdata = (client_data*) malloc(sizeof(client_data));
      if (!cdata) {
        perror("malloc");
        close(cfd);
        return;
      }
    
      cdata->cfd = cfd;
      cdata->ip_address = remoteaddr.sin_addr.s_addr;
      cdata->port = remoteaddr.sin_port;
      cdata->rbuf.offset = 0;
      cdata->rbuf.size = BUF_SIZE;
      cdata->wbuf.offset = cdata->wbuf.size = 0;
    
      struct epoll_event ev;
      memset(&ev, 0, sizeof(ev));
      ev.events = EPOLLIN | EPOLLOUT;
      ev.data.fd = cfd;
      ev.data.ptr = cdata;
    
      if (epoll_ctl(epollfd, EPOLL_CTL_ADD, cfd, &ev) == -1) {
        perror("epoll_ctl: add client");
        close(cfd);
        free(cdata);
        return;
      }
    
      printf("client %d connected\n", cfd);
      ++num_clients;
    }
    
    ssize_t read_buf(int cfd, void *buf, size_t size)
    {
      char *pbuf = (char*) buf;
      ssize_t num_total = 0;
      while (size > 0) {
        ssize_t num_read = recv(cfd, pbuf, size, MSG_DONTWAIT);
        if (num_read < 0) {
          if ((errno != EWOULDBLOCK) && (errno != EAGAIN)) {
            return -1;
          }
          break;
        }
        if (num_read == 0) {
          return 0;
        }
        pbuf += num_read;
        size -= num_read;
        num_total += num_read;
      }
      return num_total;
    }
    
    ssize_t send_buf(int cfd, void *buf, size_t size)
    {
      char *pbuf = (char*) buf;
      ssize_t num_total = 0;
      while (size > 0) {
        ssize_t num_sent = send(cfd, pbuf, size, 0);
        if (num_read < 0) {
          if ((errno != EWOULDBLOCK) && (errno != EAGAIN)) {
            return -1;
          }
          break;
        }
        pbuf += num_sent;
        size -= num_sent;
        num_total += num_sent;
      }
      return num_total;
    }
    
    int read_client_data(client_data *cdata)
    {
      ssize_t num_read = read_buf(cdata->cfd, cdata->rbuf.data + cdata->rbuf.offset, cdata->rbuf.size - cdata->rbuf.offset);
      if (num_read > 0) {
        cdata->rbuf.offset += num_read;
      }
      else {
        if (num_read == 0) {
          printf("client %d disconnected\n", cdata->cfd);
        }
        else {
          printf("client %d recv error: %s\n", cdata->cfd, strerror(errno));
        }
        close_client(cdata);
      }
      return num_read;
    }
    
    int send_client_data(client_data *cdata)
    {
      ssize_t num_sent = send_buf(cdata->cfd, cdata->wbuf.data + cdata->wbuf.offset, cdata->wbuf.size - cdata->wbuf.offset);
      if (num_sent < 0) {
        printf("client %d send error: %s\n", cdata->cfd, strerror(errno));
        close_client(cdata);
        return -1;
      }
      cdata->wbuf.offset += num_sent;
      if (cdata->wbuf.offset == cdata->wbuf.size) {
        cdata->wbuf.offset = cdata->wbuf.size = 0;
      }
      return 0;
    }
    
    void process_client(struct epoll_event *event)
    {
      client_data *cdata = (client_data*) event->data.ptr;
    
      if ((event->events & EPOLLIN) && (cdata->rbuf.offset < cdata->rbuf.size)) {
        if (read_client_data(cdata) <= 0) {
          return;
        }
      }
    
      if ((event->events & EPOLLOUT) && (cdata->wbuf.offset < cdata->wbuf.size)) {
        if (send_client_data(cdata) < 0) {
          return;
        }
      }
    
      if (cdata->wbuf.offset < cdata->wbuf.size) {
        return;
      }
    
      while (cdata->rbuf.offset >= CLI_MSG_SIZE) {
    
        char msg[CLI_MSG_SIZE];
        memcpy(msg, cdata->rbuf.data, CLI_MSG_SIZE);
        memmove(cdata->rbuf.data, cdata->rbuf.data + CLI_MSG_SIZE, cdata->rbuf.offset - CLI_MSG_SIZE);
        cdata->rbuf.offset -= CLI_MSG_SIZE;
    
        cdata->wbuf.data[0] = msg[0];
        memcpy(cdata->wbuf.data + 1, &cdata->ip_address, sizeof(cdata->ip_address));
        memcpy(cdata->wbuf.data + 5, &cdata->port, sizeof(cdata->port));
        memcpy(cdata->wbuf.data + 7, msg + 1, 32);
        cdata->wbuf.data[8] = '\n';
        cdata->wbuf.size = 7 + 32 + 1;
        cdata->wbuf.offset = 0;
    
        if (send_client_data(cdata) < 0) {
          return;
        }
    
        if (cdata->wbuf.offset < cdata->wbuf.size) {
          return;
        }   
      }
    }
    
    int main(int argc, char *argv[])
    {
      if (argc < 3) {
        printf("usage: \"%s\" <port> <max_clients>\n", argv[0]);
        return 0;
      }
    
      int port = atoi(argv[1]);
      max_clients = atoi(argv[2]);
      num_clients = 0;
    
      if ((port < 0) || (port > 65535) || (max_clients <= 0)) {
        printf("invalid input parameter\n");
        return 0;
      }
    
      struct epoll_event *events = (struct epoll_event *) malloc((1 + max_clients) * sizeof(struct epoll_event));
      if (!events) {
        fatal_error("malloc");
      }
    
      start_server(port);
    
      while (1) {
        int nfds = epoll_wait(epollfd, events, 1 + num_clients, -1);
        if (nfds == -1) {
          fatal_error("epoll_wait");
        }
    
        for (int i = 0; i < nfds; ++i) {
          if (events[i].data.fd == sfd) {
            accept_new_client();
          }
          else {
            process_client(&events[i]);
          }
        }
      }
    }