Re: [Mesa3d-dev] [PATCH] gallium: fix dri_destroy_option_cache() when optionCache.info is NULL

[Corbin S. <mos...@gm...>]

On Fri, Apr 2, 2010 at 6:44 AM, Yann Droneaud <ya...@dr...> wrote:
> With an Intel 855GM handled by intel_drv, there's a crash with Gallium3D
> enabled DRI driver for Intel i915 (--enable-gallium-intel).
> The Gallium3D driver doesn't support the 855GM as expected by
> intel_drv, it failed to open the screen and give an half
> initialized screen structure to dri_destroy_option_cache():
> optionCache.info is NULL, so it's crashing while trying
> to free array content. This patch at least fix the crash in the function.
>
> Here's some logs of the fixed version:
>
> [ 16274.137] LoaderOpen(/opt/mesa/lib/xorg/modules/drivers/intel_drv.so)
> [ 16274.139] (II) Loading /opt/mesa/lib/xorg/modules/drivers/intel_drv.so
> [ 16274.183] (II) Module intel: vendor="X.Org Foundation"
> [ 16274.183]    compiled for 1.8.0, module version = 2.11.0
> [ 16274.183]    Module class: X.Org Video Driver
> [ 16274.183]    ABI class: X.Org Video Driver, version 7.0
> [ 16274.183] (II) intel: Driver for Intel Integrated Graphics Chipsets: i810,
>        i810-dc100, i810e, i815, i830M, 845G, 852GM/855GM, 865G, 915G,
>        E7221 (i915), 915GM, 945G, 945GM, 945GME, Pineview GM, Pineview G,
>        965G, G35, 965Q, 946GZ, 965GM, 965GME/GLE, G33, Q35, Q33, GM45,
>        4 Series, G45/G43, Q45/Q43, G41, B43, Clarkdale, Arrandale
> [ 16274.382] (II) intel(0): Integrated Graphics Chipset: Intel(R) 855GME
> [ 16274.382] (--) intel(0): Chipset: "852GM/855GM"
> [ 16276.675] (II) intel(0): [DRI2] Setup complete
> [ 16276.675] (II) intel(0): [DRI2]   DRI driver: i915
> debug_get_option: GALLIUM_TRACE = (null)
> debug_get_bool_option: GALLIUM_RBUG = FALSE
> debug_get_bool_option: INTEL_DUMP_CMD = FALSE
> i915_create_screen: unknown pci id 0x3582, cannot create screen
> dri_init_screen_helper: failed to create pipe_screen
> [ 16276.794] (EE) AIGLX error: Calling driver entry point failed
> [ 16276.794] (EE) AIGLX: reverting to software rendering
> [ 16276.794] (II) AIGLX: Screen 0 is not DRI capable
> [ 16276.796] (II) AIGLX: Loaded and initialized /opt/mesa/lib/dri/swrast_dri.so
> [ 16276.796] (II) GLX: Initialized DRISWRAST GL provider for screen 0
>
> Signed-off-by: Yann Droneaud <ya...@dr...>
>
> ---
>  src/gallium/state_trackers/dri/common/dri_screen.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/state_trackers/dri/common/dri_screen.c b/src/gallium/state_trackers/dri/common/dri_screen.c
> index 6fc7487..6e17614 100644
> --- a/src/gallium/state_trackers/dri/common/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/common/dri_screen.c
> @@ -288,12 +288,14 @@ dri_destroy_option_cache(struct dri_screen * screen)
>  {
>    int i;
>
> -   for (i = 0; i < (1 << screen->optionCache.tableSize); ++i) {
> -      FREE(screen->optionCache.info[i].name);
> -      FREE(screen->optionCache.info[i].ranges);
> +   if (screen->optionCache.info) {
> +      for (i = 0; i < (1 << screen->optionCache.tableSize); ++i) {
> +         FREE(screen->optionCache.info[i].name);
> +         FREE(screen->optionCache.info[i].ranges);
> +      }
> +      FREE(screen->optionCache.info);
>    }
>
> -   FREE(screen->optionCache.info);
>    FREE(screen->optionCache.values);
>  }
>
> --
> 1.6.4.4

Applied and pushed, thanks.

In general, this is the correct behavior; screens should fail to be
created if they can't find proper hardware to talk to. Not sure if
that behavior's documented, but it's correct.

~ C.

-- 
When the facts change, I change my mind. What do you do, sir? ~ Keynes

Corbin Simpson
<Mos...@gm...>