[PATCH] xserver: Fix double free's in config file parser

Dan Nicholson dbn.lists at gmail.com
Thu Jul 14 07:57:57 PDT 2011


On Thu, Jul 14, 2011 at 12:03 AM, Benjamin Herrenschmidt
<benh at kernel.crashing.org> 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 <benh at kernel.crashing.org>
> ---
>
> 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 <dbn.lists at gmail.com>


More information about the xorg-devel mailing list