JustACodingGrunt
JustACodingGrunt

Reputation: 5

segmentation fault passing pointer to function

So the segmentation fault occurs upon calling the parseIP function from the parseEther method.
I really don't know why I'm getting a segmentation fault, I'm simply taking my packet pointer and adding the length of the ethernet header (14).
The output from printing the addresses gives me hexadecimal addresses which are 14 apart, so I know that the pointer is pointing to the correct location in memory. Despite this I still get a segmentation fault when passing it through?
Am I missing something blindingly obvious?
An example output: https://s1.postimg.org/96owsptn8v/seg_fault.png
edit
I just took the code out of parseIP and dropped it in the if statement that calls it and it works absolutely fine. Really strange. Anyone have any idea?

void parseIP(const unsigned char *packet)
{
    printf("before we cast ip header");
    struct ip *ip_header = (struct ip*) *packet;
    printf("Before we initialise length");
    short length = ip_header -> ip_len;
    printf("Before we initialise headerlength");
    int headerlength = ntohs(ip_header-> ip_hl); //iphdr points to header of network packet

    printf("No segmentation fault here! \n");

    printf("\n Source Ip address \n");
    printf("%s", inet_ntoa(ip_header -> ip_src));
    printf("\n Destination Ip address \n");
    printf("%s", inet_ntoa(ip_header -> ip_dst));

    int data_bytes = length - headerlength;
    const unsigned char *payload = packet + headerlength;
}

void parseEther(const unsigned char *packet, int length)
{

    int i;
    int pcount = 0;
    struct ether_header *eth_header = (struct ether_header *) packet;

    printf("\n\n === PACKET %ld HEADER ===", pcount);
    printf("\nSource MAC: ");

    for (i = 0; i < 6; ++i)
    {
        printf("%02x", eth_header->ether_shost[i]);
        if (i < 5)
        {
            printf(":");
        }
    }

    printf("\nDestination MAC: ");
    for (i = 0; i < 6; ++i)
    {
        printf("%02x", eth_header->ether_dhost[i]);
        if (i < 5)
        {
            printf(":");
        }   
    }

    int data_bytes = length - ETH_HLEN;
    printf(" \n total length - %d -- length of header - %d -- remaining length - %d \n", length, ETH_HLEN, data_bytes);
    printf("Packet address - %p\n", packet);
    const unsigned char *payload = packet + ETH_HLEN;
    printf("Payload address - %p\n", payload);
    //payload pointer now points to start of the network header

    printf("\nType: %d\n", ntohs(eth_header->ether_type));
    if(ntohs(eth_header->ether_type) == 0x0806)
    {
      printf("ARP detected \n");
     // parseARP(payload);
    }
    else if(ntohs(eth_header->ether_type) == 0x0800)
    {
      printf("\nIP detected\n");
      parseIP(payload);
    }
}

Upvotes: 0

Views: 1088

Answers (1)

John Bollinger
John Bollinger

Reputation: 180103

Am I missing something blindingly obvious?

I guess it depends on your standard of obviousness, but certainly this is wrong:

struct ip *ip_header = (struct ip*) *packet;

Function paramter packet is a const unsigned char *, so that statement converts the value of the single unsigned char to which it points to a pointer, which is then used as if it pointed to a struct ip. This almost certainly produces undefined behavior as a violation of the strict aliasing rule, and the chances are essentially nil that it produces the behavior you actually want. Do this instead:

struct ip *ip_header = (struct ip*) packet;

Be aware also, however, that mapping structure types onto raw byte arrays is a tricky business. Getting the correct member order and data types is usually not so hard, but C permits structure layouts to contain arbitrary padding to between (and after) structure members. In particular, although most C implementations provide mechanisms to avoid that, there is no standard mechanism for it.

You have that problem doubly in this case. Not only are your struct ip and struct ether_header subject to it, but so is the system's struct in_addr. Although you can control the declarations of your own structures, it is perilous to assume that you can map a struct in_addr onto your raw packet data, as you are doing to produce arguments for inet_ntoa.

Upvotes: 2

Related Questions