1
votes

printf() throws a segmentation fault when I pass a null-terminated string for some reason.

Here's a demonstration of my problem in GDB


     λ sudo gdb -q notesearch
    Reading symbols from notesearch...done.
    (gdb) break 53
    Breakpoint 1 at 0x400b32: file notesearch.c, line 53.
    (gdb) run
    Starting program: /home/frosty/hack/chapter_2/code/notesearch 
    [DEBUG] UserID: 0
    [DEBUG] File Descriptor: 3

    Breakpoint 1, print_notes (fd=3, uid=0, searchstring=0x7fff3daf7fc0 "")
        at notesearch.c:53
    53          printf("%s\n", note_buffer);
    (gdb) x/8xb note_buffer
    0x7feb5a997168: 0x68    0x65    0x6c    0x6c    0x6f    0x0a    0x00    0x00
    (gdb) x/s note_buffer
    0x7feb5a997168: "hello\n"
    (gdb) next

    Program received signal SIGSEGV, Segmentation fault.
    _dl_fixup (l=, reloc_arg=)
        at ../elf/dl-runtime.c:148
    148 ../elf/dl-runtime.c: No such file or directory.
    (gdb) 

Here's the source code around the problem


    int print_notes(int fd, int uid, char *searchstring){
        int note_length = find_user_note(fd, uid);

        if (note_length == -1) 
            return 0; // End of file

        char* note_buffer;
        read(fd, note_buffer, note_length);

        note_buffer[note_length] = 0; // null terminator byte

        if(search_note(note_buffer, searchstring)) {
            printf("%s\n", note_buffer);
        }
        return 1;
    }

2
You have char* note_buffer;, but never allocate the memory. So when you use it, you're putting info in memory, but you have no clue where. Either allocate the memory first, or use a char array. - AntonH
@AntonH Although, I am kinda confused on why it worked. If you looked in my gdb output, read() worked fine and read the expected string. Is it because it's in another area in the memory (other than the heap) and printf can't read that certain area of the memory? - Timothy Samson
"I am kinda confused on why it worked" --> Code broke the rules, writing with uninitialized note_buffer. C does not require a sensible outcome - welcome to Undefined Behavior (UB). - chux - Reinstate Monica
@TimothySamson It didn't work. It looked like it worked, but didn't really. Just because you see what you expected, does not mean that it worked. As "chux" said, Undefined Behaviour. - AntonH
Alright. Thanks a lot guys! :-) - Timothy Samson

2 Answers

0
votes

remember that in C, arrays are indexed from 0 through (length of array-1)

This line:

char* note_buffer;

declares an initialized pointer to character. I.E. its value depends on what ever trash is currently in memory at that location. Such an action as accessing where that trash 'points' would be undefined behavior and can lead to a seg fault event.

Strongly suggest, after this line:

char* note_buffer;

inserting the following: (including the checking for a malloc() failure:

note_buffer = malloc( note_length+1 );
if( !note_buffer )
{
    perror( "malloc failed" ); 
    exit( EXIT_FAILURE );
}

// implied else, malloc successful

Note: without the +1 on the call to malloc() the following line:

note_buffer[note_length] = 0;

would be setting a byte that is one past the end of the allocated area. That would be undefined behavior and can lead to a seg fault event

Also, a 0 has the bit pattern 0x000000000000000000000000 and a '\0' has the bit pattern 0x00000000. the implicit conversion feature will save you in this instance, but do not rely on that feature, instead properly code literals. so the line should be:

note_buffer[note_length] = '\0';

Note: perror() outputs the enclosed text string AND the reason the OS thinks the error occurred to stderr, which is where all error messages should be output.

0
votes

Null terminator is indicated by \0 not 0

note_buffer[note_length] = 0;

should be

note_buffer[note_length] = '\0';