C concurrent UDP socket , weird segmentation fault

Question

I'm either very tired and not noticing something simple , or this is completely screwing with me. I'm getting a segmentation fault ( core dumped ) and I've managed to pinpoint it to the sendto() in the worker function. (in the server)

Server code:

    //UDPServer.c

/* 
 *  gcc -o server UDPServer.c
 *  ./server <port> <buffersize>
 */
#include <arpa/inet.h>
#include <netinet/in.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <unistd.h>
#include <stdlib.h> 
#include <string.h>


void err(char *str)
{
    perror(str);
    exit(1);
}

int sock;

 typedef struct
        {
                struct sockaddr_in client;
                int buffsize;
                char *msg;
        } data;

void *worker (void* asd)
{
    int len;
    FILE *fp;
    data d;
    d = *(data*) asd;
    char buff[d.buffsize];
    printf("Received packet from %s:%d\nData:%sSize:%d\n",
               inet_ntoa(d.client.sin_addr), ntohs(d.client.sin_port)
               ,d.msg,d.buffsize);


    char * fn;
    memcpy (fn,d.msg,strlen(d.msg)-1);
    fp = fopen(fn,"rb");
    int bytes;
    len = sizeof(d.client);
    printf ("%d\n",len);

   while (bytes=fread(buff,sizeof(char),d.buffsize,fp))
        {
            printf ("Server sent %d bytes.\n",bytes);
              -> this if right here. this causes the core dump when attempting to send
              if(sendto(sock , &buff , sizeof(buff),0,(struct sockaddr *)&d.client,len)<0)
                err("Error sending.");

        }
     fclose(fp);


}


int main(int argc, char** argv)
{
    struct sockaddr_in server, client;
    int port, i;
    socklen_t slen=sizeof(client);

    if(argc != 3)
    {
      printf("Usage: <Port> <Bytes>\n");
      exit(0);
    }
    else 
        sscanf(argv[1],"%d",&port);

    int buffsize = atoi(argv[2]);

    char buff[buffsize];

    if ((sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP))==-1)
      err("socket");
    else 
      printf("Server : Socket() successful\n");

    bzero(&server, sizeof(server));
    server.sin_family = AF_INET;
    server.sin_port = htons(port);
    server.sin_addr.s_addr = htonl(INADDR_ANY);

    if (bind(sock, (struct sockaddr* ) &server, sizeof(server))==-1)
      err("bind");
    else
      printf("Server : bind() successful\n");

    while(1) 
    {
        memset(&buff,0,sizeof(buff));
        if (recvfrom(sock, &buff, sizeof(buff), 0, (struct sockaddr*)&client, &slen)==-1)
            err("recvfrom()");
        data d;
        d.client = client;
        d.buffsize = buffsize;
        d.msg = buff;

        pthread_t t;
        pthread_create(&t,NULL,worker,&d);
        pthread_join(t,NULL);
    }

    return 0;
}

I don't think the client is relevant here since it's only job is to send the filename. The read works btw , I've tested.

Anyway , I'm just trying to send the content of the file for the moment.I've been trying to figure this out for the past hour and for the life of me I can't find out what's it's problem. The segmentation fault makes no sense to me.

Any suggestions are greatly appreciated.

Is buffsize very large (e.g. more than a couple of kilobytes)? If so, you might be using up more stack space than was allocated to your worker thread. The traditional solution would be to allocate the buffer on the heap instead. — Jeremy Friesner, Mar 13 '14 at 00:49

score 1 · Answer 1 · edited May 23 '17 at 12:21

I'd be nervous about the sizeof(buff) in the sendto. buff's size is fixed at runtime based on the argument. But sizeof is a compile-time operation. (Or at least it was back in the good old days - I'm not sure about C99) Oh, nevermind - I see that has changed

Still, why not use d.buffsize there instead? Or maybe bytes, since you might not have filled the buffer.

Although @21Zoo is wrong about dynamic arrays in C99, I think he found the root problem

char * fn;
memcpy (fn,d.msg,strlen(d.msg)-1);

fn has no memory allocated to copy into, so you are writing to a random point in memory. Something in the sendto is probably stumbling over that memory which now contains garbage.

You either need to malloc(strlen(d.msg)+1) or use strdup instead.

There are a lot of problems here. `char buff[d.buffsize];` can't do that, array size needs to be known at compile time, use malloc. `char * fn;` followed by `memcpy (fn,d.msg...) ` is no good either, fn hasn't been initialized yet. Again, read up on malloc, etc — Oliver, Mar 12 '14 at 21:42

C concurrent UDP socket , weird segmentation fault

1 Answers1

Linked