C Segmentation Error

Associate
Joined
14 Jun 2010
Posts
737
Evening,

I have been learning C these last few days and trying examples from a book I have, One such program is giving me trouble with segmentation errors. I have found the two lines that are causing it by using GDB and a bit of trial and error.

I have checked the source code distributed with the book and it also has the same segmentation error upon compilation.

Here is the function the issue is occurring in:

Code:
void decode_ip(const u_char *header_start) {
	const struct ip_hdr *ip_header;

	ip_header = (const struct ip_hdr *)header_start;
	printf("\t((  Layer 3 ::: IP Header  ))\n");
	printf("\t( Source: %s\t", inet_ntoa(ip_header->ip_src_addr)); // Problem
	printf("Dest: %s )\n", inet_ntoa(ip_header->ip_dest_addr)); // Problem
	printf("\t( Type: %u\t", (u_int) ip_header->ip_type);
	printf("ID: %hu\tLength: %hu )\n", ntohs(ip_header->ip_id), ntohs(ip_header->ip_len));
}

I have commented the two lines that cause the segmentation error- everything runs fine when they're commented out.

Any help would be great, I am quite new to C. :(
 
Associate
Joined
24 May 2011
Posts
253
Shouldn't those 2 lines have another bracket at the end?

EDIT: Actually the second one's bracket structure just looks wrong, I'm not an expert in C though.

EDIT: My bad, they are within quotes, ignore.
 
Last edited:
Associate
Joined
26 Jan 2009
Posts
848
Location
Amsterdam
You're casting a custom char array type to a custom struct; even without knowing the implementation I can guess this isn't going to work. Are you sure you're not meant to call a function which converts your char array into the struct?

I would guess the bottom three don't error because you're still within the correct memory range and not calling a complex function (ntohs is just byte-order swapping) on them.

Actually the more I think about it, the more I think that might be "legal", but it's still a horrible thing to do. You need to guarantee the order of the bytes in the header char is the same as that in the struct... I guess they'll be the same anyway. But yuck. Check the content of ip_header values anyway, to be sure...
 
Last edited:
Associate
OP
Joined
14 Jun 2010
Posts
737
Thanks for the fast replies.

ip_hdr is just a custom implementation of the standard IP layer header struct. (In /usr/include/netinet/ip.h)

It is just for readability's sake and to get used to writing structs.

In the "Official" ip.h file, 32 bits are required for the Source address and 32 bits for the Destination address. In my struct, they are each defined as an integer as 4 x 8 bits matches the specification. I didn't think I would run into any memory errors.

Reckon it is worth scrapping the custom struct? I have two others in my program that are very similar and work fine.
 
Associate
Joined
26 Jan 2009
Posts
848
Location
Amsterdam
If you're just learning C I'd try to avoid this sort of **** in the beginning, but since you're after an answer...

You have to be careful with the endianness of integers in C. This is the order each bit is stored in memory - most Intel-based architectures are little-endian so the smallest byte is stored first; a bit weird to get to use to. To demonstrate this, look at the header you mentioned for FreeBSD here. They've got a lovely #ifdef in there to change the order of the struct based on the endianness of the given architecture.

Looking it up, the inet_ntoa functions take a struct as a parameter, perhaps it's this struct that's not being decoded correctly?
 
Associate
OP
Joined
14 Jun 2010
Posts
737
Phew, just found the issue with a look at that header file, some GDB and your point about endianness

It appears that the source and destination address are stored in little endian format, but inet_ntoa is expecting to receive it in big endian. Maybe I should write a function to reverse the byte order or replace the use of inet_ntoa- as it is mentioned that is is deprecated?
 
Associate
Joined
26 Jan 2009
Posts
848
Location
Amsterdam
Phew, just found the issue with a look at that header file, some GDB and your point about endianness

It appears that the source and destination address are stored in little endian format, but inet_ntoa is expecting to receive it in big endian. Maybe I should write a function to reverse the byte order or replace the use of inet_ntoa- as it is mentioned that is is deprecated?

I believe the ntohl function (similar to the ntohs you were using; these functions are Net TO Host Short / Long) converts a TCP long (32-bit) value into local endian.

If it's deprecated it just means there'll be a better / more secure way of doing it; you could research what the better way is or just use the old version (I've yet to see deprecated stuff actually be removed from either the Linux or Windows kernels; both OS's are very concerned with backwards compatibility and removing these functions would break that).
 
Associate
OP
Joined
14 Jun 2010
Posts
737
Just a quick update, I decided to scrap the use of the functions as they were causing some issues. Instead, I have printed the values of memory at an offset from the start of the structure header_start. This means that I am offsetting the value by the amount of information in the IP header I don't need. I could get rid of the ip_header struct all together and just access it from header_start.

Thank you very much for your help, I am pleased this now works. :)
 
Top Bottom