[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