FreeBSD's malloc problem ?

From: Anton Alin-Adrian (aanton_at_reversedhell.net)
Date: 04/24/04

  • Next message: Nicolas Rachinsky: "Re: FreeBSD's malloc problem ?"
    Date: Sat, 24 Apr 2004 21:27:23 +0300
    To: freebsd-hackers@freebsd.org
    
    
    

    Hello hackers !

            Before anything else, I must mention I am running FreeBSD 4.9-RELEASE, and
    that is the OS where I encounter the problem.

            I was developing some functions fooling around for parsing some bigger
    opensource project's config file and look what happened:

            There are many functions inside, only adjust_spaces(char *s) is the
    interesting one!

            The problem does *not* exist on Linux systems.

            Now, before getting into anything fancy, I want to mention that I am
    horribly ill and this posting may be terribly lame and the only one who has
    faults may be me with my code.

            BUT, if that is not the case, then this might be a bug in FreeBSD malloc's
    mechanism.

            Ok here it goes. The problem is at line 68 in the attached file, in
    function adjust_spaces(char *s):

    ---code snippet---
    char *adjust_spaces(char *s)
    {
            char *tmp;
            int i=0,j=0,count=0;
            int is_leading=0,last_space=0;

            tmp = (char *) malloc(strlen(s)); // line 68
    ........
    ........
    }
    ---code snippet---
            
            If the function is called exactly as in the code, with the buffer setup to:

    ---code snippet---
    int main (void)
    {
            char buf[]="allowed_ip abcde#"; // like that, other combination
                                            // have the same result
            char *s;
            
            if (is_definition(buf)) fprintf(stderr,"DEFINITION found\n");
            else fprintf(stderr,"Nothing found\n");
            split_line(buf);

            
    }
    ---code snippet---

    then at line 90 and possibly 79 too:
                                    tmp[j]=s[i]; // line 90

    the first character of the string pointed to by *tmp (which was malloced
    before, at line 68) will *overwrite* the null terminating '\0' character of
    the string pointed to by *s (and given as input to the function).

    This later results in an infinite counter loop and an integer overflow, but
    the integer overflow will not become reality because the program will try
    to read/write from invalid memory locations and will crash receiving a
    segmentation fault.

    ALL THIS never happens, if at line 68 I use this code:

    tmp = (char *) malloc(strlen(s)+1); // new line 68 adds +1

    In the attached file t.c, you will find the corrected version of line 68,
    just like above.

    I do not understand the malloc mechanism (sadly, i will appreciate any
    links for reading as much as a gift), and this is why I do not see (I may
    be just way too dizzy) the logic of why adding +1 fixes the problem.

    Please note that adding +1 is not necessary on Linux, I just tested it, but
    on FreeBSD without adding +1, in rare situations like the one I presented
    it will crash exactly like I described. Linux works fine on all situations,
    including this one which crashes on FreeBSD (the very same code).

    If I am just talking crap please forgive me.

    I really want to understand what's going on, is it me who is doing it the
    wrong way?

    The code is just fooling around to get things working properly, it is not
    beautified and optimized (yet).

    PS: another possibility is that I have a 1 byte overflow inside the string
    pointed to by *s, even before calling the adjust_spaces(char *s) function;
    but I simply cannot focus anymore,so please help me a bit.

    Thanks for your time.

    Sincerely Yours,

    -- 
    Alin-Adrian Anton
    Reversed Hell Networks
    GPG keyID 0x1E2FFF2E (2963 0C11 1AF1 96F6 0030 6EE9 D323 639D 1E2F FF2E)
    gpg --keyserver pgp.mit.edu --recv-keys 1E2FFF2E
    
    

    #include <stdio.h>
    #include <ctype.h>

    char *remove_comments(char *s)
    {
            char *tmp;
            int i=0;

            tmp=(char*) malloc (strlen(s));
            while ((s[i]!='\0') && (s[i]!='#'))
            {
                    tmp[i]=s[i];
                    i++;
            }
            tmp[i]='\0';
            return tmp;
    }

    char *remove_unallowed(char *s)
    {
            char *tmp;
            int i=0,j=0;

            tmp=(char*) malloc(strlen(s));

            while (s[i]!='\0')
            {
                    if ( (isalnum(s[i])) || (isspace(s[i])) || (s[i]==',') || (s[i]=='_') || (s[i]=='.') )
                                    {
                                            tmp[j]=s[i];
                                            j++;
                                    }
                    i++;
            }
            tmp[j]='\0';
            return tmp;

    }

    char *remove_unallowed_section(char *s)
    {
            char *tmp;
            int i=0,j=0;

            tmp=(char*) malloc(strlen(s));

            while (s[i]!='\0')
            {
                    if ( (isalnum(s[i])) || (s[i]=='_') )
                                    {
                                            tmp[j]=s[i];
                                            j++;
                                    }
                    i++;
            }
            tmp[j]='\0';
            return tmp;

    }

    char *adjust_spaces(char *s)
    {
            char *tmp;
            int i=0,j=0,count=0;
            int is_leading=0,last_space=0;

            tmp = (char *) malloc(strlen(s)+1);
            //fprintf(stderr,"Converting |%s| length=%d\n",s,strlen(s));
            while (s[i]!='\0')
            {
            //fprintf(stderr,"i=%d j=%d len=%d s[len]=%d\n",i,j,strlen(s),s[strlen(s)]);
            //fprintf(stderr,"tmp= |%s|\n",tmp);
            //sleep(1);
                    if (isspace(s[i])) {
                                                    //fprintf(stderr,"s1=|%s|\n",s);
                                                    if (is_leading==0) is_leading=1;
                                                    if ((!count) && (is_leading!=1)) {
                                                                             tmp[j]=s[i];
                                                                            j++;
                                                                      }
                                                    if (is_leading==1) is_leading=-1;

                                                    count++;
                                               }
                    else {
                                    //fprintf(stderr,"s2=|%s|\n",s);
                                    is_leading=-1;
                                    count=0;
                                    tmp[j]=s[i];
                                    j++;
                            }
                    i++;
            } // end while
            if (isspace(tmp[j-1])) --j;
            tmp[j]='\0';

            return tmp;
    }

    int is_section(char *s)
    {
            int i;
            int br_left=0,br_right=0;
            char *tmp;

            s=remove_comments(s);

            for (i=0;i<strlen(s);i++) {
                                                            if (s[i]=='[') br_left++;
                                                            if (s[i]==']') br_right++;
                                                    }

            if ((br_left!=1) || (br_right!=1))
                    if ( (br_left!=0)|| (br_right!=0))
            {
                                    fprintf(stderr,"Syntax error in config file: \n%s\n\n",s);
                                    exit(1);
            }
            tmp=remove_unallowed_section(s);
            if (strlen(tmp)==0) {
                            fprintf(stderr,"NULL section name in line:\n%s\n\n",s);
                            exit(1);
            }
            return br_left;
    }

    int is_definition(char *s)
    {
            int i;
            int count=0,virgo=0;

            s=remove_comments(s);
            s=remove_unallowed(s);
            s=adjust_spaces(s);

            for (i=0;i<strlen(s);i++) {
                                                            if (isspace(s[i])) count++;
                                                            if ((i>0) && (i<strlen(s)))
                                                                    if ((s[i]==',') && (s[i-1]==',')) virgo++;
                                                    }
            if (s[i-1]==',') virgo++;
            
            if ((!count) || (virgo)){
                            fprintf(stderr,"Syntax error in definition on line:\n%s\n\n",s);
                            exit(1);
            }
            
            return strlen(s);
    }

    int split_line(char *s)
    {
            int i=0;
            char *tmp,*ptr;

            s=remove_comments(s);

            s=remove_unallowed(s);

            s=adjust_spaces(s);

            tmp= (char *) malloc(strlen(s));

            ptr=(char *) strchr(s,' ');
            *ptr++='\0';

            fprintf(stderr,"name = |%s|\n",s);

            while (*ptr!='\0') tmp[i++]=*ptr++;
       

            fprintf(stderr,"value = |%s|\n",tmp);

            return;
            
    }

    int main (void)
    {
            char buf[]="allowed_ip abcde#";
            char *s;
            
            if (is_definition(buf)) fprintf(stderr,"DEFINITION found\n");
            else fprintf(stderr,"Nothing found\n");
            split_line(buf);

            
    }

    
    

    _______________________________________________
    freebsd-hackers@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
    To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"


  • Next message: Nicolas Rachinsky: "Re: FreeBSD's malloc problem ?"

    Relevant Pages

    • [PATCH] toshiba-acpi: Fix integer overrun that causes heap trashing
      ... Avoid heap trashing triggered by an integer overflow of the ... int main ... char *buf; ...
      (Linux-Kernel)
    • Re: how to use recursive algorithm to determine all of the arrangements?
      ... The simplest way is to check for integer overflow is to do the multiplication in floating point precision, and check if the result is larger than what would fit in whatever primitive type you're trying to store the value in. ... In the case of int, you could use long instead of double. ... so that if there is no overflow temp does ... public static void main{ ...
      (comp.lang.java.programmer)
    • Re: Overflow errors?
      ... by integer overflow i mean the one that the C99 standard states would ... This is an expression whose type is int, ... is a signed type, paragraph 3 applies. ... indistinguishable from having undefined behavior. ...
      (comp.lang.c)
    • [PATCH] thinkpad-acpi: Avoid heap buffer overrun
      ... Avoid a heap buffer overrun triggered by an integer overflow of the userspace ... int main(int argc, char **argv) ...
      (Linux-Kernel)