From 9a1327f2ec344ea94ad94b136267447246ee708c 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: (cherry picked from commit 0c220741e6c6a70250cd822bce7bf408c989194b) --- .pick_status.json | 2 +- .../auxiliary/pipe-loader/pipe_loader.c | 2 +- .../auxiliary/pipe-loader/pipe_loader_drm.c | 34 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index eeddcd2de41..c040fbea1ae 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -4504,7 +4504,7 @@ "description": "pipe-loader: fix driconf memory management", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null, "notes": null 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 70b3f3a59d7..371d0e9cae1 100644 --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c @@ -350,8 +350,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);