[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