Segfault when casting char array to struct

Question

I'm trying to cast a char array to a struct and receiving a segfault when I compile with g++ 4.7 and run but not when I compile with clang 3.3 and run the program.

This is the message struct description:

typedef struct {
 char code[4];
 char nickname[24];
 struct in_addr addr;
 int port;
 int seqNum;
 char body[MESSAGE_MAX_LENGTH - (sizeof(char) * 28) - (sizeof(int) * 2) - sizeof(struct in_addr)];
} Message;

This is the line which is causing the segfault:

Message *recvMsg = (Message *)packet.buffer;

The packet object is of the following type:

class UDPPacket {
     public:
         static const size_t maxBufferSize = MESSAGE_MAX_LENGTH;
         uint8_t buffer[maxBufferSize];
         size_t length;
         struct in_addr addr;
         int port;

         UDPPacket();
         UDPPacket(struct in_addr addr, int port);
         std::string getRemoteAddrString();
         std::string getDataAsString();
 };

This is the backtrace produced by gdb:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff67f5700 (LWP 22753)]
0x00007ffff79787f7 in ?? () from /usr/lib64/libstdc++.so.6
(gdb) backtrace
#0  0x00007ffff79787f7 in ?? () from /usr/lib64/libstdc++.so.6
#1  0x00007ffff7978850 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() () from /usr/lib64/libstdc++.so.6
#2  0x0000000000411cb7 in SendMessage () at /home1/user123/ugh/final-project/main.cpp:189
#3  0x00000000004172df in std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>) (
    this=0x620310) at /usr/include/c++/4.7/functional:1598
#4  0x000000000041722f in std::_Bind_simple<void (*())()>::operator()() (this=0x620310)
    at /usr/include/c++/4.7/functional:1586
#5  0x00000000004171c8 in std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run() (
    this=0x6202f8) at /usr/include/c++/4.7/thread:115
#6  0x00007ffff796f340 in ?? () from /usr/lib64/libstdc++.so.6
#7  0x00007ffff7bc6e0f in start_thread () from /lib64/libpthread.so.0
#8  0x00007ffff70e044d in clone () from /lib64/libc.so.6

I'm not sure why this is segfaulting when I compile with g++ but not with clang?

EDIT: This is what SendMessage() looks like:

 56 void SendMessage() {
 57     while(true){
 58         std::cout << "> ";
 59         std::string input;
 60         std::getline(std::cin, input);
 61
 62         if(input.length() > 140){
 63             std::cout << "[Message is too long and was not sent]" << std::endl;
 64             continue;
 65         }
 66
 67         Message sendMsg;
 68         ::strncpy(sendMsg.code, "SEND", 4);
 69         ::strncpy(sendMsg.body, input.c_str(), MESSAGE_MAX_LENGTH - 28);
 70
 71         if(isLeader){
 72             // Leader Code
 73             ::strncpy(sendMsg.nickname, lState->name.c_str(), lState->name.length());
 74             sendMsg.addr = lState->ownAddr.addr;
 75             sendMsg.port = lState->ownAddr.port;
 76
 77             sendMsg.seqNum = lState->seqNum;
 78             broadcast(lState->allClients, &sendMsg);
*lines 79 to 187 commented out*
187             lState->seqNum += 1;
188          }
189     } //end of while loop
190 }

You should show us the method `SendMessage()`. From the trace, we see (#2), that this is the last method written by you which is involved. It doesn't look like an error in the assignment of `recvMsg` (actually nothing can go wrong there), but in the use of `std::string`. — stefan, Apr 30 '14 at 05:41
The backtrace doesn't point to the line you say causes the segfault. — n. m. could be an AI, Apr 30 '14 at 05:42
There is something wrong when it is trying to destruct a `string` — anonymous, Apr 30 '14 at 05:42
Your structure isn't packed and without knowing what the size of "struct in_addr adde;", you can't be sure that padding bytes won't be added in the middle of the structure. Your calculation for the size of Message.body is probably wrong. This will most likely result in a reference passed the end of valid memory after the cast that you are pointing out. — dboals, Apr 30 '14 at 05:45
Additionally, the alignment may be wrong. In C++11, you can use `std::aligned_storage` for that. — Paul J. Lucas, Apr 30 '14 at 05:47
So the backtrace says in #2 the issue was on line 189 of the main.cpp file which is part of the SendMessage() function. Line 189 is the end bracket of the while loop in SendMessage() which doesn't make any sense. When I stepped through line by line in gdb, I found that the segfault occurs at the line mentioned above (where I try to cast the packet to Message). I'll also update the question with what SendMessage looks like. — user1710707, Apr 30 '14 at 05:56
@user1710707 Line 189 being the end bracket makes perfect sense: It's the line where the destructors are called. — stefan, Apr 30 '14 at 05:59
@stefan updated the code to show what SendMessage() currently looks like — user1710707, Apr 30 '14 at 06:02

score 1 · Answer 1 · edited May 23 '17 at 10:26

1

There's no guarantee the actual size of your struct is going to be the sum of the nominal sizes of each member of the struct.

The actual size could be LARGER due to padding or other compiler- and/or platform-specific issues.

Here are more details:

Why isn't sizeof for a struct equal to the sum of sizeof of each member?

edited May 23 '17 at 10:26

Community

1
1

answered Apr 30 '14 at 05:42

FoggyDay

11,962
4
34
48

score 0 · Answer 2 · answered Apr 30 '14 at 07:21

The setup

Ignoring the fact that sizeof(Message) may not be what you think it is (though it may be!), you're putting Message sendMsg on the stack.

Assuming standard type widths, sendMsg.body has space for [MESSAGE_MAX_LENGTH - 36 - sizeof(in_addr)] chars.

The problem

Then you're coming along with ::strncpy(sendMsg.body, input.c_str(), MESSAGE_MAX_LENGTH - 28); which potentially writes a lot more characters than there's space for! This can keep writing past the end of sendMsg and whatever happens to be next to it on the stack, which can vary between compilers. Maybe your clang compilation & input are getting lucky (though still incorrect).

Once you've overwritten innocent stack variables, Bad Things(tm) like segfaults can happen at any time.

The prevention

Assuming you don't need your char arrays null-terminated (which you might), make sure you never copy more data than you have room for.

::strncpy(sendMsg.code, "SEND", sizeof(sendMsg.code));
::strncpy(sendMsg.body, input.c_str(), sizeof(sendMsg.body));
::strncpy(sendMsg.nickname, lState->name.c_str(), sizeof(sendMsg.nickname));

Bear in mind this may still be discarding information and not null-terminating, but at least it won't stomp your stack.

Segfault when casting char array to struct

2 Answers2

The setup

The problem

The prevention