Mesa (master): loader: Avoid use-after-free / use of uninitialized local variables

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: eb03141f52fe9f620d941006077ce2cefff84e65
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=eb03141f52fe9f620d941006077ce2cefff84e65

Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Thu Sep 26 11:02:46 2019 +0200

loader: Avoid use-after-free / use of uninitialized local variables

Per the valgrind output below, we were returning the pointer to freed
memory if none of the later conditional pointer assignments were
executed. This caused dEQP CI jobs to crash on certain runners,
presumably due to a double-free down the line.

Also, we were skipping to the out: label before the vendor_id & chip_id
variables used by it were initialized, resulting in broken
LIBGL_DEBUG=verbose output such as

libGL: pci id for fd 4: 51108f00:51108f00, driver radeonsi

Fixes: 5a545e355b23 "loader: always map the "amdgpu" kernel driver name to radeonsi (v2)"

==403== Invalid read of size 1
==403==    at 0x4AFD576: surfaceless_probe_device (platform_surfaceless.c:316)
==403==    by 0x4AFD915: dri2_initialize_surfaceless (platform_surfaceless.c:391)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:984)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:958)
==403==    by 0x4AF1EEC: _eglMatchAndInitialize (egldriver.c:75)
==403==    by 0x4AF1F3B: _eglMatchDriver (egldriver.c:96)
==403==    by 0x4AE9367: eglInitialize (eglapi.c:617)
==403==    by 0x1D99C9: tcu::surfaceless::EglRenderContext::EglRenderContext(glu::RenderConfig const&, tcu::CommandLine const&) [clone .constprop.57] (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x1DABB0: tcu::surfaceless::ContextFactory::createContext(glu::RenderConfig const&, tcu::CommandLine const&, glu::RenderContext const*) const (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x53EBD1: glu::createRenderContext(tcu::Platform&, tcu::CommandLine const&, glu::RenderConfig const&, glu::RenderContext const*) (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x53EFE9: glu::createDefaultRenderContext(tcu::Platform&, tcu::CommandLine const&, glu::ApiType) (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x1DE07A: deqp::gles2::Context::Context(tcu::TestContext&) (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x1DB5EF: deqp::gles2::TestPackage::init() (in /deqp/modules/gles2/deqp-gles2)
==403==  Address 0x56bd340 is 0 bytes inside a block of size 4 free'd
==403==    at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==403==    by 0x4B01767: loader_get_driver_for_fd (loader.c:464)
==403==    by 0x4AFD553: surfaceless_probe_device (platform_surfaceless.c:308)
==403==    by 0x4AFD915: dri2_initialize_surfaceless (platform_surfaceless.c:391)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:984)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:958)
==403==    by 0x4AF1EEC: _eglMatchAndInitialize (egldriver.c:75)
==403==    by 0x4AF1F3B: _eglMatchDriver (egldriver.c:96)
==403==    by 0x4AE9367: eglInitialize (eglapi.c:617)
==403==    by 0x1D99C9: tcu::surfaceless::EglRenderContext::EglRenderContext(glu::RenderConfig const&, tcu::CommandLine const&) [clone .constprop.57] (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x1DABB0: tcu::surfaceless::ContextFactory::createContext(glu::RenderConfig const&, tcu::CommandLine const&, glu::RenderContext const*) const (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x53EBD1: glu::createRenderContext(tcu::Platform&, tcu::CommandLine const&, glu::RenderConfig const&, glu::RenderContext const*) (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x53EFE9: glu::createDefaultRenderContext(tcu::Platform&, tcu::CommandLine const&, glu::ApiType) (in /deqp/modules/gles2/deqp-gles2)
==403==  Block was alloc'd at
==403==    at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==403==    by 0x4EE5E09: strndup (strndup.c:43)
==403==    by 0x4B010B1: loader_get_kernel_driver_name (loader.c:101)
==403==    by 0x4B016AF: loader_get_driver_for_fd (loader.c:462)
==403==    by 0x4AFD553: surfaceless_probe_device (platform_surfaceless.c:308)
==403==    by 0x4AFD915: dri2_initialize_surfaceless (platform_surfaceless.c:391)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:984)
==403==    by 0x4AF5EEA: dri2_initialize (egl_dri2.c:958)
==403==    by 0x4AF1EEC: _eglMatchAndInitialize (egldriver.c:75)
==403==    by 0x4AF1F3B: _eglMatchDriver (egldriver.c:96)
==403==    by 0x4AE9367: eglInitialize (eglapi.c:617)
==403==    by 0x1D99C9: tcu::surfaceless::EglRenderContext::EglRenderContext(glu::RenderConfig const&, tcu::CommandLine const&) [clone .constprop.57] (in /deqp/modules/gles2/deqp-gles2)
==403==    by 0x1DABB0: tcu::surfaceless::ContextFactory::createContext(glu::RenderConfig const&, tcu::CommandLine const&, glu::RenderContext const*) const (in /deqp/modules/gles2/deqp-gles2)

Acked-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>

---

 src/loader/loader.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/loader/loader.c b/src/loader/loader.c
index a45328e2464..6e3e59d1687 100644
--- a/src/loader/loader.c
+++ b/src/loader/loader.c
@@ -459,15 +459,6 @@ loader_get_driver_for_fd(int fd)
       return driver;
 #endif
 
-   driver = loader_get_kernel_driver_name(fd);
-   bool is_amdgpu = driver && strcmp(driver, "amdgpu") == 0;
-   free(driver);
-
-   if (is_amdgpu) {
-      driver = strdup("radeonsi");
-      goto out;
-   }
-
    if (!loader_get_pci_id_for_fd(fd, &vendor_id, &chip_id)) {
       driver = loader_get_kernel_driver_name(fd);
       if (driver)
@@ -494,6 +485,15 @@ loader_get_driver_for_fd(int fd)
          }
    }
 
+   driver = loader_get_kernel_driver_name(fd);
+   bool is_amdgpu = driver && strcmp(driver, "amdgpu") == 0;
+   free(driver);
+
+   if (is_amdgpu)
+      driver = strdup("radeonsi");
+   else
+      driver = NULL;
+
 out:
    log_(driver ? _LOADER_DEBUG : _LOADER_WARNING,
          "pci id for fd %d: %04x:%04x, driver %s\n",