sendto creating a segmentation fault when sending a packet

Question

I'm trying to send a packet through UDP, however I'm getting a seg fault on line 198:

sendto(socketfd, buffer_str, total_len, 0, res->ai_addr, res->ai_addrlen);

And I'm not quite sure what is causing it. I've run the program through GDB, and none of the arguments seem to have anything wrong with them. the file I'm attempting to send is just a simple txt file which has the text "Lorem ipsum dolor sit amet" in it.

The first sendto on line 76 works absolutely fine, as does the recv from on line 78. When I tried replacing the sendto on 198 with the code from like 76, I got the same segfault.

#include <stdio.h> 
#include <stdlib.h> 
#include <unistd.h> 
#include <string.h> 
#include <sys/types.h> 
#include <sys/socket.h> 
#include <arpa/inet.h> 
#include <netinet/in.h>
#include <netdb.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/time.h>

struct packet {
    unsigned int total_frag;
    unsigned int frag_no;
    unsigned int size;
    char* filename;
    char filedata[1000];
};

struct node {
    struct node* next;
    struct packet data;
};

struct list {
    struct node* head;
};

int main(int argc, char *argv[]) {
    printf("ftp <file name>\n");
    
    struct sockaddr_storage their_addr;
    socklen_t addr_size = sizeof(their_addr);
    struct addrinfo hints, *res;
    char* port = "5050";//argv[2];
    struct sockaddr_in *serverAddr;
    serverAddr = malloc(sizeof(struct sockaddr_in));
    memset(&hints, 0, sizeof(hints));
    memset(serverAddr, 0, sizeof(*serverAddr));
    serverAddr->sin_family = AF_INET;
    hints.ai_family = AF_INET;
    hints.ai_socktype = SOCK_DGRAM;
    hints.ai_addr = (struct sockaddr *) serverAddr;

    getaddrinfo("ug168.eecg.utoronto.ca"/*argv[1]*/, port, &hints, &res);
    int socketfd = socket(PF_INET, SOCK_DGRAM, 0);
    char fileName[100];
    scanf("%s", fileName);
    int fd = open(fileName, O_RDWR);
    if(fd == -1) {
        return 0;
    }

    struct timeval pre_time, post_time;
    gettimeofday(&pre_time, NULL);
    sendto(socketfd, (char*)"ftp", 3, 0, res->ai_addr, res->ai_addrlen);
    char buf[1000];
    int length = recvfrom(socketfd, buf, 1000, 0, (struct sockaddr *)&serverAddr, &addr_size);
    gettimeofday(&post_time, NULL);

    buf[length] = '\0';
    if(strcmp("yes", buf) == 0) {
        printf("A file transfer can start\n");
    }
    printf("seconds : %ld\nmicro seconds : %ld\n", post_time.tv_sec - pre_time.tv_sec, post_time.tv_usec - pre_time.tv_usec);

    FILE *file;
    file = fopen(fileName, "rb"); //rb = read as binary
    fseek(file, 0, SEEK_END);
    int fileLength = ftell(file);
    fseek(file, 0, SEEK_SET);

    char buffer[fileLength];
    fread(buffer, fileLength, 1, file);
    fclose(file);
    int frag_total = fileLength / 1000;
    if(fileLength % 1000 != 0) {
        frag_total++;
    }

    struct list packet_list;
    packet_list.head = malloc(sizeof(struct node));
    struct node* curr = packet_list.head;
    for(int i = 0; i < frag_total; i++) {
        curr->data.total_frag = frag_total;
        curr->data.frag_no = i + 1;
        if(i == frag_total - 1) {
            curr->data.size = fileLength % 1000;
        } else {
            curr->data.size = 1000;
        }
        curr->data.filename = malloc(sizeof(char) * (strlen(fileName) + 1));
        strcpy(curr->data.filename, fileName);
        if(i == frag_total - 1) {
            //copy data equal to remainder
            for(int j = 0; j < fileLength % 1000; j++) {
                curr->data.filedata[j] = buffer[1000 * i + j];
            }
        } else {
            //copy 1000 bytes
            for(int j = 0; j < 1000; j++) {
                curr->data.filedata[j] = buffer[1000 * i + j];
            }
        }
        if(i < frag_total - 1) {
            curr->next = malloc(sizeof(struct node));
            curr = curr->next;
        }
    }

    curr = packet_list.head;
    while(curr != NULL) {
        int j = curr->data.total_frag;
        int frag_total_len = 0;
        while(j != 0) {
            j /= 10;
            frag_total_len++;
        }
        j = curr->data.frag_no;
        int frag_no_len = 0;
        while(j != 0) {
            j /= 10;
            frag_no_len++;
        }
        j = curr->data.size;
        int size_len = 0;
        while(j != 0) {
            j /= 10;
            size_len++;
        }
        int name_len = strlen(curr->data.filename);

        int total_len = frag_total_len + frag_no_len + size_len + name_len + 4 + curr->data.size; //4 bc 4 colons

        char buffer_str[total_len];
        j = 0;

        char strbuf[total_len];

        sprintf(strbuf, "%d", curr->data.total_frag);
        for(int k = j; k < j + frag_total_len; k++) {
           buffer_str[k] = strbuf[k-j];
        }
        j += frag_total_len;
        buffer_str[j] = ':';
        j++;

        sprintf(strbuf, "%d", curr->data.frag_no);
        for(int k = j; k < j + frag_no_len; k++) {
           buffer_str[k] = strbuf[k-j];
        }
        j += frag_no_len;
        buffer_str[j] = ':';
        j++;

        sprintf(strbuf, "%d", curr->data.size);
        for(int k = j; k < j + size_len; k++) {
           buffer_str[k] = strbuf[k-j];
        }
        j += size_len;
        buffer_str[j] = ':';
        j++;

        for(int k = 0 ; k < name_len; k++, j++) {
            buffer_str[j] = curr->data.filename[k];
        }

        buffer_str[j] = ':';
        j++;
        for(int k = 0; k < curr->data.size; k++, j++) {
            buffer_str[j] = curr->data.filedata[k];
        }

        sendto(socketfd, buffer_str, total_len, 0, res->ai_addr, res->ai_addrlen);
        curr = curr->next;
    }
    return 0;
}

I'm not really sure what could be causing this

send to doesn't cause segmentation faults (it returns an `EFAULT` error instead because that's what system calls do). So if you're getting a segfault on that line then it's because of *your* code, not `sendto`. The only things which could cause a segfault on that line are `res->ai_addr` and `res->ai_addrlen`. Are you sure that `res` points to something valid? — user253751, Oct 21 '20 at 11:29
You're right - I just checked and res points to 0x0. I'm still confused as to how to fix this though, since everything works fine in the first sendto (it sends to the right port and address, and the server program acknowledges it) — Parker Johnston, Oct 21 '20 at 11:45

score 0 · Answer 1 · answered Oct 21 '20 at 11:46

0

The original posted code omitted error checking for getaddrinfo(), which is returning an error code and left res unchanged; that led to an uninitialized pointer being dereferenced when calling sendto(); example revision:

int rv = getaddrinfo("ug168.eecg.utoronto.ca"/*argv[1]*/, port, &hints, &res);
if (rv) {
    fprintf(stderr, "getaddrinfo error, rv %d\n", rv);
    return (1);
}

answered Oct 21 '20 at 11:46

Milag

1,793
2
9
8

I added in the error checking like you said, however I'm not getting an error from getaddrinfo. rv = 0 when I run it – Parker Johnston Oct 21 '20 at 11:50
Error reproduced on a Debian system. Try compiling with all warnings enabled and fix anything reported. If `getaddrinfo()` actually succeeds on your system, right after confirm whether `res` is valid, then check struct members. – Milag Oct 21 '20 at 12:05
I compiled with -Wall but no warnings. getaddrinfo() is returning 0, however res is pointing to 0x0. I'm confused as to why the first sendto() works fine despite this, however the second fails – Parker Johnston Oct 21 '20 at 12:23
If `res` was actually NULL, there should be a crash when attempting to dereference any struct member. Once `getaddrinfo()` succeeds, I'm seeing a non-NULL `res`. Original code also omits checking the return value from each `sendto()`, I'm seeing -1 and errno=1. – Milag Oct 21 '20 at 12:48
the first sendto returns 3 for me, and res is non null after it. I suppose what's next is to figure out when it stops being that? – Parker Johnston Oct 21 '20 at 12:58
Possibly data corruption; check `res` and struct members at various places before the second `sendto()`. – Milag Oct 21 '20 at 13:04
1

ok, going through step by step, it's the recvfrom line which sets res to 0x0. Any idea as to why that might be? – Parker Johnston Oct 21 '20 at 13:07

Armali · Accepted Answer · 2020-10-22T06:54:40.493

0

The local variable res is overwritten by the call

int length = recvfrom(socketfd, buf, 1000, 0, (struct sockaddr *)&serverAddr, &addr_size);

this is because the address of the pointer serverAddr is passed instead of the value of the pointer:

int length = recvfrom(socketfd, buf, 1000, 0, (struct sockaddr *)serverAddr, &addr_size);

You'll probably see that the program now segfaults for another reason: The buffer handling fails for short files with less than two fragments, because curr->next is uninitialized.

edited Oct 22 '20 at 06:54

answered Oct 21 '20 at 13:13

Armali

18,255
14
57
171

1

interesting, thanks for the info, I changed it to be: socklen_t addr_size = sizeof(struct sockaddr_in); but res is still being set to 0x0 by recvfrom – Parker Johnston Oct 21 '20 at 13:20
My mistake - the wrong `addr_size` wasn't the overwrite reason. I corrected the answer. – Armali Oct 22 '20 at 06:54

sendto creating a segmentation fault when sending a packet

2 Answers2