From 0c220741e6c6a70250cd822bce7bf408c989194b Mon Sep 17 00:00:00 2001 From: Mike Blumenkrantz Date: Mon, 5 Aug 2024 12:05:42 -0400 Subject: [PATCH] pipe-loader: fix driconf memory management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this had a number of issues: * pipe_loader_get_driinfo_xml() frees driver_driconf immediately, except the driOptionCache struct string pointers are all just copied in merge_driconf instead of having the strings copied, which means any subsequent access of driver_driconf strings is invalid access * pipe_loader_drm_get_driconf_by_name() is a disaster that only happened to work because the dlopen here is the same lib that gets opened elsewhere by mesa and not closed. if the lib here is actually closed, then all the statically allocated strings become invalid, which means they need to be manually copied cc: mesa-stable Reviewed-by: Marek Olšák Part-of: --- .../auxiliary/pipe-loader/pipe_loader.c | 2 +- .../auxiliary/pipe-loader/pipe_loader_drm.c | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c index f7a56ea8e82..721204d95a7 100644 --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c @@ -159,10 +159,10 @@ pipe_loader_get_driinfo_xml(const char *driver_name) unsigned merged_count; const driOptionDescription *merged_driconf = merge_driconf(driver_driconf, driver_count, &merged_count); - free((void *)driver_driconf); char *xml = driGetOptionsXml(merged_driconf, merged_count); + free((void *)driver_driconf); free((void *)merged_driconf); return xml; diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c index 0e80eb95899..263c80618bd 100644 --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c @@ -384,8 +384,42 @@ pipe_loader_drm_get_driconf_by_name(const char *driver_name, unsigned *count) } else { *count = dd->driconf_count; size_t size = sizeof(*driconf) * *count; + size_t base_size = size; + /* factor in all the statically allocated string lengths */ + for (unsigned i = 0; i < dd->driconf_count; i++) { + if (dd->driconf[i].desc) + size += strlen(dd->driconf[i].desc) + 1; + if (dd->driconf[i].info.name) + size += strlen(dd->driconf[i].info.name) + 1; + if (dd->driconf[i].info.type == DRI_STRING) + size += strlen(dd->driconf[i].value._string) + 1; + } driconf = malloc(size); memcpy(driconf, dd->driconf, size); + + uint8_t *ptr = (void*)driconf; + ptr += base_size; + /* manually set up pointers and copy in all the statically allocated strings */ + for (unsigned i = 0; i < dd->driconf_count; i++) { + if (dd->driconf[i].desc) { + driconf[i].desc = (void*)ptr; + size_t str_size = strlen(dd->driconf[i].desc) + 1; + memcpy((void*)driconf[i].desc, dd->driconf[i].desc, str_size); + ptr += str_size; + } + if (dd->driconf[i].info.name) { + driconf[i].info.name = (void*)ptr; + size_t str_size = strlen(dd->driconf[i].info.name) + 1; + memcpy((void*)driconf[i].info.name, dd->driconf[i].info.name, str_size); + ptr += str_size; + } + if (dd->driconf[i].info.type == DRI_STRING) { + driconf[i].value._string = (void*)ptr; + size_t str_size = strlen(dd->driconf[i].value._string) + 1; + memcpy((void*)driconf[i].value._string, dd->driconf[i].value._string, str_size); + ptr += str_size; + } + } } if (lib) util_dl_close(lib);