[PATCH xserver] Fix a segfault that occurs if xorg.conf.d is absent:

walter harms wharms at bfs.de
Wed Nov 16 14:12:17 UTC 2016



Am 15.11.2016 22:34, schrieb Ben Crocker:
> In InitOutput, if xf86HandleConfigFile returns CONFIG_NOFILE
> (which it does if no config file or directory is present), the
> autoconfig flag is set, causing xf86AutoConfig to be called
> later on.
> 
> xf86AutoConfig calls xf86OutputClassDriverList via the
> call tree:
> 
> xf86AutoConfig =>
>   listPossibleVideoDrivers =>
>     xf86PlatformMatchDriver =>
>       xf86OutputClassDriverList
> 
> and xf86OutputClassDriverList attempts to traverse a linked list
> that is a member of the XF86ConfigRec struct pointed to by the
> global xf86configptr, which is NULL at this point because the
> XF86ConfigRec struct is only allocated (by xf86readConfigFile)
> AFTER the config file and directory have been successfully
> opened; the CONFIG_NOFILE return from xf86HandleConfigFile
> occurs BEFORE the call to xf86readConfigFile which allocates
> the XF86ConfigRec struct.
> 
> Rx: In read.c (for symmetry with xf86freeConfig, which already
> appears in this file), add a new function xf86allocateConfig
> which tests the value of xf86configptr and, if it's NULL,
> allocates the XF86ConfigRec struct and deposits the pointer
> in xf86configptr.  In xf86Parser.h, add a prototype for the
> new xf86allocateConfig function.
> 
> Back in read.c, #include "xf86Config.h".  In xf86readConfigFile,
> change the open-code call to calloc to a call to the new
> xf86allocateConfig function.
> 
> In xf86AutoConfig.c, add a call to the new xf86allocateConfig function
> to the beginning of xf86AutoConfig to make sure the XF86ConfigRec struct
> is allocated.
> 
> Signed-off-by: Ben Crocker <bcrocker at redhat.com>
> ---
>  hw/xfree86/common/xf86AutoConfig.c |  9 +++++++++
>  hw/xfree86/parser/read.c           | 16 +++++++++++++++-
>  hw/xfree86/parser/xf86Parser.h     |  1 +
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index 9402651..c3e17be 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -149,6 +149,15 @@ xf86AutoConfig(void)
>      char buf[1024];
>      ConfigStatus ret;
>  
> +    /* Make sure config rec is there */
> +    if (xf86allocateConfig() != NULL) {
> +        ret = CONFIG_OK;    /* OK so far */
> +    }
> +    else {
> +        xf86Msg(X_ERROR, "Couldn't allocate Config record.\n");
> +        return FALSE;
> +    }
> +


you can simplify with:

ret = CONFIG_OK;
if ( ! xf86allocateConfig() )
 {
       xf86Msg(X_ERROR, "Couldn't allocate Config record.\n");
        return FALSE;
  }

you do not need to store the return value ?


>      listPossibleVideoDrivers(deviceList, 20);
>  
>      for (p = deviceList; *p; p++) {
> diff --git a/hw/xfree86/parser/read.c b/hw/xfree86/parser/read.c
> index ec038ae..d7e7312 100644
> --- a/hw/xfree86/parser/read.c
> +++ b/hw/xfree86/parser/read.c
> @@ -56,6 +56,7 @@
>  #include <xorg-config.h>
>  #endif
>  
> +#include "xf86Config.h"
>  #include "xf86Parser.h"
>  #include "xf86tokens.h"
>  #include "Configint.h"
> @@ -91,7 +92,7 @@ xf86readConfigFile(void)
>      int token;
>      XF86ConfigPtr ptr = NULL;
>  
> -    if ((ptr = calloc(1, sizeof(XF86ConfigRec))) == NULL) {
> +    if ((ptr = xf86allocateConfig()) == NULL) {
>          return NULL;
>      }
To improve readability:

XF86ConfigPtr ptr;

ptr=xf86allocateConfig();

if (!ptr)
	 return NULL;


just my 2 cents,

re
 wh
>  
> @@ -270,6 +271,19 @@ xf86itemNotSublist(GenericListPtr list_1, GenericListPtr list_2)
>      return (!(last_1 == last_2));
>  }
>  
> +/*
> + * Conditionally allocate config struct, but only allocate it
> + * if it's not already there.  In either event, return the pointer
> + * to the global config struct.
> + */
> +XF86ConfigPtr xf86allocateConfig(void)
> +{
> +    if (!xf86configptr) {
> +        xf86configptr = calloc(1, sizeof(XF86ConfigRec));
> +    }
> +    return xf86configptr;
> +}
> +
>  void
>  xf86freeConfig(XF86ConfigPtr p)
>  {
> diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
> index ff35846..9c4b403 100644
> --- a/hw/xfree86/parser/xf86Parser.h
> +++ b/hw/xfree86/parser/xf86Parser.h
> @@ -449,6 +449,7 @@ extern char *xf86openConfigDirFiles(const char *path, const char *cmdline,
>  extern void xf86setBuiltinConfig(const char *config[]);
>  extern XF86ConfigPtr xf86readConfigFile(void);
>  extern void xf86closeConfigFile(void);
> +extern XF86ConfigPtr xf86allocateConfig(void);
>  extern void xf86freeConfig(XF86ConfigPtr p);
>  extern int xf86writeConfigFile(const char *, XF86ConfigPtr);
>  extern _X_EXPORT XF86ConfDevicePtr xf86findDevice(const char *ident,


More information about the xorg-devel mailing list