On Thu, Jul 14, 2011 at 12:03 AM, Benjamin Herrenschmidt
<[email protected]> wrote:
> Feeding the parser a bad config file, I crashed the server a few times.
>
> It looks like whoever free's "val.str" (yeah, "val" is a global ..  yuck)
> is also responsible for clearing the pointer or something else might try
> to free it again some time later.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>
> ---
>
> diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/Input.c 
> xorg-server-1.10.2.902/hw/xfree86/parser/Input.c
> --- xorg-server-1.10.2.902.orig/hw/xfree86/parser/Input.c       2011-02-25 
> 14:27:14.000000000 +1100
> +++ xorg-server-1.10.2.902/hw/xfree86/parser/Input.c    2011-07-14 
> 16:57:18.912426863 +1000
> @@ -106,6 +106,7 @@
>                         if (strcmp(val.str, "keyboard") == 0) {
>                             ptr->inp_driver = strdup("kbd");
>                             free(val.str);
> +                           val.str = NULL;
>                         }
>                         else
>                            ptr->inp_driver = val.str;
> diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/InputClass.c 
> xorg-server-1.10.2.902/hw/xfree86/parser/InputClass.c
> --- xorg-server-1.10.2.902.orig/hw/xfree86/parser/InputClass.c  2011-02-25 
> 14:27:14.000000000 +1100
> +++ xorg-server-1.10.2.902/hw/xfree86/parser/InputClass.c       2011-07-14 
> 16:57:28.608402699 +1000
> @@ -114,6 +114,7 @@
>             if (strcmp(val.str, "keyboard") == 0) {
>                 ptr->driver = strdup("kbd");
>                 free(val.str);
> +               val.str = NULL;
>             }
>             else
>                 ptr->driver = val.str;
> diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/Screen.c 
> xorg-server-1.10.2.902/hw/xfree86/parser/Screen.c
> --- xorg-server-1.10.2.902.orig/hw/xfree86/parser/Screen.c      2011-02-25 
> 14:27:14.000000000 +1100
> +++ xorg-server-1.10.2.902/hw/xfree86/parser/Screen.c   2011-07-14 
> 16:56:38.492527593 +1000
> @@ -316,6 +316,7 @@
>                                Error (QUOTE_MSG, "SubSection");
>                        {
>                                free(val.str);
> +                               val.str = NULL;
>                                HANDLE_LIST (scrn_display_lst, 
> xf86parseDisplaySubSection,
>                                                         XF86ConfDisplayPtr);
>                        }

I also see a couple instances of "free (val.str)" in parser/Files.c
that don't set it to NULL afterward. Yay for custom parsers! With
those two instances fixed:

Reviewed-by: Dan Nicholson <[email protected]>
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to