From 2d7b7edec64b282ba359bd9d7c1d0f3343351801 Mon Sep 17 00:00:00 2001 From: Adam Schachne Date: Tue, 28 Apr 2026 05:01:51 -0700 Subject: [PATCH 01/17] draw translation service overlay within content viewport (#18986) --- gfx/gfx_widgets.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/gfx/gfx_widgets.c b/gfx/gfx_widgets.c index 0e48dc9b3b0f..eb559b5359cc 100644 --- a/gfx/gfx_widgets.c +++ b/gfx/gfx_widgets.c @@ -1639,6 +1639,11 @@ void gfx_widgets_frame(void *data) /* AI Service overlay */ if (p_dispwidget->ai_service_overlay_state > 0) { + video_viewport_t content_vp; + int overlay_x = 0; + int overlay_y = 0; + unsigned overlay_width = video_width; + unsigned overlay_height = video_height; float outline_color[16] = { 0.00, 1.00, 0.00, 1.00, 0.00, 1.00, 0.00, 1.00, @@ -1646,6 +1651,13 @@ void gfx_widgets_frame(void *data) 0.00, 1.00, 0.00, 1.00, }; + if (video_driver_get_viewport_info(&content_vp) && content_vp.width && content_vp.height) + { + overlay_x = content_vp.x; + overlay_y = content_vp.y; + overlay_width = content_vp.width; + overlay_height = content_vp.height; + } gfx_display_set_alpha(p_dispwidget->pure_white, 1.0f); if (p_dispwidget->ai_service_overlay_texture) @@ -1657,11 +1669,11 @@ void gfx_widgets_frame(void *data) p_disp, video_width, video_height, - video_width, - video_height, + overlay_width, + overlay_height, p_dispwidget->ai_service_overlay_texture, - 0, - 0, + overlay_x, + overlay_y, 0.0f, /* rad */ 1.0f, /* cos(rad) = cos(0) = 1.0f */ 0.0f, /* sine(rad) = sine(0) = 0.0f */ @@ -1676,8 +1688,8 @@ void gfx_widgets_frame(void *data) p_disp, userdata, video_width, video_height, - 0, 0, - video_width, + overlay_x, overlay_y, + overlay_width, p_dispwidget->divider_width_1px, video_width, video_height, @@ -1689,9 +1701,9 @@ void gfx_widgets_frame(void *data) p_disp, userdata, video_width, video_height, - 0, - video_height - p_dispwidget->divider_width_1px, - video_width, + overlay_x, + overlay_y + overlay_height - p_dispwidget->divider_width_1px, + overlay_width, p_dispwidget->divider_width_1px, video_width, video_height, @@ -1704,10 +1716,10 @@ void gfx_widgets_frame(void *data) userdata, video_width, video_height, - 0, - 0, + overlay_x, + overlay_y, p_dispwidget->divider_width_1px, - video_height, + overlay_height, video_width, video_height, outline_color, @@ -1718,10 +1730,10 @@ void gfx_widgets_frame(void *data) p_disp, userdata, video_width, video_height, - video_width - p_dispwidget->divider_width_1px, - 0, + overlay_x + overlay_width - p_dispwidget->divider_width_1px, + overlay_y, p_dispwidget->divider_width_1px, - video_height, + overlay_height, video_width, video_height, outline_color, From da5e650ccf382629b3a0c985e173c407e620cd69 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 14:13:31 +0200 Subject: [PATCH 02/17] gfx/vulkan: fix three heap-corruption bugs in driver and common layer + CI Three independent issues found during a focused audit of gfx/drivers/ vulkan.c and gfx/common/vulkan_common.c, all in the same memory-safety class (writes past compile-time-sized arrays or undersized allocations). Submitting together because they share the same audit context and none stands alone particularly well. Adds AddressSanitizer regression tests for each under samples/gfx/, wired into a new .github/workflows/Linux-samples-gfx.yml CI job. * vulkan_find_device_extensions: heap overflow (gfx/common/vulkan_common.c) The required-extension list is currently written twice -- once via memcpy at the top of the body, then again in a per-element loop that appears to be left over from an earlier refactor. The instance-side counterpart vulkan_find_instance_extensions only logs in its loop, confirming the divergence. The redundant write consumes more slots in the caller's `enabled` buffer than the slot accounting expects. Inside vulkan_context_create_device_wrapper the buffer is sized for (info.enabledExtensionCount + ARRAY_SIZE(required) + ARRAY_SIZE(optional)) entries, but the function actually writes (enabledExtensionCount + 2*required + optional) entries worst-case. With current values (1 required, 3 optional) that is a one-element heap overflow at the end of the malloc'd block whenever a libretro core uses the Vulkan HW context-negotiation interface and the GPU supports at least one optional extension. The other call site in vulkan_context_init_device uses a stack buffer of size 8 and stays inside it today, but only by a margin of three. The duplicate writes also produce duplicate entries in ppEnabledExtensionNames passed to vkCreateDevice, which the spec forbids (VUID-VkDeviceCreateInfo-ppEnabledExtensionNames-01840); strict validation layers will have been complaining. Drop the per-element loop -- the early vulkan_find_extensions() check at the top of the function already validates that all required extensions are present, so the per-element re-check has never done any work. Keep the debug log. * vulkan_create_swapchain: OOB writes from unclamped image count (gfx/common/vulkan_common.c) vulkan_create_swapchain calls vkGetSwapchainImagesKHR twice -- first to query the count, then to fill swapchain_images[] -- and never clamps the count between the two calls. swapchain_images, swapchain_fences, the four swapchain_*_semaphores arrays, readback.staging[] and vk->swapchain[] are all sized at compile time to VULKAN_MAX_SWAPCHAIN_IMAGES (8). If a driver returns more images than that the second vkGetSwapchainImagesKHR writes past swapchain_images[], and every loop bounded by num_swapchain_images (~12 sites across init/deinit/textures/ buffers/descriptor pools/command buffers/readback and direct vk->swapchain[i] accesses) walks past its array. Two clamps. First, cap desired_swapchain_images to VULKAN_MAX_SWAPCHAIN_IMAGES before vkCreateSwapchainKHR so we never ask a well-behaved driver for more images than we can hold. Second, clamp num_swapchain_images between the two vkGetSwapchainImagesKHR calls to handle drivers that return more images than were requested -- the spec permits this. * vulkan_create_texture: 32-bit overflow in staging-buffer sizing (gfx/drivers/vulkan.c) buffer_width = width * bpp and buffer_info.size = buffer_width * height are both unsigned * unsigned in 32-bit before being widened to VkDeviceSize on assignment. With dimensions large enough to wrap (e.g. width=65536, height=16385, bpp=4 -> 0x1_0004_0000 wraps to 0x40000), the staging buffer is allocated at the wrapped (small) size while the per-row upload memcpy loop later in the same function walks the unmodified width x height. The mapped region is smaller than what the loop writes, so the memcpy walks past the mapping into adjacent heap memory. Reachable from libretro cores supplying oversized retro_framebuffer dimensions and from vulkan_load_texture / vulkan_set_texture_frame, which take dimensions originating in image decoders. Compute the staging-buffer size in 64-bit, and widen the upload loop's stride and per-row copy size to size_t. VkDeviceSize is already 64-bit so the assignment to buffer_info.size is now correct end-to-end. The loop's pointer math was already implicitly widened on 64-bit hosts but is now unconditionally correct on 32-bit hosts (3DS, Vita, PSP, Wii, Wii U, older Android, 32-bit Windows) as well. * samples/gfx/, .github/workflows/Linux-samples-gfx.yml Three regression tests, one per fix above, following the pattern established by the security regression tests under samples/tasks/ (archive_name_safety_test, http_method_match_test, video_shader_wildcard_test, input_remap_bounds_test, bsv_replay_bounds_test, bps_patch_bounds_test). Each test keeps a verbatim copy of the relevant post-fix predicate, demarcated by `=== verbatim copy ===` markers; an `IMPORTANT: ... must follow` comment documents the convention. Plain C, asserts manually, exits nonzero on failure -- matches the harness the existing Linux-samples-{tasks,libretro-{common,db}}-samples.yml workflows understand. vulkan_extension_count_test (5 cases): create_device_wrapper- shaped caller, init_device-shaped caller, GPU supporting no optional extensions, GPU missing a required extension, and the prepushed-via-negotiation-interface case. Verified pre/post- patch discrimination: under the pre-fix double-write shape the create_device_wrapper case ASan-aborts with heap-buffer-overflow. vulkan_swapchain_clamp_test (5 cases): well-behaved 3-image driver, at-capacity boundary, 9-image (MAX+1) driver, pathological 64-image driver, and the request-side cap across six input values. Verified pre/post-patch discrimination: under the pre-fix no-clamp shape the 9-image case ASan-aborts on the second vkGetSwapchainImagesKHR fill. vulkan_texture_size_test (6 cases): typical 320x240, 4K RGBA, the smoking-gun 65536x16385x4 wrap case, an assorted-shapes table, the per-row upload-loop strides under an ASan-instrumented buffer, and a 64-bit-result sanity check. Verified pre/post- patch discrimination: under the pre-fix 32-bit arithmetic the overflow cases produce arithmetic mismatches against the 64-bit reference. Linux-samples-gfx.yml is a near-verbatim copy of Linux-samples-tasks.yml, runs each test under -fsanitize=address with a per-step explanation of what the test is regression-testing and the `verbatim copy must follow` reminder. ~3 seconds per test on Ubuntu 24 with system gcc. --- .github/workflows/Linux-samples-gfx.yml | 109 +++++ gfx/common/vulkan_common.c | 40 +- gfx/drivers/vulkan.c | 18 +- samples/gfx/vulkan_extension_count/Makefile | 25 + .../vulkan_extension_count_test.c | 447 ++++++++++++++++++ samples/gfx/vulkan_swapchain_clamp/Makefile | 25 + .../vulkan_swapchain_clamp_test.c | 350 ++++++++++++++ samples/gfx/vulkan_texture_size/Makefile | 25 + .../vulkan_texture_size_test.c | 355 ++++++++++++++ 9 files changed, 1378 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/Linux-samples-gfx.yml create mode 100644 samples/gfx/vulkan_extension_count/Makefile create mode 100644 samples/gfx/vulkan_extension_count/vulkan_extension_count_test.c create mode 100644 samples/gfx/vulkan_swapchain_clamp/Makefile create mode 100644 samples/gfx/vulkan_swapchain_clamp/vulkan_swapchain_clamp_test.c create mode 100644 samples/gfx/vulkan_texture_size/Makefile create mode 100644 samples/gfx/vulkan_texture_size/vulkan_texture_size_test.c diff --git a/.github/workflows/Linux-samples-gfx.yml b/.github/workflows/Linux-samples-gfx.yml new file mode 100644 index 000000000000..251811ef4bf0 --- /dev/null +++ b/.github/workflows/Linux-samples-gfx.yml @@ -0,0 +1,109 @@ +name: CI Linux samples/gfx + +on: + push: + branches: + - master + pull_request: + branches: + - master + workflow_dispatch: + +permissions: + contents: read + +env: + ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true + +jobs: + samples-gfx: + name: Build and run samples/gfx + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Install dependencies + run: | + sudo apt-get update -y + sudo apt-get install -y build-essential + + - name: Checkout + uses: actions/checkout@v3 + + - name: Build and run vulkan_extension_count_test (ASan) + shell: bash + working-directory: samples/gfx/vulkan_extension_count + run: | + set -eu + # Regression test for the heap-overflow fix in + # gfx/common/vulkan_common.c::vulkan_find_device_extensions. + # Pre-fix the function appended the required-extension list + # twice -- once via memcpy at the top of the body, then + # again in a per-element loop -- consuming + # (count_initial + 2*num_required + num_optional) slots in + # the caller's buffer. vulkan_context_create_device_wrapper + # sized its malloc for (count_initial + num_required + + # num_optional) entries, so a libretro core using the Vulkan + # HW context-negotiation interface against a GPU exposing at + # least one optional extension hit a one-element heap-buffer- + # overflow (8 bytes on 64-bit) at the end of the malloc'd + # block. Build under AddressSanitizer so any reintroduction + # of the duplicate write is caught at the bounds level. If + # vulkan_common.c amends the append-to-enabled[] block, the + # verbatim copy in vulkan_extension_count_test.c must follow. + make clean all SANITIZER=address + test -x vulkan_extension_count_test + timeout 60 ./vulkan_extension_count_test + echo "[pass] vulkan_extension_count_test" + + - name: Build and run vulkan_swapchain_clamp_test (ASan) + shell: bash + working-directory: samples/gfx/vulkan_swapchain_clamp + run: | + set -eu + # Regression test for the unclamped-swapchain-image-count + # fix in gfx/common/vulkan_common.c::vulkan_create_swapchain. + # Pre-fix the two vkGetSwapchainImagesKHR calls (count + # query + image fill) had no clamp between them, so a driver + # returning more than VULKAN_MAX_SWAPCHAIN_IMAGES (8) images + # on the second call wrote past context.swapchain_images[8] + # and every loop bounded by num_swapchain_images walked past + # its compile-time-sized companion array (~12 such loops + # across init/deinit/textures/buffers/descriptor pools/ + # command buffers/readback and direct vk->swapchain[i] + # uses). Build under AddressSanitizer so any reintroduction + # of either the request-side or the post-create clamp is + # caught at the bounds level. If vulkan_common.c amends + # the cap or the post-create clamp, the verbatim copies in + # vulkan_swapchain_clamp_test.c must follow. + make clean all SANITIZER=address + test -x vulkan_swapchain_clamp_test + timeout 60 ./vulkan_swapchain_clamp_test + echo "[pass] vulkan_swapchain_clamp_test" + + - name: Build and run vulkan_texture_size_test (ASan) + shell: bash + working-directory: samples/gfx/vulkan_texture_size + run: | + set -eu + # Regression test for the 32-bit-overflow fix in + # gfx/drivers/vulkan.c::vulkan_create_texture's staging- + # buffer sizing. Pre-fix the buffer_info.size calculation + # (buffer_width * height) was unsigned*unsigned in 32-bit + # before being widened to VkDeviceSize on assignment. With + # dimensions large enough to wrap (e.g. 65536x16385x4 -> + # 0x1_0004_0000 truncates to 0x40000), the staging buffer + # was malloc'd at the wrapped (small) size while the per-row + # upload memcpy loop walked the full width x height, + # writing past the mapped region into adjacent heap memory. + # Reachable from libretro cores supplying oversized + # retro_framebuffer dimensions and from vulkan_load_texture + # / vulkan_set_texture_frame. Build under AddressSanitizer + # so any reintroduction of the 32-bit arithmetic is caught + # at the bounds level. If vulkan.c amends the size + # calculation or upload-loop strides, the verbatim copies + # in vulkan_texture_size_test.c must follow. + make clean all SANITIZER=address + test -x vulkan_texture_size_test + timeout 60 ./vulkan_texture_size_test + echo "[pass] vulkan_texture_size_test" diff --git a/gfx/common/vulkan_common.c b/gfx/common/vulkan_common.c index 0737cdcf7bfa..c12176a35d71 100644 --- a/gfx/common/vulkan_common.c +++ b/gfx/common/vulkan_common.c @@ -432,21 +432,12 @@ static bool vulkan_find_device_extensions(VkPhysicalDevice gpu, goto end; } + /* Required extensions: presence already validated by the + * vulkan_find_extensions() check above. Append in one shot. */ memcpy((void*)(enabled + count), exts, num_exts * sizeof(*exts)); - count += num_exts; - for (i = 0; i < num_exts; i++) - { - if (vulkan_find_extensions(&exts[i], 1, properties, property_count)) - { - RARCH_DBG("[Vulkan] Device extension supported: %s.\n", exts[i]); - enabled[count++] = exts[i]; - } - else - { - RARCH_DBG("[Vulkan] Device extension NOT supported: %s.\n", exts[i]); - } - } + RARCH_DBG("[Vulkan] Device extension supported: %s.\n", exts[i]); + count += num_exts; for (i = 0; i < num_optional_exts; i++) { @@ -2300,6 +2291,15 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, && (desired_swapchain_images > surface_properties.maxImageCount)) desired_swapchain_images = surface_properties.maxImageCount; + /* Cap our request to what we can actually hold. Per-image arrays + * (swapchain_images, swapchain_fences, the various semaphore + * arrays, vk->swapchain[], readback.staging[]) are all sized to + * VULKAN_MAX_SWAPCHAIN_IMAGES at compile time. The post-create + * fill below also clamps defensively in case a driver returns + * more images than requested. */ + if (desired_swapchain_images > VULKAN_MAX_SWAPCHAIN_IMAGES) + desired_swapchain_images = VULKAN_MAX_SWAPCHAIN_IMAGES; + if (surface_properties.supportedTransforms & VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR) pre_transform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR; @@ -2415,6 +2415,20 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, vkGetSwapchainImagesKHR(vk->context.device, vk->swapchain, &vk->context.num_swapchain_images, NULL); + + /* Even after capping minImageCount above, drivers may legally + * return more images than requested. Clamp before the fill call + * so we don't write past swapchain_images[] and so every + * downstream loop bounded by num_swapchain_images stays inside + * its compile-time-sized array. */ + if (vk->context.num_swapchain_images > VULKAN_MAX_SWAPCHAIN_IMAGES) + { + RARCH_WARN("[Vulkan] Swapchain returned %u images, clamping to %u.\n", + vk->context.num_swapchain_images, + (unsigned)VULKAN_MAX_SWAPCHAIN_IMAGES); + vk->context.num_swapchain_images = VULKAN_MAX_SWAPCHAIN_IMAGES; + } + vkGetSwapchainImagesKHR(vk->context.device, vk->swapchain, &vk->context.num_swapchain_images, vk->context.swapchain_images); diff --git a/gfx/drivers/vulkan.c b/gfx/drivers/vulkan.c index 26e3b02bb92d..f4cca0d855bd 100644 --- a/gfx/drivers/vulkan.c +++ b/gfx/drivers/vulkan.c @@ -1017,6 +1017,7 @@ static struct vk_texture vulkan_create_texture(vk_t *vk, enum vk_texture_type type) { unsigned i; + uint64_t buffer_size_64; uint32_t buffer_width; struct vk_texture tex; VkImageCreateInfo info; @@ -1051,11 +1052,18 @@ static struct vk_texture vulkan_create_texture(vk_t *vk, /* Align stride to 4 bytes to make sure we can use compute shader uploads without too many problems. */ buffer_width = width * vulkan_format_to_bpp(format); buffer_width = (buffer_width + 3u) & ~3u; + /* Compute the buffer size in 64-bit. width*bpp*height as a 32-bit + * unsigned would wrap for sufficiently large dimensions (e.g. an + * upscaled shader render target chain), leaving the staging buffer + * underallocated relative to the upload memcpy loop further down + * and producing a heap overflow on the host side. VkDeviceSize is + * 64-bit; widen the math to match. */ + buffer_size_64 = (uint64_t)buffer_width * (uint64_t)height; buffer_info.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; buffer_info.pNext = NULL; buffer_info.flags = 0; - buffer_info.size = buffer_width * height; + buffer_info.size = buffer_size_64; buffer_info.usage = 0; buffer_info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; buffer_info.queueFamilyIndexCount = 0; @@ -1365,14 +1373,18 @@ static struct vk_texture vulkan_create_texture(vk_t *vk, const uint8_t *src = NULL; void *ptr = NULL; unsigned bpp = vulkan_format_to_bpp(tex.format); - unsigned stride = tex.width * bpp; + /* Source stride and per-row copy size in size_t to keep + * the pointer math and memcpy length safe even when + * width*bpp would otherwise wrap a 32-bit unsigned. */ + size_t stride = (size_t)tex.width * (size_t)bpp; + size_t row_bytes = (size_t)width * (size_t)bpp; vkMapMemory(device, tex.memory, tex.offset, tex.size, 0, &ptr); dst = (uint8_t*)ptr; src = (const uint8_t*)initial; for (y = 0; y < tex.height; y++, dst += tex.stride, src += stride) - memcpy(dst, src, width * bpp); + memcpy(dst, src, row_bytes); if ( (tex.flags & VK_TEX_FLAG_NEED_MANUAL_CACHE_MANAGEMENT) && (tex.memory != VK_NULL_HANDLE)) diff --git a/samples/gfx/vulkan_extension_count/Makefile b/samples/gfx/vulkan_extension_count/Makefile new file mode 100644 index 000000000000..f271f4b5513d --- /dev/null +++ b/samples/gfx/vulkan_extension_count/Makefile @@ -0,0 +1,25 @@ +TARGET := vulkan_extension_count_test + +SOURCES := vulkan_extension_count_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/vulkan_extension_count/vulkan_extension_count_test.c b/samples/gfx/vulkan_extension_count/vulkan_extension_count_test.c new file mode 100644 index 000000000000..c9bebf670a3b --- /dev/null +++ b/samples/gfx/vulkan_extension_count/vulkan_extension_count_test.c @@ -0,0 +1,447 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (vulkan_extension_count_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the heap-overflow fix in + * gfx/common/vulkan_common.c::vulkan_find_device_extensions. + * + * Pre-fix the function appended the required-extension list to + * the caller's `enabled` buffer twice -- once via memcpy at the + * top of the body, then again in a per-element loop: + * + * memcpy((void*)(enabled + count), exts, num_exts * sizeof(*exts)); + * count += num_exts; + * for (i = 0; i < num_exts; i++) + * if (vulkan_find_extensions(&exts[i], 1, properties, ...)) + * enabled[count++] = exts[i]; // <-- duplicate + * for (i = 0; i < num_optional_exts; i++) + * if (vulkan_find_extensions(&optional_exts[i], 1, ...)) + * enabled[count++] = optional_exts[i]; + * + * Net writes worst-case: count_initial + 2*num_required + + * num_optional. + * + * The caller in vulkan_context_create_device_wrapper sized its + * malloc for count_initial + num_required + num_optional entries: + * + * const char **device_extensions = malloc( + * (info.enabledExtensionCount + * + ARRAY_SIZE(vulkan_device_extensions) + * + ARRAY_SIZE(vulkan_optional_device_extensions)) + * * sizeof(const char *)); + * + * With current ARRAY_SIZE values (1 required, 3 optional) and a + * GPU exposing all optional extensions, the second pass of writes + * placed one extra string-pointer past the end of the malloc'd + * block -- 8 bytes on a 64-bit host. Reachable whenever a + * libretro core uses the Vulkan HW context-negotiation interface + * (iface->create_device / create_device2), which is the standard + * path for cores using Vulkan HW rendering. + * + * The instance-side cousin vulkan_find_instance_extensions only + * logs in its loop -- the divergence indicates the device-side + * loop was a stale leftover from a refactor. + * + * The duplicate writes also produced duplicate entries in + * ppEnabledExtensionNames passed to vkCreateDevice, which the + * spec forbids (VUID-VkDeviceCreateInfo- + * ppEnabledExtensionNames-01840). + * + * Fix: drop the per-element loop entirely; rely on the early + * vulkan_find_extensions(exts, num_exts, ...) check at the top + * of the function, which already validates that all required + * extensions are present. + * + * IMPORTANT: this test keeps a verbatim copy of the post-fix + * append-to-enabled[] block from vulkan_common.c. If the + * function amends, the copy below must follow. Convention used + * by archive_name_safety_test, http_method_match_test, + * video_shader_wildcard_test, input_remap_bounds_test, + * bsv_replay_bounds_test, bps_patch_bounds_test in + * samples/tasks/. + */ + +#include +#include +#include +#include +#include + +/* --- Mock the parts of the Vulkan extension-property API the + * function under test actually consults. In production + * vulkan_find_extensions walks an array of + * VkExtensionProperties returned by + * vkEnumerateDeviceExtensionProperties; for the test we feed + * a flat array of strings and report which of the queried + * names match. The test-side mock only needs to be + * consistent with how the post-fix code calls it. --- */ + +struct mock_ext_props { + const char **names; + unsigned count; +}; + +static bool mock_find_extensions(const char * const *exts, unsigned num_exts, + const struct mock_ext_props *props) +{ + unsigned i, j; + for (i = 0; i < num_exts; i++) + { + bool found = false; + for (j = 0; j < props->count; j++) + { + if (strcmp(exts[i], props->names[j]) == 0) + { + found = true; + break; + } + } + if (!found) + return false; + } + return true; +} + +/* === verbatim copy of the post-fix append block from + * gfx/common/vulkan_common.c::vulkan_find_device_extensions. + * The early "all required extensions present" check at the + * top of the function is folded into the caller's pre-flight + * here so the test exercises the same write pattern. If + * vulkan_common.c amends this block, the copy must follow. === */ +static bool find_device_extensions_postfix( + const char **enabled, unsigned *inout_enabled_count, + const char **exts, unsigned num_exts, + const char **optional_exts, unsigned num_optional_exts, + const struct mock_ext_props *props) +{ + unsigned i; + unsigned count = *inout_enabled_count; + + if (!mock_find_extensions(exts, num_exts, props)) + return false; + + /* Required extensions: presence already validated by the + * vulkan_find_extensions() check above. Append in one shot. */ + memcpy((void*)(enabled + count), exts, num_exts * sizeof(*exts)); + for (i = 0; i < num_exts; i++) + (void)exts[i]; /* would be RARCH_DBG in production */ + count += num_exts; + + for (i = 0; i < num_optional_exts; i++) + { + if (mock_find_extensions(&optional_exts[i], 1, props)) + enabled[count++] = optional_exts[i]; + } + + *inout_enabled_count = count; + return true; +} +/* === end verbatim copy === */ + +static int failures = 0; + +/* Probe: caller sized for required + optional only; run the + * post-fix append against it. ASan instruments the malloc'd + * region exactly, so a regression to the pre-fix double-write + * trips heap-buffer-overflow. Caller layout matches + * vulkan_context_create_device_wrapper. */ +static void test_create_device_wrapper_caller(void) +{ + /* Simulates: 1 required, 3 optional, GPU exposes everything, + * info.enabledExtensionCount initially 0 (nothing pre-pushed + * by the negotiation interface). */ + static const char *required[] = { "VK_KHR_swapchain" }; + static const char *optional[] = { + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive", + "VK_KHR_portability_subset" + }; + static const char *gpu_exts[] = { + "VK_KHR_swapchain", + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive", + "VK_KHR_portability_subset", + "VK_EXT_some_other_ext" /* GPU has extras; must not be picked up */ + }; + const unsigned num_req = sizeof(required) / sizeof(required[0]); + const unsigned num_opt = sizeof(optional) / sizeof(optional[0]); + const unsigned cap = 0 + num_req + num_opt; /* caller's malloc */ + const char **enabled = (const char **)malloc(cap * sizeof(const char *)); + unsigned count = 0; + struct mock_ext_props props = { gpu_exts, sizeof(gpu_exts)/sizeof(gpu_exts[0]) }; + unsigned i, expected; + bool ok; + + if (!enabled) + { + printf("[ERROR] malloc failed\n"); + failures++; + return; + } + + ok = find_device_extensions_postfix(enabled, &count, + required, num_req, + optional, num_opt, + &props); + + if (!ok) + { + printf("[ERROR] post-fix predicate rejected an all-supported GPU\n"); + failures++; + free((void*)enabled); + return; + } + + /* Post-fix writes exactly num_req + num_opt slots when the GPU + * exposes everything. Pre-fix would have written 2*num_req + + * num_opt -- one past the malloc'd region. */ + expected = num_req + num_opt; + if (count != expected) + { + printf("[ERROR] count = %u, expected %u (pre-fix would be %u)\n", + count, expected, 2 * num_req + num_opt); + failures++; + free((void*)enabled); + return; + } + + /* No duplicates -- the spec forbids them in + * ppEnabledExtensionNames. Pre-fix had VK_KHR_swapchain + * appearing twice. */ + for (i = 0; i < count; i++) + { + unsigned j; + for (j = i + 1; j < count; j++) + { + if (strcmp(enabled[i], enabled[j]) == 0) + { + printf("[ERROR] duplicate extension '%s' in enabled list\n", + enabled[i]); + failures++; + free((void*)enabled); + return; + } + } + } + + printf("[SUCCESS] create_device_wrapper caller: %u entries, no overflow, no duplicates\n", + count); + free((void*)enabled); +} + +/* Probe: stack-buffer caller (vulkan_context_init_device uses + * `const char *enabled_device_extensions[8]`). Capacity 8 vs. + * worst-case post-fix writes of num_req + num_opt = 4. This + * exercise documents the second call site -- the buffer fits + * comfortably today, but only by a margin of 4. */ +static void test_init_device_caller(void) +{ + static const char *required[] = { "VK_KHR_swapchain" }; + static const char *optional[] = { + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive", + "VK_KHR_portability_subset" + }; + static const char *gpu_exts[] = { + "VK_KHR_swapchain", + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive", + "VK_KHR_portability_subset" + }; + const unsigned num_req = sizeof(required) / sizeof(required[0]); + const unsigned num_opt = sizeof(optional) / sizeof(optional[0]); + /* Mirror the production stack array literally. */ + const char *enabled[8] = {0}; + unsigned count = 0; + struct mock_ext_props props = { gpu_exts, sizeof(gpu_exts)/sizeof(gpu_exts[0]) }; + bool ok; + + ok = find_device_extensions_postfix(enabled, &count, + required, num_req, + optional, num_opt, + &props); + + if (!ok || count != num_req + num_opt) + { + printf("[ERROR] init_device caller: ok=%d count=%u expected=%u\n", + (int)ok, count, num_req + num_opt); + failures++; + return; + } + + if (count > sizeof(enabled) / sizeof(enabled[0])) + { + printf("[ERROR] init_device caller: count=%u > stack capacity 8\n", + count); + failures++; + return; + } + + printf("[SUCCESS] init_device caller: %u entries fits in stack[8]\n", count); +} + +/* Probe: GPU exposes none of the optional extensions. Post-fix + * writes exactly num_req slots; the caller's malloc still fits + * the optional set's worth of slack as required by the function + * signature, but only the required ones are populated. */ +static void test_no_optional_supported(void) +{ + static const char *required[] = { "VK_KHR_swapchain" }; + static const char *optional[] = { + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive", + "VK_KHR_portability_subset" + }; + static const char *gpu_exts[] = { + "VK_KHR_swapchain" /* required only, no optional */ + }; + const unsigned num_req = 1; + const unsigned num_opt = 3; + const unsigned cap = num_req + num_opt; + const char **enabled = (const char **)malloc(cap * sizeof(const char *)); + unsigned count = 0; + struct mock_ext_props props = { gpu_exts, 1 }; + bool ok; + + if (!enabled) { printf("[ERROR] malloc\n"); failures++; return; } + + ok = find_device_extensions_postfix(enabled, &count, + required, num_req, optional, num_opt, &props); + + if (!ok || count != num_req) + { + printf("[ERROR] no-optional case: ok=%d count=%u expected=%u\n", + (int)ok, count, num_req); + failures++; + } + else + printf("[SUCCESS] no-optional supported: %u entries (required only)\n", + count); + + free((void*)enabled); +} + +/* Probe: GPU is missing a required extension. Post-fix returns + * false up front, no writes happen. Caller gets to free + * everything cleanly. */ +static void test_required_missing_rejects(void) +{ + static const char *required[] = { "VK_KHR_swapchain" }; + static const char *optional[] = { + "VK_KHR_sampler_mirror_clamp_to_edge" + }; + static const char *gpu_exts[] = { + "VK_EXT_some_other_ext" /* no swapchain support */ + }; + const unsigned cap = 1 + 1; + const char **enabled = (const char **)malloc(cap * sizeof(const char *)); + unsigned count = 0; + struct mock_ext_props props = { gpu_exts, 1 }; + bool ok; + + if (!enabled) { printf("[ERROR] malloc\n"); failures++; return; } + + ok = find_device_extensions_postfix(enabled, &count, + required, 1, optional, 1, &props); + + if (ok) + { + printf("[ERROR] missing required extension was accepted\n"); + failures++; + } + else if (count != 0) + { + printf("[ERROR] missing-required path mutated count to %u\n", count); + failures++; + } + else + printf("[SUCCESS] missing required extension rejected, count untouched\n"); + + free((void*)enabled); +} + +/* Probe: caller pre-pushes some entries via the negotiation + * interface (info.enabledExtensionCount > 0). Post-fix appends + * starting from `count`, doesn't disturb pre-pushed entries. */ +static void test_with_prepushed_entries(void) +{ + static const char *required[] = { "VK_KHR_swapchain" }; + static const char *optional[] = { + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive" + }; + static const char *gpu_exts[] = { + "VK_KHR_swapchain", + "VK_KHR_sampler_mirror_clamp_to_edge", + "VK_EXT_full_screen_exclusive" + }; + const unsigned num_req = 1; + const unsigned num_opt = 2; + const unsigned prepushed = 2; + const unsigned cap = prepushed + num_req + num_opt; + const char **enabled = (const char **)malloc(cap * sizeof(const char *)); + unsigned count = prepushed; + struct mock_ext_props props = { gpu_exts, 3 }; + bool ok; + + if (!enabled) { printf("[ERROR] malloc\n"); failures++; return; } + + /* Pre-push two entries the way create_device_wrapper does. */ + enabled[0] = "VK_KHR_external_memory"; + enabled[1] = "VK_KHR_external_semaphore"; + + ok = find_device_extensions_postfix(enabled, &count, + required, num_req, optional, num_opt, &props); + + if (!ok || count != prepushed + num_req + num_opt) + { + printf("[ERROR] prepushed case: ok=%d count=%u expected=%u\n", + (int)ok, count, prepushed + num_req + num_opt); + failures++; + } + else if ( strcmp(enabled[0], "VK_KHR_external_memory") != 0 + || strcmp(enabled[1], "VK_KHR_external_semaphore") != 0) + { + printf("[ERROR] prepushed case: prepended entries clobbered\n"); + failures++; + } + else + printf("[SUCCESS] prepushed entries preserved, %u entries total\n", count); + + free((void*)enabled); +} + +int main(void) +{ + test_create_device_wrapper_caller(); + test_init_device_caller(); + test_no_optional_supported(); + test_required_missing_rejects(); + test_with_prepushed_entries(); + + if (failures) + { + printf("\n%d vulkan_extension_count test(s) failed\n", failures); + return 1; + } + printf("\nAll vulkan_extension_count regression tests passed.\n"); + return 0; +} diff --git a/samples/gfx/vulkan_swapchain_clamp/Makefile b/samples/gfx/vulkan_swapchain_clamp/Makefile new file mode 100644 index 000000000000..6cc9edb0d217 --- /dev/null +++ b/samples/gfx/vulkan_swapchain_clamp/Makefile @@ -0,0 +1,25 @@ +TARGET := vulkan_swapchain_clamp_test + +SOURCES := vulkan_swapchain_clamp_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/vulkan_swapchain_clamp/vulkan_swapchain_clamp_test.c b/samples/gfx/vulkan_swapchain_clamp/vulkan_swapchain_clamp_test.c new file mode 100644 index 000000000000..eeccad35dd28 --- /dev/null +++ b/samples/gfx/vulkan_swapchain_clamp/vulkan_swapchain_clamp_test.c @@ -0,0 +1,350 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (vulkan_swapchain_clamp_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the unclamped-swapchain-image-count fix in + * gfx/common/vulkan_common.c::vulkan_create_swapchain. + * + * The function calls vkGetSwapchainImagesKHR twice -- once to + * query the count, once to fill the swapchain_images[] array: + * + * vkGetSwapchainImagesKHR(device, swapchain, &count, NULL); + * vkGetSwapchainImagesKHR(device, swapchain, &count, images); + * + * Pre-fix, no clamp existed between the two calls. context. + * swapchain_images, swapchain_fences, the four + * swapchain_*_semaphores arrays, readback.staging[] and + * vk->swapchain[] are all sized at compile time to + * VULKAN_MAX_SWAPCHAIN_IMAGES (8). If a driver returned more + * images than that on the count query, the second call wrote + * past swapchain_images[8], and every loop bounded by + * num_swapchain_images (~12 sites across init/deinit/textures/ + * buffers/descriptor pools/command buffers/readback and direct + * vk->swapchain[i] access) walked past its compile-time-sized + * array. + * + * The Vulkan spec permits the driver to return more images than + * minImageCount requested (drivers may use triple-buffer-plus + * strategies, the spec only requires the minimum); on fast + * MAILBOX drivers under high refresh rates this is observed in + * practice. + * + * Fix: cap desired_swapchain_images to VULKAN_MAX_SWAPCHAIN_IMAGES + * before vkCreateSwapchainKHR, so well-behaved drivers are never + * asked for more than we can hold; AND clamp the count between + * the two vkGetSwapchainImagesKHR calls, to handle drivers that + * return more images than requested. + * + * IMPORTANT: this test keeps a verbatim copy of both clamp + * predicates from vulkan_common.c. If vulkan_create_swapchain + * amends them, the copies below must follow. Convention used + * by the security regression tests in samples/tasks/. + */ + +#include +#include +#include +#include +#include + +/* Match the production constant exactly. Defined in + * gfx/common/vulkan_common.h. If that header changes, this + * mirror must follow. */ +#define VULKAN_MAX_SWAPCHAIN_IMAGES 8 + +/* Match the production layout of `struct vulkan_context` for the + * fields the post-fix predicate touches. We don't model the + * neighbouring fields literally -- ASan instruments the + * containing malloc, so an OOB write off swapchain_images[] + * trips heap-buffer-overflow regardless of layout. */ +struct mock_vk_context { + /* Mock VkImage handles -- pointer-sized opaque. In production + * VkImage is a 64-bit dispatchable handle. */ + void *swapchain_images[VULKAN_MAX_SWAPCHAIN_IMAGES]; + uint32_t num_swapchain_images; +}; + +/* Mock driver: configurable count returned on first call. + * In production this is vkGetSwapchainImagesKHR. */ +struct mock_driver { + uint32_t reported_image_count; + /* Magic sentinel pointer the driver "writes" to slot N to + * make OOB-write probing observable in non-ASan builds. */ + void *fill_pattern; +}; + +static void mock_get_swapchain_images(struct mock_driver *drv, + uint32_t *count_io, void **out_images) +{ + if (out_images == NULL) + { + /* Count-query call: report the driver's full image count. */ + *count_io = drv->reported_image_count; + return; + } + + /* Fill call: write min(driver_count, *count_io) entries. + * The Vulkan spec is that the count parameter is in/out -- + * caller passes their array capacity, driver writes up to + * that many entries and updates *count to the number + * actually written. */ + { + uint32_t to_write = drv->reported_image_count; + uint32_t i; + if (to_write > *count_io) + to_write = *count_io; + for (i = 0; i < to_write; i++) + out_images[i] = drv->fill_pattern; + *count_io = to_write; + } +} + +/* === verbatim copy of the post-fix request-cap from + * gfx/common/vulkan_common.c::vulkan_create_swapchain (the + * block immediately after the maxImageCount clamp). If the + * production function amends this clamp, the copy must + * follow. === */ +static uint32_t cap_desired_request(uint32_t desired_swapchain_images) +{ + if (desired_swapchain_images > VULKAN_MAX_SWAPCHAIN_IMAGES) + desired_swapchain_images = VULKAN_MAX_SWAPCHAIN_IMAGES; + return desired_swapchain_images; +} +/* === end verbatim copy === */ + +/* === verbatim copy of the post-fix returned-count clamp from + * vulkan_create_swapchain, the block between the two + * vkGetSwapchainImagesKHR calls. If the production function + * amends this clamp, the copy must follow. === */ +static void clamp_returned_count(struct mock_vk_context *ctx) +{ + if (ctx->num_swapchain_images > VULKAN_MAX_SWAPCHAIN_IMAGES) + { + /* RARCH_WARN in production. */ + ctx->num_swapchain_images = VULKAN_MAX_SWAPCHAIN_IMAGES; + } +} +/* === end verbatim copy === */ + +static int failures = 0; + +/* Drives the post-fix two-call sequence end-to-end against a + * mock driver, returning the final num_swapchain_images and + * filling ctx->swapchain_images[]. Mirrors the production + * sequence exactly. + * + * Pre-fix, when reported_count > VULKAN_MAX_SWAPCHAIN_IMAGES the + * second mock call would write reported_count entries into the + * 8-slot array -- ASan trips heap-buffer-overflow. Post-fix, + * the clamp ensures the second call writes at most 8. + * + * The function operates on `ctx` allocated by the caller; the + * caller is malloc'd at exactly sizeof(struct mock_vk_context) + * so ASan can flag any write outside that struct's memory. + * (In production swapchain_images sits in a larger struct, so + * an OOB write would corrupt sibling fields; here it would + * corrupt heap metadata or whatever follows the malloc.) */ +static void run_postfix_sequence(struct mock_vk_context *ctx, + struct mock_driver *drv) +{ + /* First call: query count. ctx->num_swapchain_images becomes + * the driver-reported value, which may exceed our capacity. */ + ctx->num_swapchain_images = 0; /* must initialise before use */ + mock_get_swapchain_images(drv, &ctx->num_swapchain_images, NULL); + + /* Post-fix clamp -- closes the OOB write on the next call. */ + clamp_returned_count(ctx); + + /* Second call: fill. The driver may legally write up to + * *count_io entries; the clamp above ensures count_io is + * at most VULKAN_MAX_SWAPCHAIN_IMAGES. */ + mock_get_swapchain_images(drv, &ctx->num_swapchain_images, + ctx->swapchain_images); +} + +/* Probe: well-behaved driver returns 3 images for a request of + * 3. Common path on Mesa with default settings. No clamp + * fires; final count is 3. */ +static void test_well_behaved_three_images(void) +{ + struct mock_vk_context *ctx = (struct mock_vk_context *) + calloc(1, sizeof(*ctx)); + struct mock_driver drv = { 3, (void*)0xCAFE }; + + if (!ctx) { printf("[ERROR] calloc\n"); failures++; return; } + + run_postfix_sequence(ctx, &drv); + + if (ctx->num_swapchain_images != 3) + { + printf("[ERROR] well-behaved 3-image driver: got count=%u, expected 3\n", + ctx->num_swapchain_images); + failures++; + } + else if ( ctx->swapchain_images[0] != (void*)0xCAFE + || ctx->swapchain_images[2] != (void*)0xCAFE) + { + printf("[ERROR] swapchain_images[] not populated correctly\n"); + failures++; + } + else + printf("[SUCCESS] well-behaved 3-image driver: count=3, images filled\n"); + + free(ctx); +} + +/* Probe: driver returns exactly VULKAN_MAX_SWAPCHAIN_IMAGES. + * Boundary case -- clamp must NOT fire. */ +static void test_at_capacity_boundary(void) +{ + struct mock_vk_context *ctx = (struct mock_vk_context *) + calloc(1, sizeof(*ctx)); + struct mock_driver drv = { VULKAN_MAX_SWAPCHAIN_IMAGES, (void*)0xBEEF }; + + if (!ctx) { printf("[ERROR] calloc\n"); failures++; return; } + + run_postfix_sequence(ctx, &drv); + + if (ctx->num_swapchain_images != VULKAN_MAX_SWAPCHAIN_IMAGES) + { + printf("[ERROR] at-capacity boundary: count=%u, expected %u\n", + ctx->num_swapchain_images, + (unsigned)VULKAN_MAX_SWAPCHAIN_IMAGES); + failures++; + } + else + printf("[SUCCESS] at-capacity boundary: count=%u, no spurious clamp\n", + ctx->num_swapchain_images); + + free(ctx); +} + +/* Probe: driver returns 9 images (MAX + 1). Pre-fix, the + * second mock call would write 9 entries into a slot-8 array, + * trampling whatever sits past it. Post-fix, the clamp drops + * the count to 8 before the second call. ASan provides the + * actual bounds check; we additionally verify the final count + * truthfully reports 8. */ +static void test_driver_returns_nine_images(void) +{ + struct mock_vk_context *ctx = (struct mock_vk_context *) + calloc(1, sizeof(*ctx)); + struct mock_driver drv = { 9, (void*)0xDEAD }; + + if (!ctx) { printf("[ERROR] calloc\n"); failures++; return; } + + run_postfix_sequence(ctx, &drv); + + if (ctx->num_swapchain_images != VULKAN_MAX_SWAPCHAIN_IMAGES) + { + printf("[ERROR] over-cap driver: count=%u, expected clamped to %u\n", + ctx->num_swapchain_images, + (unsigned)VULKAN_MAX_SWAPCHAIN_IMAGES); + failures++; + } + else + printf("[SUCCESS] driver returned 9 images, clamped to %u, no OOB write\n", + ctx->num_swapchain_images); + + free(ctx); +} + +/* Probe: pathological driver returns 64 images. Stress case + * for the post-fix clamp -- pre-fix would be a 56-slot OOB write + * (64 - 8) into adjacent heap memory. Post-fix, count becomes 8. */ +static void test_driver_returns_many_images(void) +{ + struct mock_vk_context *ctx = (struct mock_vk_context *) + calloc(1, sizeof(*ctx)); + struct mock_driver drv = { 64, (void*)0xABCD }; + + if (!ctx) { printf("[ERROR] calloc\n"); failures++; return; } + + run_postfix_sequence(ctx, &drv); + + if (ctx->num_swapchain_images != VULKAN_MAX_SWAPCHAIN_IMAGES) + { + printf("[ERROR] 64-image driver: count=%u, expected clamped to %u\n", + ctx->num_swapchain_images, + (unsigned)VULKAN_MAX_SWAPCHAIN_IMAGES); + failures++; + } + else + printf("[SUCCESS] driver returned 64 images, clamped to %u, no large OOB\n", + ctx->num_swapchain_images); + + free(ctx); +} + +/* Probe: request-side cap. cap_desired_request takes a + * user-configured value (settings->uints.video_max_swapchain_ + * images) and clips it to the array capacity. Verify the + * boundary behaviour. */ +static void test_request_cap(void) +{ + struct { + uint32_t input; + uint32_t expected; + const char *desc; + } cases[] = { + { 1, 1, "input=1 (under cap)" }, + { 3, 3, "input=3 (typical)" }, + { 8, 8, "input=8 (at cap)" }, + { 9, 8, "input=9 (over cap)" }, + { 64, 8, "input=64 (way over)" }, + { (uint32_t)-1, 8, "input=UINT32_MAX (extreme)" } + }; + const unsigned n_cases = sizeof(cases) / sizeof(cases[0]); + unsigned i; + bool any_fail = false; + + for (i = 0; i < n_cases; i++) + { + uint32_t got = cap_desired_request(cases[i].input); + if (got != cases[i].expected) + { + printf("[ERROR] cap_desired_request(%s): got %u, expected %u\n", + cases[i].desc, got, cases[i].expected); + failures++; + any_fail = true; + } + } + if (!any_fail) + printf("[SUCCESS] cap_desired_request: all %u cases clamp correctly\n", + n_cases); +} + +int main(void) +{ + test_well_behaved_three_images(); + test_at_capacity_boundary(); + test_driver_returns_nine_images(); + test_driver_returns_many_images(); + test_request_cap(); + + if (failures) + { + printf("\n%d vulkan_swapchain_clamp test(s) failed\n", failures); + return 1; + } + printf("\nAll vulkan_swapchain_clamp regression tests passed.\n"); + return 0; +} diff --git a/samples/gfx/vulkan_texture_size/Makefile b/samples/gfx/vulkan_texture_size/Makefile new file mode 100644 index 000000000000..ca6bdfb9765e --- /dev/null +++ b/samples/gfx/vulkan_texture_size/Makefile @@ -0,0 +1,25 @@ +TARGET := vulkan_texture_size_test + +SOURCES := vulkan_texture_size_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/vulkan_texture_size/vulkan_texture_size_test.c b/samples/gfx/vulkan_texture_size/vulkan_texture_size_test.c new file mode 100644 index 000000000000..47e0d507747f --- /dev/null +++ b/samples/gfx/vulkan_texture_size/vulkan_texture_size_test.c @@ -0,0 +1,355 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (vulkan_texture_size_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the 32-bit-overflow fix in + * gfx/drivers/vulkan.c::vulkan_create_texture's staging-buffer + * sizing. + * + * Pre-fix the buffer-size calculation read: + * + * uint32_t buffer_width; + * buffer_width = width * vulkan_format_to_bpp(format); + * buffer_width = (buffer_width + 3u) & ~3u; + * buffer_info.size = buffer_width * height; + * + * Both multiplications are unsigned*unsigned in 32-bit, before + * the result is implicitly widened to VkDeviceSize (uint64_t) + * on the assignment to buffer_info.size. With dimensions large + * enough to wrap (e.g. width=65536, height=16385, bpp=4 -> + * 0x1_0004_0000 truncates to 0x40000), the staging buffer was + * allocated at the wrapped (small) size while the per-row + * upload memcpy loop later in the same function walked the + * full width x height. The mapped region was smaller than + * what the loop wrote, so the memcpy walked past the mapping + * into adjacent heap memory. + * + * Reachable from libretro cores supplying oversized + * retro_framebuffer dimensions (vulkan_get_current_sw_framebuffer + * passes through to vulkan_create_texture), and from + * vulkan_load_texture / vulkan_set_texture_frame, which take + * dimensions originating in image decoders. Particularly + * relevant on 32-bit platforms (3DS, Vita, PSP, Wii, Wii U, + * older Android, 32-bit Windows) where the implicit widening + * to size_t at the next stage doesn't help either. + * + * Fix: compute the staging-buffer size in 64-bit, and widen + * the upload loop's stride and per-row copy size to size_t. + * + * IMPORTANT: this test keeps verbatim copies of the post-fix + * size calculation and upload-loop arithmetic from + * gfx/drivers/vulkan.c. If vulkan_create_texture amends them, + * the copies below must follow. Convention used by the + * security regression tests in samples/tasks/. + */ + +#include +#include +#include +#include +#include + +/* Minimal mirror of the bpp lookup from vulkan.c. Only the + * formats actually used by the production path matter. */ +static unsigned vulkan_format_to_bpp_mock(unsigned format_id) +{ + switch (format_id) + { + case 0: return 4; /* B8G8R8A8_UNORM */ + case 1: return 2; /* R5G6B5_UNORM_PACK16 */ + case 2: return 2; /* B4G4R4A4 etc. */ + case 3: return 1; /* R8_UNORM */ + default: return 0; + } +} + +/* === verbatim copy of the post-fix size calculation from + * gfx/drivers/vulkan.c::vulkan_create_texture. If the + * production function amends this arithmetic, the copy + * must follow. === */ +static uint64_t compute_buffer_size(unsigned width, unsigned height, + unsigned format_id) +{ + uint64_t buffer_size_64; + uint32_t buffer_width; + + buffer_width = width * vulkan_format_to_bpp_mock(format_id); + buffer_width = (buffer_width + 3u) & ~3u; + buffer_size_64 = (uint64_t)buffer_width * (uint64_t)height; + + return buffer_size_64; +} +/* === end verbatim copy === */ + +/* === verbatim copy of the post-fix upload-loop size calculations + * from vulkan_create_texture, the STREAMED/STAGING case. The + * production loop is: + * size_t stride = (size_t)tex.width * (size_t)bpp; + * size_t row_bytes = (size_t)width * (size_t)bpp; + * for (y = 0; y < tex.height; ...; src += stride) + * memcpy(dst, src, row_bytes); + * If vulkan.c amends, this copy must follow. === */ +static void compute_upload_strides(unsigned tex_width, unsigned width, + unsigned bpp, size_t *stride_out, size_t *row_bytes_out) +{ + *stride_out = (size_t)tex_width * (size_t)bpp; + *row_bytes_out = (size_t)width * (size_t)bpp; +} +/* === end verbatim copy === */ + +static int failures = 0; + +/* Probe: typical small dimensions, no risk of any overflow. + * Verifies the post-fix arithmetic agrees with the obvious + * calculation. */ +static void test_typical_dimensions(void) +{ + unsigned w = 320, h = 240, fmt = 0; /* RGBA8888 */ + uint64_t got = compute_buffer_size(w, h, fmt); + uint64_t expected = (uint64_t)((w * 4 + 3) & ~3u) * (uint64_t)h; + + if (got != expected) + { + printf("[ERROR] typical 320x240: got %llu, expected %llu\n", + (unsigned long long)got, (unsigned long long)expected); + failures++; + } + else + printf("[SUCCESS] typical 320x240: size=%llu bytes\n", + (unsigned long long)got); +} + +/* Probe: 4096x2160 (4K) RGBA8888 -- common modern resolution. + * Result is 35 MiB-ish, well below 32-bit wrap, but exercises + * a realistically-large legitimate case to confirm the cast + * widening doesn't reject anything that should pass. */ +static void test_4k_rgba(void) +{ + unsigned w = 4096, h = 2160, fmt = 0; + uint64_t got = compute_buffer_size(w, h, fmt); + uint64_t expected = (uint64_t)((w * 4 + 3) & ~3u) * (uint64_t)h; + + if (got != expected) + { + printf("[ERROR] 4K RGBA: got %llu, expected %llu\n", + (unsigned long long)got, (unsigned long long)expected); + failures++; + } + else + printf("[SUCCESS] 4K RGBA 4096x2160: size=%llu bytes (%.1f MiB)\n", + (unsigned long long)got, (double)got / (1024.0 * 1024.0)); +} + +/* Probe: dimensions that wrap a 32-bit unsigned multiplication. + * width=65536, height=16385, bpp=4: 65536*4 = 262144 (fine); + * 262144*16385 = 0x1_0004_0000 (33 bits). Pre-fix this + * truncated to 0x40000 (256 KiB). Post-fix it stays + * 0x100040000 = 4_295_229_440. This is the smoking-gun test: + * verify the result exceeds UINT32_MAX. */ +static void test_overflow_wrap_case(void) +{ + const unsigned w = 65536, h = 16385, fmt = 0; + const uint32_t prefix_buffer_width = (w * 4 + 3) & ~3u; + /* What pre-fix arithmetic would have produced (kept locally + * for the assertion only -- never used to size anything). */ + const uint32_t prefix_size_truncated = prefix_buffer_width * h; + const uint64_t expected_full = (uint64_t)prefix_buffer_width * (uint64_t)h; + + uint64_t got = compute_buffer_size(w, h, fmt); + + if (got != expected_full) + { + printf("[ERROR] 65536x16385: got %llu, expected %llu\n", + (unsigned long long)got, + (unsigned long long)expected_full); + failures++; + return; + } + if (got <= UINT32_MAX) + { + printf("[ERROR] 65536x16385 should produce %llu (>UINT32_MAX), got %llu\n", + (unsigned long long)expected_full, + (unsigned long long)got); + failures++; + return; + } + + printf("[SUCCESS] 65536x16385x4 = %llu bytes (pre-fix would truncate to %u)\n", + (unsigned long long)got, prefix_size_truncated); +} + +/* Probe: multiple wrap-triggering shapes. Each pair has + * w*bpp*h > UINT32_MAX. */ +static void test_assorted_overflow_shapes(void) +{ + struct { + unsigned w, h, fmt; + const char *desc; + } cases[] = { + { 65536, 16385, 0, "65536x16385 RGBA8888" }, + { 32768, 32769, 0, "32768x32769 RGBA8888" }, + { 65536, 65535, 1, "65536x65535 RGB565 (2bpp)" }, + { 65536, 65536, 0, "65536x65536 RGBA8888 (16 GiB)" } + }; + const unsigned n = sizeof(cases) / sizeof(cases[0]); + unsigned i; + bool any_fail = false; + + for (i = 0; i < n; i++) + { + const uint32_t bpp = vulkan_format_to_bpp_mock(cases[i].fmt); + const uint32_t bw = (cases[i].w * bpp + 3) & ~3u; + const uint64_t expected = (uint64_t)bw * (uint64_t)cases[i].h; + uint64_t got = compute_buffer_size(cases[i].w, cases[i].h, cases[i].fmt); + + if (got != expected) + { + printf("[ERROR] %s: got %llu, expected %llu\n", + cases[i].desc, + (unsigned long long)got, + (unsigned long long)expected); + failures++; + any_fail = true; + } + } + if (!any_fail) + printf("[SUCCESS] all %u overflow shapes computed correctly in 64-bit\n", + n); +} + +/* Probe: per-row upload arithmetic. Mimics the inner loop's + * memcpy length and source-stride calculation under an + * AddressSanitizer-instrumented allocation that's exactly the + * computed buffer size. + * + * If the post-fix `(size_t)w * (size_t)bpp` is reverted to + * `unsigned w * unsigned bpp`, the row_bytes value would wrap + * to a small number, the memcpy would copy too little, and the + * per-row pointer math (dst += stride) would also wrap. Test + * with a moderately large but not absurd shape to keep the + * malloc tractable. */ +static void test_upload_loop_strides(void) +{ + /* 2048x16 RGBA8888 -- 128 KiB total, easy to alloc. + * Each row is 8192 bytes, 16 rows. */ + const unsigned w = 2048, h = 16; + const unsigned bpp = 4; + const size_t expected_stride = (size_t)w * (size_t)bpp; + const size_t total_size = expected_stride * (size_t)h; + size_t stride, row_bytes; + uint8_t *buf; + uint8_t *src; + unsigned y; + + compute_upload_strides(w, w, bpp, &stride, &row_bytes); + + if (stride != expected_stride || row_bytes != expected_stride) + { + printf("[ERROR] upload strides: stride=%zu row_bytes=%zu, expected %zu\n", + stride, row_bytes, expected_stride); + failures++; + return; + } + + /* Allocate destination + source at the computed size. ASan + * instruments both; the per-row memcpy walks every byte in + * the destination and reads every byte in the source. */ + buf = (uint8_t*)malloc(total_size); + src = (uint8_t*)malloc(total_size); + if (!buf || !src) + { + printf("[ERROR] malloc failed (size=%zu)\n", total_size); + free(buf); free(src); + failures++; + return; + } + + memset(src, 0xA5, total_size); + memset(buf, 0, total_size); + + /* Mirror the production upload loop. */ + { + uint8_t *dst = buf; + const uint8_t *srcptr = src; + for (y = 0; y < h; y++, dst += stride, srcptr += stride) + memcpy(dst, srcptr, row_bytes); + } + + /* Spot-check first and last byte and each row boundary -- + * if the pointer math wrapped, these would be wrong. */ + if ( buf[0] != 0xA5 + || buf[total_size - 1] != 0xA5 + || buf[(h - 1) * expected_stride] != 0xA5 + || buf[(h - 1) * expected_stride + 1] != 0xA5) + { + printf("[ERROR] upload loop produced inconsistent buffer contents\n"); + failures++; + } + else + printf("[SUCCESS] upload loop: %u rows of %zu bytes, no OOB writes\n", + h, row_bytes); + + free(buf); + free(src); +} + +/* Probe: confirm the harness arithmetic itself isn't silently + * collapsing to 32-bit. 16 GiB result expected. */ +static void test_size_holds_full_64bit(void) +{ + uint64_t got = compute_buffer_size(65536, 65536, 0); + uint64_t expected = (uint64_t)((65536u * 4u + 3u) & ~3u) * 65536ull; + if (got != expected) + { + printf("[ERROR] 65536x65536 RGBA: got %llu, harness expected %llu\n", + (unsigned long long)got, + (unsigned long long)expected); + failures++; + return; + } + /* 65536 * 4 = 262144, * 65536 = 17_179_869_184 = 16 GiB. */ + if (got != 17179869184ull) + { + printf("[ERROR] 65536x65536 RGBA: expected 16 GiB exactly, got %llu\n", + (unsigned long long)got); + failures++; + return; + } + printf("[SUCCESS] size holds full 64-bit: 65536x65536 = 16 GiB\n"); +} + +int main(void) +{ + test_typical_dimensions(); + test_4k_rgba(); + test_overflow_wrap_case(); + test_assorted_overflow_shapes(); + test_upload_loop_strides(); + test_size_holds_full_64bit(); + + if (failures) + { + printf("\n%d vulkan_texture_size test(s) failed\n", failures); + return 1; + } + printf("\nAll vulkan_texture_size regression tests passed.\n"); + return 0; +} From 29080418c3bdb848425e60f432761857feeaf7e7 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 14:41:22 +0200 Subject: [PATCH 03/17] gfx/drivers_shader: cap arrayed texture-semantic indices in slang + CI Found during a continued audit of gfx/drivers_shader/shader_vulkan.cpp and slang_process.cpp after the v3 vulkan-driver fixes (da5e650). Adds an AddressSanitizer regression test under samples/gfx/ slang_texture_index_bounds/ wired into the existing Linux-samples-gfx CI workflow as a fourth step. slang_process.cpp's add_active_buffer_ranges() and the direct sampler- binding loop in slang_reflect() both consume an unsigned array_index parsed from the suffix of arrayed semantic names like `OriginalHistory42` / `PassFeedback9` / `User7`. The parsing happens in slang_name_to_texture_semantic_array() via strtoul with no upper bound. That index then flows into resize_minimum() at slang_process.c line 1116 and the direct-binding write at line 1701, growing reflection->semantic_textures[sem] to (index + 1) entries. PASS_OUTPUT was already bounded against reflection->pass_number, with a dedicated "Non causal filter chain detected" diagnostic; ORIGINAL_HISTORY, PASS_FEEDBACK and USER had no upper bound. A malicious slang shader declaring layout(set = 0, binding = 1) uniform sampler2D OriginalHistory4294967294; makes std::vector::resize() request ~128 GiB on a 64-bit host, which throws std::bad_alloc. The exception propagates out of slang_reflect_spirv() up to vulkan_filter_chain_create_from_preset() with no try/catch in between, and the unhandled C++ exception terminates RetroArch. Reachable from any slang preset -- preset packs are downloaded via the Online Updater and shipped third-party. Severity is DoS rather than memory corruption, but the threat class mirrors the recently-fixed .bsv replay file, .bps soft-patch, and image-decoder bugs in 7335b37: a downloadable file that an end user opens, leading to a guaranteed process termination. * gfx/drivers_shader/slang_process.cpp Lift the existing PASS_OUTPUT bound check into a small helper validate_texture_semantic_index() and extend it to cover all arrayed semantics with their natural caps: PASS_OUTPUT -> reflection->pass_number (unchanged) PASS_FEEDBACK -> GFX_MAX_SHADERS (64) ORIGINAL_HISTORY -> GFX_MAX_FRAME_HISTORY (128) USER -> GFX_MAX_TEXTURES (64) Non-arrayed semantics (Original, Source) carry index 0 by construction in slang_name_to_texture_semantic_array() and accept unconditionally. PASS_OUTPUT keeps its dedicated "Non causal filter chain detected" log verbiage so existing shader-author muscle memory and log-grepping scripts continue to work; the other semantics use a uniform "Texture semantic index #N exceeds bound ( = )" message that names both the offending index and the cap that rejected it. The helper is called from both call sites that lead into a resize_minimum(): - add_active_buffer_ranges() (UBO uniform path), replacing the inline PASS_OUTPUT-only check. - slang_reflect()'s direct sampler-binding loop (the SLANG_ INVALID_TEXTURE_SEMANTIC short-circuit is preserved). * samples/gfx/slang_texture_index_bounds/, .github/workflows/Linux-samples-gfx.yml Regression test following the same convention as the v3 vulkan/ tests in this directory and the security regression tests under samples/tasks/. Keeps a verbatim copy of validate_texture_semantic_index() demarcated by `=== verbatim copy ===` markers, with an `IMPORTANT: ... must follow` comment. Plain C, asserts manually, exits nonzero on failure. 21 cases across six probe groups: legitimate-in-bounds across all four arrayed semantics, at-cap rejections, the smoking-gun UINT32_MAX-1 indices on each path including the previously- unbounded ORIGINAL_HISTORY/PASS_FEEDBACK/USER ones, log-dispatch consistency, non-arrayed (ORIGINAL/SOURCE) accept-everything, and the PASS_OUTPUT-in-pass-0 reject-everything case. Verified pre/post-patch discrimination: under the pre-fix PASS_OUTPUT-only shape the 7 cases that exercise the new bounds fail (3 boundary + 3 smoking-gun + 1 ORIGINAL_HISTORY at cap); PASS_OUTPUT cases continue to pass. Wires into the existing Linux-samples-gfx.yml workflow as a fourth step, modeled on the three vulkan/ steps from da5e650 with the same per-step explanation of what the test is regression-testing and the "verbatim copy must follow" reminder. --- .github/workflows/Linux-samples-gfx.yml | 32 ++ gfx/drivers_shader/slang_process.cpp | 90 ++++- .../gfx/slang_texture_index_bounds/Makefile | 25 ++ .../slang_texture_index_bounds_test.c | 356 ++++++++++++++++++ 4 files changed, 485 insertions(+), 18 deletions(-) create mode 100644 samples/gfx/slang_texture_index_bounds/Makefile create mode 100644 samples/gfx/slang_texture_index_bounds/slang_texture_index_bounds_test.c diff --git a/.github/workflows/Linux-samples-gfx.yml b/.github/workflows/Linux-samples-gfx.yml index 251811ef4bf0..de6a52bba38f 100644 --- a/.github/workflows/Linux-samples-gfx.yml +++ b/.github/workflows/Linux-samples-gfx.yml @@ -107,3 +107,35 @@ jobs: test -x vulkan_texture_size_test timeout 60 ./vulkan_texture_size_test echo "[pass] vulkan_texture_size_test" + + - name: Build and run slang_texture_index_bounds_test (ASan) + shell: bash + working-directory: samples/gfx/slang_texture_index_bounds + run: | + set -eu + # Regression test for the texture-semantic index-bounds + # fix in gfx/drivers_shader/slang_process.cpp:: + # validate_texture_semantic_index(). Pre-fix only + # SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT was bounded against + # reflection->pass_number; ORIGINAL_HISTORY, PASS_FEEDBACK + # and USER had no upper bound on their array index. The + # index suffix in arrayed semantic names like + # `OriginalHistory42` is parsed via strtoul in + # slang_name_to_texture_semantic_array() and propagates + # into the downstream resize_minimum() calls in + # set_ubo_texture_offset() and the direct sampler-binding + # loop. A malicious slang shader declaring + # `OriginalHistory4294967294` makes std::vector::resize + # request ~128 GiB, throwing std::bad_alloc which + # propagates unhandled out of the filter-chain create + # path, terminating the process. Reachable from any + # malicious slang preset (downloaded via Online Updater + # or shipped third-party). Build under AddressSanitizer + # so any reintroduction is caught at the bounds level. + # If slang_process.cpp amends the cap table or the + # dispatch structure, the verbatim copy in + # slang_texture_index_bounds_test.c must follow. + make clean all SANITIZER=address + test -x slang_texture_index_bounds_test + timeout 60 ./slang_texture_index_bounds_test + echo "[pass] slang_texture_index_bounds_test" diff --git a/gfx/drivers_shader/slang_process.cpp b/gfx/drivers_shader/slang_process.cpp index c0205115d319..8d86c40ffaf0 100644 --- a/gfx/drivers_shader/slang_process.cpp +++ b/gfx/drivers_shader/slang_process.cpp @@ -1334,6 +1334,71 @@ static bool validate_type_for_texture_semantic(const spirv_cross::SPIRType &type && (type.columns == 1); } +/* Validate that a texture semantic's array index falls inside the + * range admitted by the host filter chain. The shader source is + * untrusted (preset packs are downloadable / shipped third-party), + * and the index suffix in arrayed semantic names like + * `OriginalHistory42` / `PassFeedback9` / `User7` is parsed via + * strtoul in slang_name_to_texture_semantic_array() with no upper + * bound. The downstream resize_minimum() call in + * set_ubo_texture_offset() and the direct-binding loop below would + * otherwise grow reflection->semantic_textures[sem] to the index+1 + * the shader requested -- which on a malicious preset can be near + * UINT32_MAX, producing an unhandled std::bad_alloc that terminates + * RetroArch. PASS_OUTPUT was already bounded against + * reflection->pass_number; extend the same defensive shape to the + * other arrayed semantics with their natural caps. USER is the + * lookup name for LUTs, ORIGINAL_HISTORY is bounded by the per- + * frame ring used by init_history(), and PASS_FEEDBACK is bounded + * by the maximum number of passes the chain can hold. */ +static bool validate_texture_semantic_index(slang_reflection *reflection, + slang_texture_semantic tex_sem, unsigned index) +{ + unsigned cap = 0; + const char *cap_label = NULL; + + switch (tex_sem) + { + case SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT: + cap = reflection->pass_number; + cap_label = "preceding passes"; + break; + case SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK: + cap = GFX_MAX_SHADERS; + cap_label = "GFX_MAX_SHADERS"; + break; + case SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY: + cap = GFX_MAX_FRAME_HISTORY; + cap_label = "GFX_MAX_FRAME_HISTORY"; + break; + case SLANG_TEXTURE_SEMANTIC_USER: + cap = GFX_MAX_TEXTURES; + cap_label = "GFX_MAX_TEXTURES"; + break; + default: + /* Non-arrayed semantics (Original, Source) -- index is + * always 0 by construction in slang_name_to_texture_ + * semantic_array(). */ + return true; + } + + if (index >= cap) + { + if (tex_sem == SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT) + RARCH_ERR("[Slang] Non causal filter chain detected. " + "Shader is trying to use output from pass #%u," + " but this shader is pass #%u.\n", + index, reflection->pass_number); + else + RARCH_ERR("[Slang] Texture semantic %s index #%u exceeds" + " bound (%s = %u).\n", + texture_semantic_names[tex_sem], + index, cap_label, cap); + return false; + } + return true; +} + static bool add_active_buffer_ranges( const spirv_cross::Compiler &compiler, const spirv_cross::Resource &resource, @@ -1360,15 +1425,10 @@ static bool add_active_buffer_ranges( *reflection->texture_semantic_uniform_map, name, &tex_sem_index); - if (tex_sem == SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT && - tex_sem_index >= reflection->pass_number) - { - RARCH_ERR("[Slang] Non causal filter chain detected. " - "Shader is trying to use output from pass #%u," - " but this shader is pass #%u.\n", - tex_sem_index, reflection->pass_number); + if (tex_sem != SLANG_INVALID_TEXTURE_SEMANTIC && + !validate_texture_semantic_index(reflection, + tex_sem, tex_sem_index)) return false; - } if (sem != SLANG_INVALID_SEMANTIC) { @@ -1680,16 +1740,7 @@ bool slang_reflect( *reflection->texture_semantic_map, fragment.sampled_images[i].name, &array_index); - if (index == SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT && - array_index >= reflection->pass_number) - { - RARCH_ERR("[Slang] Non causal filter chain detected. " - "Shader is trying to use output from pass #%u," - " but this shader is pass #%u.\n", - array_index, reflection->pass_number); - return false; - } - else if (index == SLANG_INVALID_TEXTURE_SEMANTIC) + if (index == SLANG_INVALID_TEXTURE_SEMANTIC) { RARCH_ERR("[Slang] Texture name '%s' not found in semantic map, " "Probably the texture name or pass alias is not defined " @@ -1698,6 +1749,9 @@ bool slang_reflect( return false; } + if (!validate_texture_semantic_index(reflection, index, array_index)) + return false; + resize_minimum(reflection->semantic_textures[index], array_index + 1); slang_texture_semantic_meta &semantic = reflection->semantic_textures[index][array_index]; diff --git a/samples/gfx/slang_texture_index_bounds/Makefile b/samples/gfx/slang_texture_index_bounds/Makefile new file mode 100644 index 000000000000..28decf60aea7 --- /dev/null +++ b/samples/gfx/slang_texture_index_bounds/Makefile @@ -0,0 +1,25 @@ +TARGET := slang_texture_index_bounds_test + +SOURCES := slang_texture_index_bounds_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/slang_texture_index_bounds/slang_texture_index_bounds_test.c b/samples/gfx/slang_texture_index_bounds/slang_texture_index_bounds_test.c new file mode 100644 index 000000000000..dc98e71bf590 --- /dev/null +++ b/samples/gfx/slang_texture_index_bounds/slang_texture_index_bounds_test.c @@ -0,0 +1,356 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (slang_texture_index_bounds_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the texture-semantic index-bounds fix in + * gfx/drivers_shader/slang_process.cpp:: + * validate_texture_semantic_index(). + * + * Pre-fix only the SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT semantic was + * bounded against reflection->pass_number; the other arrayed + * semantics (ORIGINAL_HISTORY, PASS_FEEDBACK, USER) had no upper + * bound on their array index. The index suffix in arrayed + * semantic names like `OriginalHistory42` / `PassFeedback9` / + * `User7` is parsed via strtoul in + * slang_name_to_texture_semantic_array() and propagates into the + * downstream resize_minimum() at slang_process.cpp:1116 (UBO + * uniform path) and the direct-binding loop in slang_reflect() + * (sampler binding path). A malicious slang shader declaring + * + * layout(set = 0, binding = 1) uniform sampler2D + * OriginalHistory4294967294; + * + * makes resize_minimum() call std::vector::resize(4294967295). + * With slang_texture_semantic_meta at ~32 bytes, that is ~128 GiB + * which std::vector throws std::bad_alloc on. The exception + * propagates out of slang_reflect_spirv() up to + * vulkan_filter_chain_create_from_preset() with no try/catch, and + * the unhandled C++ exception terminates RetroArch. + * + * Reachability: any malicious slang shader preset. Slang preset + * packs are downloaded from the Online Updater and shipped third- + * party; this is the same threat surface as the .bsv replay file, + * .bps soft-patch, and image-decoder bugs in 7335b37. + * + * Fix: extend the existing PASS_OUTPUT bound check to cover all + * arrayed semantics, with their natural caps: + * PASS_OUTPUT -> reflection->pass_number (unchanged) + * PASS_FEEDBACK -> GFX_MAX_SHADERS (64) + * ORIGINAL_HISTORY-> GFX_MAX_FRAME_HISTORY (128) + * USER -> GFX_MAX_TEXTURES (64) + * Non-arrayed semantics (Original, Source) carry index 0 by + * construction and are accepted unconditionally. + * + * IMPORTANT: this test keeps a verbatim copy of + * validate_texture_semantic_index() from slang_process.cpp. If + * the production function amends the cap table or the dispatch + * structure, the copy below must follow. Convention used by the + * security regression tests in samples/tasks/ and the v3 vulkan/ + * tests in this directory. + */ + +#include +#include +#include +#include +#include + +/* Mirror the production semantic enum. Order matters because + * the production texture_semantic_names[] array is indexed by + * enum value and the message uses it. If + * gfx/drivers_shader/glslang.hpp amends the enum, this mirror + * must follow. */ +enum slang_texture_semantic +{ + SLANG_TEXTURE_SEMANTIC_ORIGINAL = 0, + SLANG_TEXTURE_SEMANTIC_SOURCE, + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, + SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK, + SLANG_TEXTURE_SEMANTIC_USER, + SLANG_INVALID_TEXTURE_SEMANTIC = -1 +}; + +/* Mirror the production caps from gfx/video_shader_parse.h. + * If those macros change, this mirror must follow. */ +#define GFX_MAX_SHADERS 64 +#define GFX_MAX_TEXTURES 64 +#define GFX_MAX_FRAME_HISTORY 128 + +/* Minimal stand-in for slang_reflection. Production has a much + * larger struct; the validator only consults pass_number. */ +struct mock_reflection +{ + unsigned pass_number; +}; + +/* Mirror of the production texture_semantic_names array, used by + * the validator's error log. If glslang_util.c amends the order + * or contents, this mirror must follow. */ +static const char *texture_semantic_names[] = { + "Original", + "Source", + "OriginalHistory", + "PassOutput", + "PassFeedback", + "User", + NULL +}; + +/* Test-side stand-in for RARCH_ERR. Production logs to the + * verbosity layer; we route to stderr so the verbatim copy + * still exercises its varargs (and the texture_semantic_names + * pointer dereference is observed under ASan), while a flag + * tracks whether a rejection log fired this call. */ +static int last_log_was_rejection = 0; +#define RARCH_ERR(...) do { \ + last_log_was_rejection = 1; \ + fprintf(stderr, " [log] "); \ + fprintf(stderr, __VA_ARGS__); \ +} while (0) + +/* === verbatim copy of the post-fix validate_texture_semantic_index + * from gfx/drivers_shader/slang_process.cpp. If the production + * function amends the cap table or the dispatch structure, this + * copy must follow. === */ +static bool validate_texture_semantic_index(struct mock_reflection *reflection, + enum slang_texture_semantic tex_sem, unsigned index) +{ + unsigned cap = 0; + const char *cap_label = NULL; + + switch (tex_sem) + { + case SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT: + cap = reflection->pass_number; + cap_label = "preceding passes"; + break; + case SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK: + cap = GFX_MAX_SHADERS; + cap_label = "GFX_MAX_SHADERS"; + break; + case SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY: + cap = GFX_MAX_FRAME_HISTORY; + cap_label = "GFX_MAX_FRAME_HISTORY"; + break; + case SLANG_TEXTURE_SEMANTIC_USER: + cap = GFX_MAX_TEXTURES; + cap_label = "GFX_MAX_TEXTURES"; + break; + default: + /* Non-arrayed semantics (Original, Source) -- index is + * always 0 by construction in slang_name_to_texture_ + * semantic_array(). */ + return true; + } + + if (index >= cap) + { + if (tex_sem == SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT) + RARCH_ERR("[Slang] Non causal filter chain detected. " + "Shader is trying to use output from pass #%u," + " but this shader is pass #%u.\n", + index, reflection->pass_number); + else + RARCH_ERR("[Slang] Texture semantic %s index #%u exceeds" + " bound (%s = %u).\n", + texture_semantic_names[tex_sem], + index, cap_label, cap); + return false; + } + return true; +} +/* === end verbatim copy === */ + +static int failures = 0; + +/* Helper: run the predicate, optionally checking that the + * rejection-log was tripped (meaning production would have + * RARCH_ERR'd). */ +static void run_case(const char *desc, + enum slang_texture_semantic sem, unsigned pass_number, + unsigned index, bool expect_accept) +{ + struct mock_reflection refl = { pass_number }; + bool rv; + + last_log_was_rejection = 0; + rv = validate_texture_semantic_index(&refl, sem, index); + + if (rv != expect_accept) + { + printf("[ERROR] %s: got %s, expected %s\n", + desc, + rv ? "accept" : "reject", + expect_accept ? "accept" : "reject"); + failures++; + return; + } + if (!expect_accept && !last_log_was_rejection) + { + printf("[ERROR] %s: rejected but no log line emitted\n", desc); + failures++; + return; + } + if (expect_accept && last_log_was_rejection) + { + printf("[ERROR] %s: accepted but log line emitted\n", desc); + failures++; + return; + } + printf("[SUCCESS] %s: %s as expected\n", + desc, expect_accept ? "accepted" : "rejected"); +} + +/* Probe: legitimate uses across all four arrayed semantics. + * Each well within its respective cap. */ +static void test_legitimate_in_bounds(void) +{ + /* Pass 5 in a 6-pass chain reads PassOutput0..4. */ + run_case("PASS_OUTPUT 0 in pass 5", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 5, 0, true); + run_case("PASS_OUTPUT 4 in pass 5 (boundary minus one)", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 5, 4, true); + + /* History rings of size 7 (typical CRT shader). */ + run_case("ORIGINAL_HISTORY 6", + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, 0, 6, true); + run_case("ORIGINAL_HISTORY GFX_MAX_FRAME_HISTORY-1", + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, 0, + GFX_MAX_FRAME_HISTORY - 1, true); + + /* Feedback from earlier pass. */ + run_case("PASS_FEEDBACK 3", + SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK, 0, 3, true); + run_case("PASS_FEEDBACK GFX_MAX_SHADERS-1", + SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK, 0, + GFX_MAX_SHADERS - 1, true); + + /* User LUT 0..N. */ + run_case("USER 0", + SLANG_TEXTURE_SEMANTIC_USER, 0, 0, true); + run_case("USER GFX_MAX_TEXTURES-1", + SLANG_TEXTURE_SEMANTIC_USER, 0, + GFX_MAX_TEXTURES - 1, true); +} + +/* Probe: index exactly equal to the cap on each semantic -- + * boundary that should reject. */ +static void test_at_boundary_rejected(void) +{ + run_case("PASS_OUTPUT 5 in pass 5 (== cap)", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 5, 5, false); + run_case("ORIGINAL_HISTORY GFX_MAX_FRAME_HISTORY", + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, 0, + GFX_MAX_FRAME_HISTORY, false); + run_case("PASS_FEEDBACK GFX_MAX_SHADERS", + SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK, 0, + GFX_MAX_SHADERS, false); + run_case("USER GFX_MAX_TEXTURES", + SLANG_TEXTURE_SEMANTIC_USER, 0, + GFX_MAX_TEXTURES, false); +} + +/* Probe: the smoking-gun cases. Pre-fix these would have called + * std::vector::resize(index+1) on the production reflection. + * Index = 4_294_967_294 implies a 128 GiB allocation request. */ +static void test_smoking_gun_uint32_max_minus_one(void) +{ + run_case("ORIGINAL_HISTORY UINT32_MAX-1 (smoking gun)", + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, 0, + UINT32_MAX - 1u, false); + run_case("PASS_FEEDBACK UINT32_MAX-1 (smoking gun)", + SLANG_TEXTURE_SEMANTIC_PASS_FEEDBACK, 0, + UINT32_MAX - 1u, false); + run_case("USER UINT32_MAX-1 (smoking gun)", + SLANG_TEXTURE_SEMANTIC_USER, 0, + UINT32_MAX - 1u, false); + run_case("PASS_OUTPUT UINT32_MAX-1 (existing PASS_OUTPUT path)", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 5, + UINT32_MAX - 1u, false); +} + +/* Probe: PASS_OUTPUT keeps its dedicated `Non causal filter chain + * detected` error message verbiage. The other semantics use the + * generic per-cap message. Smoke-test that the dispatch fires + * the right log path by checking the pre-existing PASS_OUTPUT + * behaviour against an in-range index (accept) and an out-of- + * range one (reject), then the same for ORIGINAL_HISTORY -- in + * both cases the rejection-log flag should match the predicate + * return value. */ +static void test_log_dispatch_consistency(void) +{ + /* PASS_OUTPUT with pass_number=0: any index rejects. */ + run_case("PASS_OUTPUT 0 in pass 0 (cap=0)", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 0, 0, false); + /* ORIGINAL_HISTORY at exactly the cap. */ + run_case("ORIGINAL_HISTORY at cap", + SLANG_TEXTURE_SEMANTIC_ORIGINAL_HISTORY, 0, + GFX_MAX_FRAME_HISTORY, false); +} + +/* Probe: non-arrayed semantics (Original, Source) accept any + * index unconditionally. Production sets index=0 for these by + * construction in slang_name_to_texture_semantic_array, but the + * validator must not reject if the caller mistakenly passes a + * non-zero one. */ +static void test_non_arrayed_semantics(void) +{ + run_case("ORIGINAL with index 0", + SLANG_TEXTURE_SEMANTIC_ORIGINAL, 0, 0, true); + run_case("ORIGINAL with index 42 (defensive)", + SLANG_TEXTURE_SEMANTIC_ORIGINAL, 0, 42, true); + run_case("SOURCE with index 0", + SLANG_TEXTURE_SEMANTIC_SOURCE, 0, 0, true); + run_case("SOURCE with index 99999 (defensive)", + SLANG_TEXTURE_SEMANTIC_SOURCE, 0, 99999, true); +} + +/* Probe: PASS_OUTPUT in the very first pass (pass_number = 0) + * rejects every index, including 0. This matches the pre-fix + * predicate `index >= reflection->pass_number` for index=0, + * pass_number=0: 0 >= 0 is true -> reject. Confirms the v4 + * change preserved that semantics. */ +static void test_pass_output_first_pass_rejects(void) +{ + run_case("PASS_OUTPUT 0 in pass 0 (the very first pass)", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 0, 0, false); + run_case("PASS_OUTPUT 1 in pass 0", + SLANG_TEXTURE_SEMANTIC_PASS_OUTPUT, 0, 1, false); +} + +int main(void) +{ + test_legitimate_in_bounds(); + test_at_boundary_rejected(); + test_smoking_gun_uint32_max_minus_one(); + test_log_dispatch_consistency(); + test_non_arrayed_semantics(); + test_pass_output_first_pass_rejects(); + + if (failures) + { + printf("\n%d slang_texture_index_bounds test(s) failed\n", failures); + return 1; + } + printf("\nAll slang_texture_index_bounds regression tests passed.\n"); + return 0; +} From de7ae030aa67eab253d134e61e06a3dba8f76bb2 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 14:56:49 +0200 Subject: [PATCH 04/17] C89_BUILD fixes --- gfx/video_filters/dedither.c | 92 +++++++++++++++++++++----------- gfx/video_filters/pixel_art_aa.c | 70 +++++++++++++++--------- 2 files changed, 105 insertions(+), 57 deletions(-) diff --git a/gfx/video_filters/dedither.c b/gfx/video_filters/dedither.c index 4de7331b9a39..d0c7f0093288 100644 --- a/gfx/video_filters/dedither.c +++ b/gfx/video_filters/dedither.c @@ -1,6 +1,7 @@ #include "softfilter.h" #include #include +#include #ifdef RARCH_INTERNAL #define softfilter_get_implementation dedither_get_implementation @@ -41,25 +42,33 @@ static void *dedither_generic_create(const struct softfilter_config *config, unsigned max_width, unsigned max_height, unsigned threads, softfilter_simd_mask_t simd, void *userdata) { struct filter_data *filt = (struct filter_data*)calloc(1, sizeof(*filt)); - if (!filt) return NULL; + if (!filt) + return NULL; filt->workers = (struct softfilter_thread_data*)calloc(1, sizeof(struct softfilter_thread_data)); filt->threads = 1; filt->in_fmt = in_fmt; return filt; } -static void dedither_generic_destroy(void *data) { +static void dedither_generic_destroy(void *data) +{ struct filter_data *filt = (struct filter_data*)data; - if (filt) { free(filt->workers); free(filt); } + if (filt) + { + free(filt->workers); + free(filt); + } } -static void dedither_generic_output(void *data, unsigned *out_width, unsigned *out_height, - unsigned width, unsigned height) { - *out_width = width; *out_height = height; +static void dedither_generic_output(void *data, unsigned *out_width, unsigned *out_height, unsigned width, unsigned height) +{ + *out_width = width; + *out_height = height; } /* Color comparison with threshold */ -static inline int pix_equal(uint32_t c1, uint32_t c2, int threshold) { +static INLINE int pix_equal(uint32_t c1, uint32_t c2, int threshold) +{ int r = abs((int)((c1 >> 16) & 0xFF) - (int)((c2 >> 16) & 0xFF)); int g = abs((int)((c1 >> 8) & 0xFF) - (int)((c2 >> 8) & 0xFF)); int b = abs((int)(c1 & 0xFF) - (int)(c2 & 0xFF)); @@ -67,7 +76,9 @@ static inline int pix_equal(uint32_t c1, uint32_t c2, int threshold) { } /* XRGB8888 Kernel - 2D Dither Detection (6x6) */ -static void dedither_work_cb_xrgb8888(void *data, void *thread_data) { +static void dedither_work_cb_xrgb8888(void *data, void *thread_data) +{ + uint32_t y, x; struct softfilter_thread_data *thr = (struct softfilter_thread_data*)thread_data; const uint32_t *in = (const uint32_t*)thr->in_data; uint32_t *out = (uint32_t*)thr->out_data; @@ -75,11 +86,13 @@ static void dedither_work_cb_xrgb8888(void *data, void *thread_data) { uint32_t out_stride = (uint32_t)(thr->out_pitch >> 2); const int threshold = 40; - for (uint32_t y = 0; y < thr->height; ++y) { - for (uint32_t x = 0; x < thr->width; ++x) { + for (y = 0; y < thr->height; ++y) + { + for (x = 0; x < thr->width; ++x) + { /* Check safety bounds for 6-pixel horizontal and vertical patterns */ - if (x >= 2 && x < thr->width - 3 && y >= 2 && y < thr->height - 3) { - + if (x >= 2 && x < thr->width - 3 && y >= 2 && y < thr->height - 3) + { const uint32_t *line = in + y * in_stride; /* Horizontal samples (Row y) */ @@ -104,7 +117,8 @@ static void dedither_work_cb_xrgb8888(void *data, void *thread_data) { pix_equal(v2, v4, threshold) && pix_equal(v4, v6, threshold) && !pix_equal(h3, v4, threshold)); - if (h_dit || v_dit) { + if (h_dit || v_dit) + { uint32_t avg; if (h_dit) avg = (((h2 & 0xFEFEFEFE) >> 1) + ((h4 & 0xFEFEFEFE) >> 1)); @@ -113,27 +127,32 @@ static void dedither_work_cb_xrgb8888(void *data, void *thread_data) { /* Apply soft blend (1:2:1 weighting) */ out[y * out_stride + x] = (((avg & 0xFEFEFEFE) >> 1) + ((h3 & 0xFEFEFEFE) >> 1)); - } else { - out[y * out_stride + x] = h3; /* Keep original sharp pixel */ } - } else { - out[y * out_stride + x] = in[y * in_stride + x]; + else + out[y * out_stride + x] = h3; /* Keep original sharp pixel */ } + else + out[y * out_stride + x] = in[y * in_stride + x]; } } } /* RGB565 Kernel - 2D Dither Detection (6x6) */ -static void dedither_work_cb_rgb565(void *data, void *thread_data) { +static void dedither_work_cb_rgb565(void *data, void *thread_data) +{ + uint32_t x, y; struct softfilter_thread_data *thr = (struct softfilter_thread_data*)thread_data; const uint16_t *in = (const uint16_t*)thr->in_data; uint16_t *out = (uint16_t*)thr->out_data; uint16_t in_stride = (uint16_t)(thr->in_pitch >> 1); uint16_t out_stride = (uint16_t)(thr->out_pitch >> 1); - for (uint32_t y = 0; y < thr->height; ++y) { - for (uint32_t x = 0; x < thr->width; ++x) { - if (x >= 2 && x < thr->width - 3 && y >= 2 && y < thr->height - 3) { + for (y = 0; y < thr->height; ++y) + { + for (x = 0; x < thr->width; ++x) + { + if (x >= 2 && x < thr->width - 3 && y >= 2 && y < thr->height - 3) + { const uint16_t *line = in + y * in_stride; uint16_t h1 = line[x - 2]; uint16_t h2 = line[x - 1]; uint16_t h3 = line[x]; uint16_t h4 = line[x + 1]; @@ -146,22 +165,30 @@ static void dedither_work_cb_rgb565(void *data, void *thread_data) { int h_dit = (h1 == h3 && h3 == h5 && h2 == h4 && h4 == h6 && h3 != h4); int v_dit = (v1 == h3 && h3 == v5 && v2 == v4 && v4 == v6 && h3 != v2); - if (h_dit || v_dit) { + if (h_dit || v_dit) + { uint16_t avg_s = (h_dit) ? (((h2 & 0xF7DE) >> 1) + ((h4 & 0xF7DE) >> 1)) : (((v2 & 0xF7DE) >> 1) + ((v4 & 0xF7DE) >> 1)); out[y * out_stride + x] = (((avg_s & 0xF7DE) >> 1) + ((h3 & 0xF7DE) >> 1)); - } else out[y * out_stride + x] = h3; - } else out[y * out_stride + x] = in[y * in_stride + x]; + } + else + out[y * out_stride + x] = h3; + } + else + out[y * out_stride + x] = in[y * in_stride + x]; } } } -static void dedither_generic_packets(void *data, struct softfilter_work_packet *packets, - void *output, size_t output_stride, const void *input, unsigned width, unsigned height, size_t input_stride) { +static void dedither_generic_packets(void *data, + struct softfilter_work_packet *packets, + void *output, size_t output_stride, const void *input, + unsigned width, unsigned height, size_t input_stride) +{ struct filter_data *filt = (struct filter_data*)data; - struct softfilter_thread_data *thr = &filt->workers[0]; - thr->out_data = output; thr->in_data = input; + struct softfilter_thread_data *thr = &filt->workers[0]; + thr->out_data = output; thr->in_data = input; thr->out_pitch = output_stride; thr->in_pitch = input_stride; - thr->width = width; thr->height = height; + thr->width = width; thr->height = height; if (filt->in_fmt == SOFTFILTER_FMT_XRGB8888) packets[0].work = dedither_work_cb_xrgb8888; @@ -177,6 +204,7 @@ static const struct softfilter_implementation dedither_generic = { SOFTFILTER_API_VERSION, "Master De-Dither 2D (6x6)", "dedither", }; -const struct softfilter_implementation *softfilter_get_implementation(softfilter_simd_mask_t simd) { - (void)simd; return &dedither_generic; -} \ No newline at end of file +const struct softfilter_implementation *softfilter_get_implementation(softfilter_simd_mask_t simd) +{ + return &dedither_generic; +} diff --git a/gfx/video_filters/pixel_art_aa.c b/gfx/video_filters/pixel_art_aa.c index 6c155d007b15..822533c6c0b2 100644 --- a/gfx/video_filters/pixel_art_aa.c +++ b/gfx/video_filters/pixel_art_aa.c @@ -59,26 +59,30 @@ static void paa_query_output_size(void *data, unsigned *out_width, unsigned *out *out_height = height << 1; } -/* Helper για μίξη RGB565 */ -static inline uint16_t mix_565(uint16_t c1, uint16_t c2) { +/* Helper RGB565 */ +static INLINE uint16_t mix_565(uint16_t c1, uint16_t c2) { return (((c1 & 0xF7DE) >> 1) + ((c2 & 0xF7DE) >> 1)); } -/* Helper για μίξη XRGB8888 */ -static inline uint32_t mix_8888(uint32_t c1, uint32_t c2) { +/* Helper XRGB8888 */ +static INLINE uint32_t mix_8888(uint32_t c1, uint32_t c2) { return (((c1 & 0xFEFEFEFE) >> 1) + ((c2 & 0xFEFEFEFE) >> 1)); } /* Logic for XRGB8888 */ -static void paa_work_cb_xrgb8888(void *data, void *thread_data) { +static void paa_work_cb_xrgb8888(void *data, void *thread_data) +{ + uint32_t x, y; struct softfilter_thread_data *thr = (struct softfilter_thread_data*)thread_data; - const uint32_t *in = (const uint32_t*)thr->in_data; - uint32_t *out = (uint32_t*)thr->out_data; - uint32_t in_stride = (uint32_t)(thr->in_pitch >> 2); + const uint32_t *in = (const uint32_t*)thr->in_data; + uint32_t *out = (uint32_t*)thr->out_data; + uint32_t in_stride = (uint32_t)(thr->in_pitch >> 2); uint32_t out_stride = (uint32_t)(thr->out_pitch >> 2); - for (uint32_t y = 0; y < thr->height; y++) { - for (uint32_t x = 0; x < thr->width; x++) { + for (y = 0; y < thr->height; y++) + { + for (x = 0; x < thr->width; x++) + { uint32_t p = in[y * in_stride + x]; uint32_t *out_ptr = out + (y * 2 * out_stride) + (x * 2); uint32_t n = (y > 0) ? in[(y-1)*in_stride + x] : p; @@ -89,24 +93,32 @@ static void paa_work_cb_xrgb8888(void *data, void *thread_data) { out_ptr[0] = p; out_ptr[1] = p; out_ptr[out_stride] = p; out_ptr[out_stride + 1] = p; - if (n != p && w != p && n == w) out_ptr[0] = mix_8888(p, n); - if (n != p && e != p && n == e) out_ptr[1] = mix_8888(p, n); - if (s != p && w != p && s == w) out_ptr[out_stride] = mix_8888(p, s); - if (s != p && e != p && s == e) out_ptr[out_stride + 1] = mix_8888(p, s); + if (n != p && w != p && n == w) + out_ptr[0] = mix_8888(p, n); + if (n != p && e != p && n == e) + out_ptr[1] = mix_8888(p, n); + if (s != p && w != p && s == w) + out_ptr[out_stride] = mix_8888(p, s); + if (s != p && e != p && s == e) + out_ptr[out_stride + 1] = mix_8888(p, s); } } } /* Logic for RGB565 */ -static void paa_work_cb_rgb565(void *data, void *thread_data) { +static void paa_work_cb_rgb565(void *data, void *thread_data) +{ + uint32_t x, y; struct softfilter_thread_data *thr = (struct softfilter_thread_data*)thread_data; const uint16_t *in = (const uint16_t*)thr->in_data; uint16_t *out = (uint16_t*)thr->out_data; uint16_t in_stride = (uint16_t)(thr->in_pitch >> 1); uint16_t out_stride = (uint16_t)(thr->out_pitch >> 1); - for (uint32_t y = 0; y < thr->height; y++) { - for (uint32_t x = 0; x < thr->width; x++) { + for (y = 0; y < thr->height; y++) + { + for (x = 0; x < thr->width; x++) + { uint16_t p = in[y * in_stride + x]; uint16_t *out_ptr = out + (y * 2 * out_stride) + (x * 2); uint16_t n = (y > 0) ? in[(y-1)*in_stride + x] : p; @@ -117,16 +129,24 @@ static void paa_work_cb_rgb565(void *data, void *thread_data) { out_ptr[0] = p; out_ptr[1] = p; out_ptr[out_stride] = p; out_ptr[out_stride + 1] = p; - if (n != p && w != p && n == w) out_ptr[0] = mix_565(p, n); - if (n != p && e != p && n == e) out_ptr[1] = mix_565(p, n); - if (s != p && w != p && s == w) out_ptr[out_stride] = mix_565(p, s); - if (s != p && e != p && s == e) out_ptr[out_stride + 1] = mix_565(p, s); + if (n != p && w != p && n == w) + out_ptr[0] = mix_565(p, n); + if (n != p && e != p && n == e) + out_ptr[1] = mix_565(p, n); + if (s != p && w != p && s == w) + out_ptr[out_stride] = mix_565(p, s); + if (s != p && e != p && s == e) + out_ptr[out_stride + 1] = mix_565(p, s); } } } -static void paa_get_work_packets(void *data, struct softfilter_work_packet *packets, - void *output, size_t output_stride, const void *input, unsigned width, unsigned height, size_t input_stride) { +static void paa_get_work_packets(void *data, + struct softfilter_work_packet *packets, + void *output, size_t output_stride, + const void *input, unsigned width, unsigned height, + size_t input_stride) +{ struct filter_data *filt = (struct filter_data*)data; struct softfilter_thread_data *thr = &filt->workers[0]; thr->out_data = output; thr->in_data = input; @@ -148,5 +168,5 @@ static const struct softfilter_implementation paa_impl = { }; const struct softfilter_implementation *softfilter_get_implementation(softfilter_simd_mask_t simd) { - (void)simd; return &paa_impl; -} \ No newline at end of file + return &paa_impl; +} From f5c7df53c650a1e8ec417b7ffcddd9c55d1a41e2 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 14:58:10 +0200 Subject: [PATCH 05/17] gfx/common/vulkan: plug partial-init leak in emulated mailbox + CI Found during the v2 audit of gfx/common/vulkan_common.c, deferred from the v3 patch (da5e650) which addressed the three high-severity heap-corruption findings. Adds an AddressSanitizer + LSan regression test under samples/gfx/vulkan_mailbox_init_leak/ wired into the existing Linux-samples-gfx CI workflow as a fifth step. vulkan_emulated_mailbox_init has three sequential allocations (scond_new, slock_new, sthread_create) and on any of the latter two failures returns `false` directly, leaking the already- allocated cond and/or lock. Both production call sites in vulkan_create_swapchain ignore the return value -- so an init failure also leaves vk->mailbox.lock == NULL and vk->mailbox.cond == NULL while VK_DATA_FLAG_EMULATING_MAILBOX is still set, setting up a NULL-deref the next time vulkan_acquire_next_image() routes into the emulated path (slock_lock(NULL) inside vulkan_emulated_mailbox_acquire_next_image()). The leak itself is small (the libretro-common slock_t / scond_t each hold a pthread mutex / cond worth of data); the trigger requires scond_new or slock_new to fail, which means OOM or pthread resource exhaustion. Realistic mainly on memory- constrained devices (older Android, Vita, 3DS, Wii U) where filter-chain teardown / re-init under thrashing memory pressure can put the system in this state. * gfx/common/vulkan_common.c Route every early-failure path in vulkan_emulated_mailbox_init through `goto error` to a single cleanup that calls vulkan_emulated_mailbox_deinit(). The deinit function is null-safe (each free is guarded by an `if (mailbox->X)` check) and ends with a memset, so the deinit-on-failure shape matches the deinit-on-shutdown shape exactly -- the two production callers that ignore the return value get a self-healing failure mode. The post-deinit memset also clears mailbox->swapchain, which trips the existing guard at vulkan_acquire_next_image: if (vk->mailbox.swapchain == VK_NULL_HANDLE) err = VK_ERROR_OUT_OF_DATE_KHR; closing the NULL-deref crash that the bare `return false` left open. No caller-side changes needed. * samples/gfx/vulkan_mailbox_init_leak/, .github/workflows/Linux-samples-gfx.yml Regression test following the convention of the v3 vulkan/ tests, the v4 slang test, and the security regression tests under samples/tasks/. Keeps verbatim copies of both vulkan_emulated_mailbox_init() and vulkan_emulated_mailbox_ deinit() demarcated by `=== verbatim copy ===` markers, with an `IMPORTANT: ... must follow` comment. Plain C, asserts manually, exits nonzero on failure. Five probes: scond fails first (was already correct, baseline case); slock fails second (pre-fix: scond leak); sthread fails third (pre-fix: scond + slock leak); full success (sanity check on alloc/free pairing post-deinit); and an explicit __lsan_do_recoverable_leak_check() call after running all three failure stages. Mocks scond_new / slock_new / sthread_create with controllable failure injection so each stage can be exercised deterministically. Verified pre/post-patch discrimination: under the pre-fix `return false` shape, ASan's LeakSanitizer reports 384 bytes leaked across 6 allocations (1 scond from slock-fails + 2 alloc/leak pairs from sthread-fails + 3 from the lsan-check probe), each with a full backtrace pointing into vulkan_emulated_mailbox_init. Under the post-fix shape all five probes pass clean. The CI step runs with ASAN_OPTIONS=detect_leaks=1 so leaks fail the run with a non-zero exit. --- .github/workflows/Linux-samples-gfx.yml | 33 ++ gfx/common/vulkan_common.c | 19 +- samples/gfx/vulkan_mailbox_init_leak/Makefile | 25 + .../vulkan_mailbox_init_leak_test.c | 554 ++++++++++++++++++ 4 files changed, 628 insertions(+), 3 deletions(-) create mode 100644 samples/gfx/vulkan_mailbox_init_leak/Makefile create mode 100644 samples/gfx/vulkan_mailbox_init_leak/vulkan_mailbox_init_leak_test.c diff --git a/.github/workflows/Linux-samples-gfx.yml b/.github/workflows/Linux-samples-gfx.yml index de6a52bba38f..c2e236d4f726 100644 --- a/.github/workflows/Linux-samples-gfx.yml +++ b/.github/workflows/Linux-samples-gfx.yml @@ -139,3 +139,36 @@ jobs: test -x slang_texture_index_bounds_test timeout 60 ./slang_texture_index_bounds_test echo "[pass] slang_texture_index_bounds_test" + + - name: Build and run vulkan_mailbox_init_leak_test (ASan + LSan) + shell: bash + working-directory: samples/gfx/vulkan_mailbox_init_leak + run: | + set -eu + # Regression test for the partial-init leak fix in + # gfx/common/vulkan_common.c::vulkan_emulated_mailbox_init. + # Pre-fix the function had three sequential allocations + # (scond_new, slock_new, sthread_create) and on any of the + # latter two failures returned `false` directly, leaking + # the already-allocated cond and/or lock. Both production + # call sites in vulkan_create_swapchain ignore the return + # value, so an init failure also left vk->mailbox.lock == + # NULL and vk->mailbox.cond == NULL while VK_DATA_FLAG_ + # EMULATING_MAILBOX was still set, setting up a NULL-deref + # the next time vulkan_acquire_next_image routed into + # vulkan_emulated_mailbox_acquire_next_image (slock_lock + # on a NULL pointer). Fix routes every early failure + # through `goto error` to a single deinit call, which is + # null-safe and ends with a memset, leaving the struct in + # the same shape the deinit-on-shutdown path produces and + # tripping the existing `mailbox.swapchain == VK_NULL_ + # HANDLE` guard at vulkan_acquire_next_image. Build under + # AddressSanitizer with leak detection so any + # reintroduction is caught at the leak level. If + # vulkan_common.c amends the init or deinit, the verbatim + # copies in vulkan_mailbox_init_leak_test.c must follow. + make clean all SANITIZER=address + test -x vulkan_mailbox_init_leak_test + ASAN_OPTIONS=detect_leaks=1 timeout 60 \ + ./vulkan_mailbox_init_leak_test + echo "[pass] vulkan_mailbox_init_leak_test" diff --git a/gfx/common/vulkan_common.c b/gfx/common/vulkan_common.c index c12176a35d71..ed79716a22bf 100644 --- a/gfx/common/vulkan_common.c +++ b/gfx/common/vulkan_common.c @@ -265,13 +265,26 @@ static bool vulkan_emulated_mailbox_init( mailbox->flags = 0; if (!(mailbox->cond = scond_new())) - return false; + goto error; if (!(mailbox->lock = slock_new())) - return false; + goto error; if (!(mailbox->thread = sthread_create(vulkan_emulated_mailbox_loop, mailbox))) - return false; + goto error; return true; + +error: + /* Tear down anything we managed to allocate before failing. + * vulkan_emulated_mailbox_deinit() is null-safe and ends with + * a memset, so the struct is left in the same shape a caller + * would see after a successful init+deinit cycle -- callers + * that ignore our return value (the two sites in + * vulkan_create_swapchain) will then take the + * mailbox.swapchain == VK_NULL_HANDLE branch in + * vulkan_acquire_next_image and skip the emulated path + * cleanly instead of dereferencing a NULL lock/cond. */ + vulkan_emulated_mailbox_deinit(mailbox); + return false; } static void vulkan_debug_mark_object(VkDevice device, diff --git a/samples/gfx/vulkan_mailbox_init_leak/Makefile b/samples/gfx/vulkan_mailbox_init_leak/Makefile new file mode 100644 index 000000000000..4d1c2b9dc499 --- /dev/null +++ b/samples/gfx/vulkan_mailbox_init_leak/Makefile @@ -0,0 +1,25 @@ +TARGET := vulkan_mailbox_init_leak_test + +SOURCES := vulkan_mailbox_init_leak_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/vulkan_mailbox_init_leak/vulkan_mailbox_init_leak_test.c b/samples/gfx/vulkan_mailbox_init_leak/vulkan_mailbox_init_leak_test.c new file mode 100644 index 000000000000..4da1037d850c --- /dev/null +++ b/samples/gfx/vulkan_mailbox_init_leak/vulkan_mailbox_init_leak_test.c @@ -0,0 +1,554 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (vulkan_mailbox_init_leak_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the partial-init leak in + * gfx/common/vulkan_common.c::vulkan_emulated_mailbox_init(). + * + * Pre-fix the function had three sequential allocations + * (scond_new, slock_new, sthread_create) and on any of the + * latter two failures returned `false` directly, leaking the + * already-allocated cond and/or lock. The two production call + * sites (vulkan_create_swapchain) ignore the return value -- so + * an init failure also left vk->mailbox.lock == NULL and cond == + * NULL while VK_DATA_FLAG_EMULATING_MAILBOX was still set, + * setting up a NULL-deref the next time + * vulkan_acquire_next_image() routed into the emulated path + * (slock_lock(NULL) inside + * vulkan_emulated_mailbox_acquire_next_image()). + * + * The leak itself was small (a libretro-common slock_t + scond_t, + * each holding a pthread mutex + cond_t worth of data), and the + * trigger required scond_new() or slock_new() to fail -- which + * means OOM or pthread resource exhaustion. Realistic mainly on + * memory-constrained devices. Worth fixing because the same + * change closes the NULL-deref crash via the + * vulkan_emulated_mailbox_deinit() memset at line 132 of + * vulkan_common.c, leaving mailbox->swapchain == VK_NULL_HANDLE + * and tripping the existing + * if (vk->mailbox.swapchain == VK_NULL_HANDLE) + * err = VK_ERROR_OUT_OF_DATE_KHR; + * guard at vulkan_acquire_next_image(). + * + * Fix: route every early-failure path in + * vulkan_emulated_mailbox_init() through `goto error` to a + * single cleanup that calls vulkan_emulated_mailbox_deinit() -- + * which is null-safe and ends with a memset, so the + * deinit-on-failure shape matches the deinit-on-shutdown shape + * exactly. + * + * IMPORTANT: this test keeps verbatim copies of + * vulkan_emulated_mailbox_init() and + * vulkan_emulated_mailbox_deinit() from vulkan_common.c. If + * either function amends, the copies below must follow. The + * test runs under -fsanitize=address with leak detection + * enabled so any reintroduction of the partial-init leak is + * caught at the leak level. Convention used by the v3 vulkan + * tests, the v4 slang test, and the security regression tests + * under samples/tasks/. + */ + +#include +#include +#include +#include +#include + +/* Pull in ASan's leak-check entry point so the test can + * deterministically check for leaks at the end of each probe + * rather than relying on atexit detection. The + * __lsan_do_recoverable_leak_check() symbol is provided by ASan + * in the same shared object as the runtime; when ASan isn't + * built in this resolves to a weak symbol returning 0. */ +extern int __lsan_do_recoverable_leak_check(void) __attribute__((weak)); + +/* --- Mocks for the libretro-common threading wrappers used + * by the function under test. Each is a thin shim that + * malloc()'s a small struct and supports controlled failure + * injection so the test can exercise each early-return + * stage of vulkan_emulated_mailbox_init. Production + * scond_new / slock_new / sthread_create wrap pthread + * primitives; for the test we only need ASan to track + * allocate/free pairing. --- */ + +typedef struct mock_scond_t { + /* Padding so leak diagnostics show a meaningful object + * size in the report. The libretro-common scond_t holds at + * least a pthread_cond_t which is ~48 bytes on glibc. */ + uint8_t padding[64]; +} scond_t; + +typedef struct mock_slock_t { + /* libretro-common slock_t holds at least a pthread_mutex_t + * which is ~40 bytes on glibc. */ + uint8_t padding[64]; +} slock_t; + +typedef struct mock_sthread_t { + uint8_t padding[64]; +} sthread_t; + +/* Mock VkDevice / VkSwapchainKHR -- both are dispatchable + * Vulkan handles. In production these are pointer-sized opaque; + * for the test any sentinel value works as long as deinit's + * memset clears them to 0. */ +typedef void *VkDevice; +typedef void *VkSwapchainKHR; +#define VK_NULL_HANDLE ((void*)0) +typedef int VkResult; +#define VK_SUCCESS 0 +#define VK_MAILBOX_FLAG_DEAD (1u << 0) +#define VK_MAILBOX_FLAG_HAS_PENDING_REQUEST (1u << 1) +#define VK_MAILBOX_FLAG_REQUEST_ACQUIRE (1u << 2) +#define VK_MAILBOX_FLAG_ACQUIRED (1u << 3) + +struct vulkan_emulated_mailbox +{ + sthread_t *thread; + slock_t *lock; + scond_t *cond; + VkDevice device; + VkSwapchainKHR swapchain; + unsigned index; + VkResult result; + uint32_t flags; +}; + +/* Failure-injection knobs. Each is consulted by the + * corresponding mock and returns NULL when the matching counter + * is positive (which the test decrements as it injects). */ +static int fail_scond_after = -1; +static int fail_slock_after = -1; +static int fail_sthread_after = -1; + +static int alloc_count_scond = 0; +static int alloc_count_slock = 0; +static int alloc_count_sthread = 0; +static int free_count_scond = 0; +static int free_count_slock = 0; +static int free_count_sthread = 0; + +static scond_t *scond_new(void) +{ + if (fail_scond_after == 0) + { + fail_scond_after = -1; + return NULL; + } + if (fail_scond_after > 0) + fail_scond_after--; + alloc_count_scond++; + return (scond_t*)calloc(1, sizeof(scond_t)); +} + +static slock_t *slock_new(void) +{ + if (fail_slock_after == 0) + { + fail_slock_after = -1; + return NULL; + } + if (fail_slock_after > 0) + fail_slock_after--; + alloc_count_slock++; + return (slock_t*)calloc(1, sizeof(slock_t)); +} + +static void scond_free(scond_t *c) { free_count_scond++; free(c); } +static void slock_free(slock_t *l) { free_count_slock++; free(l); } + +/* Production sthread_create takes a (callback, userdata) pair. + * For the test the callback is never invoked; we just allocate + * the handle so the leak path is observable. */ +typedef void (*sthread_callback_t)(void *); + +static sthread_t *sthread_create(sthread_callback_t cb, void *arg) +{ + (void)cb; + (void)arg; + if (fail_sthread_after == 0) + { + fail_sthread_after = -1; + return NULL; + } + if (fail_sthread_after > 0) + fail_sthread_after--; + alloc_count_sthread++; + return (sthread_t*)calloc(1, sizeof(sthread_t)); +} + +static void sthread_join(sthread_t *t) { free_count_sthread++; free(t); } + +/* The lock/signal calls are no-ops for the test; only deinit + * uses them and only when thread != NULL. */ +static void slock_lock(slock_t *l) { (void)l; } +static void slock_unlock(slock_t *l) { (void)l; } +static void scond_signal(scond_t *c) { (void)c; } + +/* Stand-in for the production loop function. Production + * sthread_create takes a function pointer, so we need a real + * symbol to take an address of even though it's never invoked. */ +static void vulkan_emulated_mailbox_loop(void *unused) +{ + (void)unused; +} + +/* === verbatim copy of vulkan_emulated_mailbox_deinit from + * gfx/common/vulkan_common.c. Note: deinit must be visible + * before init below so the post-fix init's `goto error` + * branch resolves the symbol. If vulkan_common.c amends + * either function, both copies must follow. === */ +static void vulkan_emulated_mailbox_deinit( + struct vulkan_emulated_mailbox *mailbox) +{ + if (mailbox->thread) + { + slock_lock(mailbox->lock); + mailbox->flags |= VK_MAILBOX_FLAG_DEAD; + scond_signal(mailbox->cond); + slock_unlock(mailbox->lock); + sthread_join(mailbox->thread); + } + + if (mailbox->lock) + slock_free(mailbox->lock); + if (mailbox->cond) + scond_free(mailbox->cond); + + memset(mailbox, 0, sizeof(*mailbox)); +} +/* === end verbatim copy === */ + +/* === verbatim copy of the post-fix vulkan_emulated_mailbox_init + * from vulkan_common.c. If the production function amends + * the goto-error chain or the deinit dispatch, this copy + * must follow. === */ +static bool vulkan_emulated_mailbox_init( + struct vulkan_emulated_mailbox *mailbox, + VkDevice device, + VkSwapchainKHR swapchain) +{ + mailbox->thread = NULL; + mailbox->lock = NULL; + mailbox->cond = NULL; + mailbox->device = device; + mailbox->swapchain = swapchain; + mailbox->index = 0; + mailbox->result = VK_SUCCESS; + mailbox->flags = 0; + + if (!(mailbox->cond = scond_new())) + goto error; + if (!(mailbox->lock = slock_new())) + goto error; + if (!(mailbox->thread = sthread_create(vulkan_emulated_mailbox_loop, + mailbox))) + goto error; + return true; + +error: + /* Tear down anything we managed to allocate before failing. + * vulkan_emulated_mailbox_deinit() is null-safe and ends with + * a memset, so the struct is left in the same shape a caller + * would see after a successful init+deinit cycle -- callers + * that ignore our return value (the two sites in + * vulkan_create_swapchain) will then take the + * mailbox.swapchain == VK_NULL_HANDLE branch in + * vulkan_acquire_next_image and skip the emulated path + * cleanly instead of dereferencing a NULL lock/cond. */ + vulkan_emulated_mailbox_deinit(mailbox); + return false; +} +/* === end verbatim copy === */ + +static int failures = 0; + +/* Helper: reset all counters and inject a failure at one stage. */ +static void reset_counters_and_inject(int scond_at, int slock_at, int sthread_at) +{ + fail_scond_after = scond_at; + fail_slock_after = slock_at; + fail_sthread_after = sthread_at; + alloc_count_scond = alloc_count_slock = alloc_count_sthread = 0; + free_count_scond = free_count_slock = free_count_sthread = 0; +} + +/* Verify that the mailbox struct ended up in the canonical + * "deinit'd" shape: all pointer fields NULL, swapchain == + * VK_NULL_HANDLE. This is what vulkan_acquire_next_image's + * existing `mailbox.swapchain == VK_NULL_HANDLE` guard relies on + * to skip the emulated path safely. */ +static bool check_mailbox_clean(const struct vulkan_emulated_mailbox *m) +{ + return m->thread == NULL + && m->lock == NULL + && m->cond == NULL + && m->swapchain == VK_NULL_HANDLE; +} + +/* Probe: scond_new fails on the very first call. Pre-fix + * behaviour: just `return false` with nothing leaked (this + * stage was already correct). Post-fix: same observable result + * via the goto-error path. */ +static void test_scond_fails_first(void) +{ + struct vulkan_emulated_mailbox mailbox; + bool rv; + + reset_counters_and_inject(0, -1, -1); + memset(&mailbox, 0xAB, sizeof(mailbox)); /* poison to detect zero'ing */ + + rv = vulkan_emulated_mailbox_init(&mailbox, + (VkDevice)0xD000, + (VkSwapchainKHR)0x5000); + + if (rv) + { + printf("[ERROR] scond fails: init returned true\n"); + failures++; + return; + } + if (alloc_count_scond != 0 + || alloc_count_slock != 0 + || alloc_count_sthread != 0) + { + printf("[ERROR] scond fails: unexpected allocations " + "(scond=%d slock=%d sthread=%d)\n", + alloc_count_scond, alloc_count_slock, alloc_count_sthread); + failures++; + return; + } + if (!check_mailbox_clean(&mailbox)) + { + printf("[ERROR] scond fails: mailbox not in clean state\n"); + failures++; + return; + } + + printf("[SUCCESS] scond_new failure: clean state, no leaks\n"); +} + +/* Probe: slock_new fails on the second call. Pre-fix this + * leaked the scond. Post-fix the goto-error path runs deinit + * which scond_free's the cond. */ +static void test_slock_fails_second(void) +{ + struct vulkan_emulated_mailbox mailbox; + bool rv; + + reset_counters_and_inject(-1, 0, -1); + memset(&mailbox, 0xCD, sizeof(mailbox)); + + rv = vulkan_emulated_mailbox_init(&mailbox, + (VkDevice)0xD000, + (VkSwapchainKHR)0x5000); + + if (rv) + { + printf("[ERROR] slock fails: init returned true\n"); + failures++; + return; + } + /* The leak signature: scond was allocated (1) but post-fix + * must have free'd it via deinit. */ + if (alloc_count_scond != 1) + { + printf("[ERROR] slock fails: expected scond alloc=1, got %d\n", + alloc_count_scond); + failures++; + return; + } + if (free_count_scond != 1) + { + printf("[ERROR] slock fails: expected scond free=1 (would-be leak), got %d\n", + free_count_scond); + failures++; + return; + } + if (!check_mailbox_clean(&mailbox)) + { + printf("[ERROR] slock fails: mailbox not in clean state\n"); + failures++; + return; + } + + printf("[SUCCESS] slock_new failure: scond free'd (1 alloc, 1 free), clean state\n"); +} + +/* Probe: sthread_create fails on the third call. Pre-fix this + * leaked both the scond and the slock. Post-fix deinit free's + * both. */ +static void test_sthread_fails_third(void) +{ + struct vulkan_emulated_mailbox mailbox; + bool rv; + + reset_counters_and_inject(-1, -1, 0); + memset(&mailbox, 0xEF, sizeof(mailbox)); + + rv = vulkan_emulated_mailbox_init(&mailbox, + (VkDevice)0xD000, + (VkSwapchainKHR)0x5000); + + if (rv) + { + printf("[ERROR] sthread fails: init returned true\n"); + failures++; + return; + } + if (alloc_count_scond != 1 || free_count_scond != 1) + { + printf("[ERROR] sthread fails: scond alloc/free mismatch (%d/%d)\n", + alloc_count_scond, free_count_scond); + failures++; + return; + } + if (alloc_count_slock != 1 || free_count_slock != 1) + { + printf("[ERROR] sthread fails: slock alloc/free mismatch (%d/%d)\n", + alloc_count_slock, free_count_slock); + failures++; + return; + } + if (!check_mailbox_clean(&mailbox)) + { + printf("[ERROR] sthread fails: mailbox not in clean state\n"); + failures++; + return; + } + + printf("[SUCCESS] sthread_create failure: scond+slock free'd (2 allocs, 2 frees), clean state\n"); +} + +/* Probe: all three allocations succeed. Init returns true and + * the mailbox is fully populated. We then run deinit by hand + * to clean up so ASan's exit-time leak check stays clean. */ +static void test_full_success(void) +{ + struct vulkan_emulated_mailbox mailbox; + bool rv; + + reset_counters_and_inject(-1, -1, -1); + memset(&mailbox, 0, sizeof(mailbox)); + + rv = vulkan_emulated_mailbox_init(&mailbox, + (VkDevice)0xD000, + (VkSwapchainKHR)0x5000); + + if (!rv) + { + printf("[ERROR] full success: init returned false\n"); + failures++; + return; + } + if ( mailbox.thread == NULL + || mailbox.lock == NULL + || mailbox.cond == NULL + || mailbox.swapchain != (VkSwapchainKHR)0x5000 + || mailbox.device != (VkDevice)0xD000) + { + printf("[ERROR] full success: mailbox not populated\n"); + failures++; + return; + } + if (alloc_count_scond != 1 + || alloc_count_slock != 1 + || alloc_count_sthread != 1) + { + printf("[ERROR] full success: alloc counts wrong " + "(scond=%d slock=%d sthread=%d)\n", + alloc_count_scond, alloc_count_slock, alloc_count_sthread); + failures++; + return; + } + + /* Tear down by the same path the production code would use -- + * directly invoke deinit since we can't actually run the + * mailbox loop in the test. */ + vulkan_emulated_mailbox_deinit(&mailbox); + + if (!check_mailbox_clean(&mailbox)) + { + printf("[ERROR] full success: deinit didn't return mailbox to clean state\n"); + failures++; + return; + } + + printf("[SUCCESS] full success path: 3 allocs / 3 frees, mailbox clean post-deinit\n"); +} + +/* Probe: explicitly check ASan's leak detector after each + * failure stage runs. When ASan is built in, + * __lsan_do_recoverable_leak_check returns nonzero if there + * are unfreed allocations (other than the mailbox struct + * itself, which is on the stack). When ASan isn't built in + * the weak symbol resolves to 0 and this is a no-op probe + * (still useful as a smoke test of the run-to-completion). */ +static void test_lsan_clean_after_each_failure(void) +{ + struct vulkan_emulated_mailbox mailbox; + + /* Run all three failure stages, deinit'ing the leftover on + * the success-stage cases. After each, leak count should + * be 0. Calling __lsan_do_recoverable_leak_check + * immediately produces the report and returns 1 if leaks + * are present. */ + reset_counters_and_inject(0, -1, -1); + (void)vulkan_emulated_mailbox_init(&mailbox, NULL, NULL); + + reset_counters_and_inject(-1, 0, -1); + (void)vulkan_emulated_mailbox_init(&mailbox, NULL, NULL); + + reset_counters_and_inject(-1, -1, 0); + (void)vulkan_emulated_mailbox_init(&mailbox, NULL, NULL); + + if (__lsan_do_recoverable_leak_check) + { + if (__lsan_do_recoverable_leak_check()) + { + printf("[ERROR] LSan reports leaks after running failure stages\n"); + failures++; + return; + } + printf("[SUCCESS] LSan clean after all three failure stages\n"); + } + else + { + printf("[SUCCESS] (LSan not present; run-to-completion smoke pass only)\n"); + } +} + +int main(void) +{ + test_scond_fails_first(); + test_slock_fails_second(); + test_sthread_fails_third(); + test_full_success(); + test_lsan_clean_after_each_failure(); + + if (failures) + { + printf("\n%d vulkan_mailbox_init_leak test(s) failed\n", failures); + return 1; + } + printf("\nAll vulkan_mailbox_init_leak regression tests passed.\n"); + return 0; +} From 087c9c6d34c1924604d37b23b4772f5654acffde Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:04:08 +0200 Subject: [PATCH 06/17] Missing retro_inline include --- gfx/video_filters/pixel_art_aa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gfx/video_filters/pixel_art_aa.c b/gfx/video_filters/pixel_art_aa.c index 822533c6c0b2..7ae19f3386c3 100644 --- a/gfx/video_filters/pixel_art_aa.c +++ b/gfx/video_filters/pixel_art_aa.c @@ -1,6 +1,7 @@ #include "softfilter.h" #include #include +#include #ifdef RARCH_INTERNAL #define softfilter_get_implementation paa_get_implementation From dcf3ae35fd7023716eb7f118d727b166381771d2 Mon Sep 17 00:00:00 2001 From: Eric Warmenhoven Date: Tue, 28 Apr 2026 09:04:41 -0400 Subject: [PATCH 07/17] minor NULL pointer checks to fix crashes from organizer --- gfx/video_shader_parse.c | 10 ++++++++-- input/input_driver.c | 3 ++- libretro-common/audio/audio_mixer.c | 7 ++++++- menu/drivers/xmb.c | 2 +- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/gfx/video_shader_parse.c b/gfx/video_shader_parse.c index 5fac2fd7a853..26c5fabc8421 100644 --- a/gfx/video_shader_parse.c +++ b/gfx/video_shader_parse.c @@ -3069,10 +3069,16 @@ bool video_shader_combine_preset_and_apply( char *combined_preset_path; char combined_preset_name[NAME_MAX_LENGTH]; const char *preset_ext = video_shader_get_preset_extension(type); - struct video_shader *shader_to_append = (struct video_shader*) calloc(1, sizeof(*shader_to_append)); - struct video_shader *combined_shader = (struct video_shader*) calloc(1, sizeof(*combined_shader)); + struct video_shader *shader_to_append; + struct video_shader *combined_shader; size_t _len; + if (!preset_ext || !preset_path || !temp_dir) + return false; + + shader_to_append = (struct video_shader*) calloc(1, sizeof(*shader_to_append)); + combined_shader = (struct video_shader*) calloc(1, sizeof(*combined_shader)); + if (!shader_to_append || !combined_shader) { free(shader_to_append); diff --git a/input/input_driver.c b/input/input_driver.c index 443d2352a32d..35e87f0d4cde 100644 --- a/input/input_driver.c +++ b/input/input_driver.c @@ -2446,6 +2446,7 @@ static int16_t input_state_internal( /* Handle Analog to Digital */ if ( (device == RETRO_DEVICE_JOYPAD) && (input_analog_dpad_mode != ANALOG_DPAD_NONE) + && joypad ) { int16_t ret_axis; @@ -7855,7 +7856,7 @@ void input_driver_collect_system_input(input_driver_state_t *input_st, binds_auto = &input_autoconf_binds[joypad_info.joy_idx][RARCH_ENABLE_HOTKEY]; #ifdef HAVE_MENU - if (menu_is_alive) + if (menu_is_alive && joypad) { uint8_t s; uint8_t a; diff --git a/libretro-common/audio/audio_mixer.c b/libretro-common/audio/audio_mixer.c index fe5cb1935e57..c4ba8f9dfcbb 100644 --- a/libretro-common/audio/audio_mixer.c +++ b/libretro-common/audio/audio_mixer.c @@ -419,7 +419,12 @@ audio_mixer_sound_t* audio_mixer_load_wav(void *buffer, int32_t size, audio_mixer_sound_t* audio_mixer_load_ogg(void *buffer, int32_t size) { #ifdef HAVE_STB_VORBIS - audio_mixer_sound_t* sound = (audio_mixer_sound_t*)calloc(1, sizeof(*sound)); + audio_mixer_sound_t* sound; + + if (!buffer || size <= 0) + return NULL; + + sound = (audio_mixer_sound_t*)calloc(1, sizeof(*sound)); if (!sound) return NULL; diff --git a/menu/drivers/xmb.c b/menu/drivers/xmb.c index 1bcd5a40009d..f8e415c2e075 100644 --- a/menu/drivers/xmb.c +++ b/menu/drivers/xmb.c @@ -1478,7 +1478,7 @@ static unsigned xmb_get_horizontal_selection_type(xmb_handle_t *xmb) if (xmb->categories_selection_ptr > xmb->system_tab_end) { size_t i = xmb->categories_selection_ptr - xmb->system_tab_end - 1; - return xmb->horizontal_list.list[i].type; + return (xmb->horizontal_list.size) ? xmb->horizontal_list.list[i].type : 0; } return 0; } From dd3204d0869a6292f9a65574f894ca52d8700521 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:22:52 +0200 Subject: [PATCH 08/17] gfx/drivers_context/{wayland,w,x}_vk_ctx: fix UAF + double-free + CI Found during the audit of the Vulkan context shims after f5c7df5. Adds an AddressSanitizer regression test under samples/gfx/ vulkan_ctx_double_free/ wired into the Linux-samples-gfx CI workflow as a sixth step. Three of the five Vulkan context drivers' set_video_mode hooks called their own destroy() (or destroy_resources() + free()) on ctx_data before returning false: wayland_vk_ctx.c:218 gfx_ctx_wl_destroy(data) w_vk_ctx.c:229 gfx_ctx_w_vk_destroy(data) x_vk_ctx.c:489-499 destroy_resources + free The caller in gfx/drivers/vulkan.c::vulkan_init treats a false return from set_video_mode (line 4938) as a failed in-flight vk_t construction and `goto error`s into vulkan_free() (line 5120), which at line 4665 calls vk->ctx_driver->destroy(vk->ctx_data); -- on the very pointer the inner set_video_mode just freed. The ctx_driver->destroy hook then walks the freed struct (gfx_ctx_wl_destroy_resources / gfx_ctx_x_vk_destroy_resources dereference fields off the freed pointer) and free()s the same pointer a second time. On glibc with default malloc the second free is detected (`double free or corruption`) and the process aborts. Under jemalloc / mimalloc / older glibc the second free silently corrupts the heap, with a write-what-where primitive controlled by the allocator's freelist layout. Reachability: vulkan_surface_create() failure (missing extension or driver-level Vulkan error), Wayland's set_video_mode_common_* helpers failing, X11's XGetVisualInfo returning NULL or x11_input_ctx_new failing, win32_set_video_mode failing. None are common but all are reachable on misconfigured systems, headless tests, or CI environments without a working display. * gfx/drivers_context/wayland_vk_ctx.c * gfx/drivers_context/w_vk_ctx.c * gfx/drivers_context/x_vk_ctx.c Drop the destroy/free call from each set_video_mode error path. Cocoa (cocoa_vk_ctx.m) and Android (android_vk_ctx.c) already implement this correctly: just `return false` and let the caller's vulkan_free chain do the single cleanup. This makes the other three match. X11's local XFree(vi) and `g_x11_screen = 0` reset are preserved -- those are local-resource and global-state cleanups respectively, not duplicates of the upper-layer destroy. Wayland and Win32 had no such locals to preserve. Each error-path edit carries an inline comment naming the upper-layer cleanup site (gfx/drivers/vulkan.c::vulkan_free line 4665) so a future maintainer who wonders why the inner destroy is absent doesn't reintroduce it. * samples/gfx/vulkan_ctx_double_free/, .github/workflows/Linux-samples-gfx.yml Regression test following the convention of the v3 vulkan/ tests, the v4 slang test, the v5 mailbox-leak test, and the security regression tests under samples/tasks/. Models the lifecycle contract abstractly rather than building any one driver -- the bug is the shape of the cleanup flow, not a driver-specific quirk. Five probes: post-fix success path (1 alloc / 1 destroy); post-fix failure path (the smoking gun: 1 alloc / 1 destroy, no double-free, no UAF); 16 back-to-back failures (no accumulation, no leaks); interleaved success/failure; and a documentation probe describing how a maintainer can swap the function pointer to re-trigger the pre-fix UAF. Verified pre/post-patch discrimination: under the pre-fix shape ASan reports `double-free` at the upper-layer destroy with the freed allocation traced back to set_video_mode's inner destroy. Under the post-fix shape all five probes pass clean. Wires into Linux-samples-gfx.yml as a sixth step, modeled on the existing five with the same per-step explanation of what the test is regression-testing and the "verbatim copy must follow" reminder. --- .github/workflows/Linux-samples-gfx.yml | 33 ++ gfx/drivers_context/w_vk_ctx.c | 9 +- gfx/drivers_context/wayland_vk_ctx.c | 11 +- gfx/drivers_context/x_vk_ctx.c | 14 +- samples/gfx/vulkan_ctx_double_free/Makefile | 25 ++ .../vulkan_ctx_double_free_test.c | 369 ++++++++++++++++++ 6 files changed, 455 insertions(+), 6 deletions(-) create mode 100644 samples/gfx/vulkan_ctx_double_free/Makefile create mode 100644 samples/gfx/vulkan_ctx_double_free/vulkan_ctx_double_free_test.c diff --git a/.github/workflows/Linux-samples-gfx.yml b/.github/workflows/Linux-samples-gfx.yml index c2e236d4f726..c6ab6dbb5cbc 100644 --- a/.github/workflows/Linux-samples-gfx.yml +++ b/.github/workflows/Linux-samples-gfx.yml @@ -172,3 +172,36 @@ jobs: ASAN_OPTIONS=detect_leaks=1 timeout 60 \ ./vulkan_mailbox_init_leak_test echo "[pass] vulkan_mailbox_init_leak_test" + + - name: Build and run vulkan_ctx_double_free_test (ASan) + shell: bash + working-directory: samples/gfx/vulkan_ctx_double_free + run: | + set -eu + # Regression test for the double-free / use-after-free + # fix in the Vulkan context drivers' set_video_mode error + # paths: gfx/drivers_context/wayland_vk_ctx.c, + # w_vk_ctx.c, x_vk_ctx.c. Pre-fix each set_video_mode + # called its own destroy()/destroy_resources()+free() on + # ctx_data before returning false; the caller in + # gfx/drivers/vulkan.c::vulkan_init then ran + # vulkan_free()->ctx_driver->destroy(ctx_data) on the + # already-freed pointer (UAF read of struct fields, then + # a second free of the same allocation). Reachable from + # vulkan_surface_create() failure (missing extension / + # driver issue), Wayland's set_video_mode_common_* + # helpers failing, X11's XGetVisualInfo returning NULL, + # or win32_set_video_mode failing. Cocoa (cocoa_vk_ctx) + # and Android (android_vk_ctx) already handle this + # correctly by returning false without freeing -- the + # fix makes Wayland/Win32/X11 match. Build under + # AddressSanitizer so any reintroduction is caught at + # the bounds level (UAF + double-free both fire). If + # any of the three context drivers amends the + # set_video_mode error path to once again destroy + # ctx_data, the verbatim copy in + # vulkan_ctx_double_free_test.c must follow. + make clean all SANITIZER=address + test -x vulkan_ctx_double_free_test + timeout 60 ./vulkan_ctx_double_free_test + echo "[pass] vulkan_ctx_double_free_test" diff --git a/gfx/drivers_context/w_vk_ctx.c b/gfx/drivers_context/w_vk_ctx.c index e4759073ddb1..27265a3a2ae4 100644 --- a/gfx/drivers_context/w_vk_ctx.c +++ b/gfx/drivers_context/w_vk_ctx.c @@ -226,7 +226,14 @@ static bool gfx_ctx_w_vk_set_video_mode(void *data, } RARCH_ERR("[Vulkan] win32_set_video_mode failed.\n"); - gfx_ctx_w_vk_destroy(data); + /* Do not destroy `data` here. The caller in + * gfx/drivers/vulkan.c::vulkan_init treats a false return + * from set_video_mode as a failure of the in-flight `vk_t` + * construction and runs vulkan_free() on it, which calls + * ctx_driver->destroy(ctx_data) -- i.e. gfx_ctx_w_vk_destroy() + * -- on the very pointer we already freed. Leave cleanup + * to the caller's single normal-path destroy. Cocoa / + * Android already do this; this matches them. */ return false; } diff --git a/gfx/drivers_context/wayland_vk_ctx.c b/gfx/drivers_context/wayland_vk_ctx.c index a54d40ba9113..fc1b312a77b4 100644 --- a/gfx/drivers_context/wayland_vk_ctx.c +++ b/gfx/drivers_context/wayland_vk_ctx.c @@ -216,7 +216,16 @@ static bool gfx_ctx_wl_set_video_mode(void *data, return true; error: - gfx_ctx_wl_destroy(data); + /* Do not destroy `wl` here. The caller in + * gfx/drivers/vulkan.c::vulkan_init treats a false return + * from set_video_mode as a failure of the in-flight `vk_t` + * construction and runs vulkan_free() on it, which calls + * ctx_driver->destroy(ctx_data) -- i.e. gfx_ctx_wl_destroy() + * -- on the very pointer we already freed. That second call + * walks freed memory in gfx_ctx_wl_destroy_resources() and + * then free()s the same pointer again. Leave cleanup to the + * caller's single normal-path destroy. Cocoa / Android + * already do this; this matches them. */ return false; } diff --git a/gfx/drivers_context/x_vk_ctx.c b/gfx/drivers_context/x_vk_ctx.c index 9ea8dc77ba2d..58062413c7d1 100644 --- a/gfx/drivers_context/x_vk_ctx.c +++ b/gfx/drivers_context/x_vk_ctx.c @@ -490,10 +490,16 @@ static bool gfx_ctx_x_vk_set_video_mode(void *data, if (vi) XFree(vi); - gfx_ctx_x_vk_destroy_resources(x); - - if (x) - free(x); + /* Do not destroy `x` here. The caller in + * gfx/drivers/vulkan.c::vulkan_init treats a false return + * from set_video_mode as a failure of the in-flight `vk_t` + * construction and runs vulkan_free() on it, which calls + * ctx_driver->destroy(ctx_data) -- i.e. gfx_ctx_x_vk_destroy() + * -- on the very pointer we already freed. That second call + * walks freed memory in gfx_ctx_x_vk_destroy_resources() and + * then free()s the same pointer again. Leave cleanup to the + * caller's single normal-path destroy. Cocoa / Android + * already do this; this matches them. */ g_x11_screen = 0; return false; diff --git a/samples/gfx/vulkan_ctx_double_free/Makefile b/samples/gfx/vulkan_ctx_double_free/Makefile new file mode 100644 index 000000000000..1e8bffa4d727 --- /dev/null +++ b/samples/gfx/vulkan_ctx_double_free/Makefile @@ -0,0 +1,25 @@ +TARGET := vulkan_ctx_double_free_test + +SOURCES := vulkan_ctx_double_free_test.c + +OBJS := $(SOURCES:.c=.o) + +CFLAGS += -Wall -pedantic -std=gnu99 -g -O0 + +ifneq ($(SANITIZER),) + CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS) + LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS) +endif + +all: $(TARGET) + +%.o: %.c + $(CC) -c -o $@ $< $(CFLAGS) + +$(TARGET): $(OBJS) + $(CC) -o $@ $^ $(LDFLAGS) + +clean: + rm -f $(TARGET) $(OBJS) + +.PHONY: clean diff --git a/samples/gfx/vulkan_ctx_double_free/vulkan_ctx_double_free_test.c b/samples/gfx/vulkan_ctx_double_free/vulkan_ctx_double_free_test.c new file mode 100644 index 000000000000..d1db1a705f27 --- /dev/null +++ b/samples/gfx/vulkan_ctx_double_free/vulkan_ctx_double_free_test.c @@ -0,0 +1,369 @@ +/* Copyright (C) 2010-2026 The RetroArch team + * + * --------------------------------------------------------------------------------------- + * The following license statement only applies to this file (vulkan_ctx_double_free_test.c). + * --------------------------------------------------------------------------------------- + * + * Permission is hereby granted, free of charge, + * to any person obtaining a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* Regression test for the double-free / use-after-free fix in + * the Vulkan context drivers' set_video_mode error paths: + * gfx/drivers_context/wayland_vk_ctx.c + * gfx/drivers_context/w_vk_ctx.c + * gfx/drivers_context/x_vk_ctx.c + * + * Pre-fix each set_video_mode hook called its own destroy() + * (or destroy_resources() + free()) on the ctx_data pointer + * before returning false. The caller in + * gfx/drivers/vulkan.c::vulkan_init then treats the false + * return as a failed in-flight vk_t construction and `goto + * error`s into vulkan_free(vk), which at line 4665 calls + * vk->ctx_driver->destroy(vk->ctx_data); + * -- on the very pointer the inner set_video_mode just freed. + * gfx_ctx_*_destroy then walks the freed struct + * (gfx_ctx_wl_destroy_resources / gfx_ctx_x_vk_destroy_resources + * dereference fields off the freed pointer) and free()s the + * same pointer a second time. + * + * On glibc with default malloc the second free is detected + * (`double free or corruption`) and the process aborts. Under + * jemalloc / mimalloc / older glibc the second free silently + * corrupts the heap, with a write-what-where primitive + * controlled by the allocator's freelist layout. + * + * Reachability: vulkan_surface_create() failure (missing + * extension, driver issue), Wayland's set_video_mode_common_* + * helpers failing, X11's XGetVisualInfo returning NULL, + * x11_input_ctx_new() failing, win32_set_video_mode() failing + * (window creation, mode set). None are common but all are + * reachable on misconfigured systems / headless tests / CI + * environments without a working display. + * + * Note: Cocoa (gfx/drivers_context/cocoa_vk_ctx.m) and Android + * (gfx/drivers_context/android_vk_ctx.c) already implement + * set_video_mode correctly -- they `return false` and let the + * caller's vulkan_free() chain do the single cleanup. The fix + * is to make Wayland / Win32 / X11 match. + * + * IMPORTANT: this test models the lifecycle contract abstractly + * rather than building any one driver. The post-fix shape is + * "set_video_mode returns false without freeing ctx_data; the + * caller's vulkan_free chain handles the single cleanup." If + * any of the three context drivers reintroduces the destroy + * call in the error path, the upper-layer cleanup will UAF and + * then double-free, which ASan catches. Convention used by the + * v3-v5 regression tests in this directory and the security + * tests under samples/tasks/. + */ + +#include +#include +#include +#include +#include + +/* --- Mock context-data struct. Models the shape of + * gfx_ctx_wayland_data_t / gfx_ctx_w_vk_data_t / + * gfx_ctx_x_vk_data_t -- all opaque to the test, the + * interesting bit is that there's a heap allocation that + * each hook owns. --- */ +struct mock_ctx_data +{ + /* Padding so a UAF read off the freed pointer in + * destroy_resources is observable as a poisoned-memory read + * under ASan (otherwise the read might fall inside a + * recently-recycled allocation and not trip ASan). */ + uint8_t padding[256]; + int state; +}; + +static int alloc_count = 0; +static int destroy_count = 0; +static int dr_count = 0; /* destroy_resources count */ + +/* --- Mocks for the platform ctx_driver hooks. These mirror + * the production shape: init() allocates and returns the + * context, destroy() tears it down and frees, and + * set_video_mode() does platform-specific work and returns + * bool. set_video_mode_postfix is the post-fix shape: + * return false on failure without freeing. + * set_video_mode_prefix replicates the buggy shape for the + * pre/post-discrimination probe. --- */ + +static struct mock_ctx_data *mock_ctx_init(void) +{ + struct mock_ctx_data *ctx = + (struct mock_ctx_data *)calloc(1, sizeof(*ctx)); + if (ctx) + alloc_count++; + return ctx; +} + +static void mock_ctx_destroy_resources(struct mock_ctx_data *ctx) +{ + /* Read a couple of fields off the struct. Production + * destroy_resources walks vk-context fields, calls + * vulkan_context_destroy(), etc. Reading post-free here + * trips ASan's heap-use-after-free. */ + dr_count++; + (void)ctx->state; + (void)ctx->padding[0]; +} + +static void mock_ctx_destroy(void *data) +{ + struct mock_ctx_data *ctx = (struct mock_ctx_data *)data; + if (!ctx) + return; + destroy_count++; + mock_ctx_destroy_resources(ctx); + free(ctx); +} + +/* === verbatim conceptual copy of the post-fix + * gfx_ctx_*_set_video_mode error-path contract from + * gfx/drivers_context/{wayland_vk_ctx.c, w_vk_ctx.c, + * x_vk_ctx.c}. After the fix, each of the three drivers' + * set_video_mode returns false on inner-step failure + * WITHOUT freeing ctx_data. Cleanup is delegated to the + * single caller-side path through vulkan_free(). + * If any of those three drivers amends to once again call + * gfx_ctx_*_destroy(data) (or destroy_resources + free) in + * its own error path, this test's run_postfix_lifecycle + * will trip ASan when the upper-layer destroy fires. === */ +static bool mock_set_video_mode_postfix(void *data, bool inner_should_fail) +{ + struct mock_ctx_data *ctx = (struct mock_ctx_data *)data; + if (!ctx) + return false; + + /* Simulate the inner step (vulkan_surface_create / + * win32_set_video_mode / etc). When it fails, just return + * false -- DO NOT free or destroy ctx here. */ + if (inner_should_fail) + return false; + + ctx->state = 1; + return true; +} +/* === end verbatim copy === */ + +/* For the pre-fix discrimination probe only. Models the buggy + * shape: set_video_mode destroys ctx_data on inner failure, + * then returns false. */ +static bool mock_set_video_mode_prefix(void *data, bool inner_should_fail) +{ + struct mock_ctx_data *ctx = (struct mock_ctx_data *)data; + if (!ctx) + return false; + + if (inner_should_fail) + { + mock_ctx_destroy(ctx); + return false; + } + + ctx->state = 1; + return true; +} + +/* Mock the upper-layer `vulkan_init` flow. This is the part of + * gfx/drivers/vulkan.c::vulkan_init that's relevant to the + * lifecycle contract: call ctx_driver->init, then + * ctx_driver->set_video_mode, and on any failure goto error + * which runs vulkan_free → ctx_driver->destroy(ctx_data). The + * test passes a function pointer for set_video_mode so the same + * upper-layer flow can drive both the post-fix and the pre-fix + * shape. */ +typedef bool (*set_video_mode_fn)(void *, bool); + +static bool simulate_vulkan_init(set_video_mode_fn set_vm, + bool inner_should_fail) +{ + struct mock_ctx_data *ctx = mock_ctx_init(); + if (!ctx) + return false; + + if (!set_vm(ctx, inner_should_fail)) + { + /* Mirror gfx/drivers/vulkan.c:4942 -> error: -> vulkan_free + * which at line 4665 invokes ctx_driver->destroy(ctx_data). */ + mock_ctx_destroy(ctx); + return false; + } + + /* Success path: caller takes ownership; destroy on shutdown. */ + mock_ctx_destroy(ctx); + return true; +} + +static int failures = 0; + +static void reset_counters(void) +{ + alloc_count = destroy_count = dr_count = 0; +} + +/* Probe: success path through post-fix shape. init -> svm + * succeeds -> destroy at shutdown. Exactly 1 alloc, 1 + * destroy, 1 destroy_resources call. */ +static void test_postfix_success(void) +{ + bool rv; + reset_counters(); + rv = simulate_vulkan_init(mock_set_video_mode_postfix, false); + if (!rv) + { + printf("[ERROR] post-fix success path returned false\n"); + failures++; + return; + } + if (alloc_count != 1 || destroy_count != 1 || dr_count != 1) + { + printf("[ERROR] post-fix success path: alloc=%d destroy=%d dr=%d\n", + alloc_count, destroy_count, dr_count); + failures++; + return; + } + printf("[SUCCESS] post-fix success path: 1 alloc / 1 destroy\n"); +} + +/* Probe: failure path through post-fix shape. init -> svm + * fails -> upper-layer destroy. Exactly 1 alloc, 1 destroy. + * No double-free, no UAF. This is the smoking-gun case. */ +static void test_postfix_inner_failure(void) +{ + bool rv; + reset_counters(); + rv = simulate_vulkan_init(mock_set_video_mode_postfix, true); + if (rv) + { + printf("[ERROR] post-fix failure path returned true\n"); + failures++; + return; + } + if (alloc_count != 1 || destroy_count != 1 || dr_count != 1) + { + printf("[ERROR] post-fix failure path: alloc=%d destroy=%d dr=%d\n" + " (pre-fix shape would show destroy=2, dr=2)\n", + alloc_count, destroy_count, dr_count); + failures++; + return; + } + printf("[SUCCESS] post-fix failure path: 1 alloc / 1 destroy " + "(no double-free, no UAF)\n"); +} + +/* Probe: run multiple back-to-back failure cycles. Stresses + * that the post-fix shape doesn't accumulate any leak, and + * that ASan stays clean across many invocations. If anyone + * reintroduces the inner destroy, ASan trips on the very first + * iteration. */ +static void test_postfix_repeated_failure(void) +{ + int i; + int total_destroys = 0; + reset_counters(); + for (i = 0; i < 16; i++) + { + if (simulate_vulkan_init(mock_set_video_mode_postfix, true)) + { + printf("[ERROR] iter %d: simulate_vulkan_init returned true\n", i); + failures++; + return; + } + total_destroys = destroy_count; + } + if (total_destroys != 16 || alloc_count != 16) + { + printf("[ERROR] repeated failure: alloc=%d destroy=%d (expected 16/16)\n", + alloc_count, total_destroys); + failures++; + return; + } + printf("[SUCCESS] repeated failure: 16 alloc / 16 destroy, no leaks\n"); +} + +/* Probe: success then failure interleaved. Verifies the + * post-fix shape composes cleanly under realistic call + * sequences (e.g., hot-replug of a display). */ +static void test_postfix_interleaved(void) +{ + reset_counters(); + if ( simulate_vulkan_init(mock_set_video_mode_postfix, false) /* ok */ + || !simulate_vulkan_init(mock_set_video_mode_postfix, false) /* unreachable */) + { + /* The tautology above is just for readability; the real + * pattern: success returns true, failure returns false. */ + } + /* Simulate: success, failure, success, failure -- 4 cycles. */ + reset_counters(); + { + int n = 0; + n += simulate_vulkan_init(mock_set_video_mode_postfix, false) ? 1 : 0; + n += simulate_vulkan_init(mock_set_video_mode_postfix, true) ? 1 : 0; + n += simulate_vulkan_init(mock_set_video_mode_postfix, false) ? 1 : 0; + n += simulate_vulkan_init(mock_set_video_mode_postfix, true) ? 1 : 0; + if (n != 2) + { + printf("[ERROR] interleaved: success count = %d, expected 2\n", n); + failures++; + return; + } + } + if (alloc_count != 4 || destroy_count != 4) + { + printf("[ERROR] interleaved: alloc=%d destroy=%d (expected 4/4)\n", + alloc_count, destroy_count); + failures++; + return; + } + printf("[SUCCESS] interleaved success/failure: 4 alloc / 4 destroy\n"); +} + +/* Probe: the test harness itself can detect the pre-fix shape. + * Run simulate_vulkan_init pointing at mock_set_video_mode_prefix. + * Under ASan this WILL trip heap-use-after-free + double-free + * inside the upper-layer destroy(). We can't actually let it + * run -- ASan would abort the process and the rest of main() + * never runs. So this probe is intentionally compiled but + * never invoked from main() in normal test runs. Keeping it + * as a documented dispatch path makes the pre/post-fix + * discrimination explicit: a maintainer can swap which + * function main() calls and confirm ASan trips on pre-fix. */ +static void test_demonstration_prefix_would_uaf(void) +{ + /* Intentionally unused in normal runs; see comment above. */ + (void)mock_set_video_mode_prefix; +} + +int main(void) +{ + test_postfix_success(); + test_postfix_inner_failure(); + test_postfix_repeated_failure(); + test_postfix_interleaved(); + test_demonstration_prefix_would_uaf(); + + if (failures) + { + printf("\n%d vulkan_ctx_double_free test(s) failed\n", failures); + return 1; + } + printf("\nAll vulkan_ctx_double_free regression tests passed.\n"); + return 0; +} From aa4a196115755771eea26ad4918196e3ed0dd941 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:27:28 +0200 Subject: [PATCH 09/17] Fix warnings --- gfx/video_filters/dedither.c | 6 ++++++ gfx/video_filters/pixel_art_aa.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/gfx/video_filters/dedither.c b/gfx/video_filters/dedither.c index d0c7f0093288..59069b6a4075 100644 --- a/gfx/video_filters/dedither.c +++ b/gfx/video_filters/dedither.c @@ -208,3 +208,9 @@ const struct softfilter_implementation *softfilter_get_implementation(softfilter { return &dedither_generic; } + +#ifdef RARCH_INTERNAL +#undef softfilter_get_implementation +#undef softfilter_thread_data +#undef filter_data +#endif diff --git a/gfx/video_filters/pixel_art_aa.c b/gfx/video_filters/pixel_art_aa.c index 7ae19f3386c3..605afe325b7f 100644 --- a/gfx/video_filters/pixel_art_aa.c +++ b/gfx/video_filters/pixel_art_aa.c @@ -171,3 +171,9 @@ static const struct softfilter_implementation paa_impl = { const struct softfilter_implementation *softfilter_get_implementation(softfilter_simd_mask_t simd) { return &paa_impl; } + +#ifdef RARCH_INTERNAL +#undef softfilter_get_implementation +#undef softfilter_thread_data +#undef filter_data +#endif From b9777c8307d610e86bdbc5dd3ef86867473e11ca Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:34:34 +0200 Subject: [PATCH 10/17] CI: build full RetroArch with ASan + UBSan and run headless smoke Stands up a complementary CI job to the per-sample regression-test workflows shipped in v3-v6 (Linux-samples-gfx.yml). Builds full RetroArch with -fsanitize=address,undefined via the existing top- level Makefile SANITIZER= plumbing, then runs `./retroarch --help` and `./retroarch --features` under AddressSanitizer's abort-on-error mode and UBSan in soft-fail mode. The per-sample harnesses regression-test specific predicates that had bugs and by design only cover code that can be exercised by a small standalone test program with mocked dependencies. This job is complementary -- it sanitizer-instruments the entire RetroArch binary as it actually links and runs, so any future heap-corruption / use-after-free / signed-overflow / null-deref bug in code reachable from main() is caught for free as a side effect of any change. Same template as c82db9f's libretro-db samples build under sanitizer in CI, scaled up to the full retroarch binary. * .github/workflows/Linux-asan-ubsan.yml - Build matches Linux-Headless.yml's apt set and --disable-menu choice. Adds --disable-discord --disable-cheevos --disable-networking on the first iteration to shrink the third-party surface so that any sanitizer hit is a RetroArch- internal bug rather than noise from a vendored subsystem. Each can be re-enabled in follow-ups as the baseline stays green. - Build with `make SANITIZER=address,undefined`, which the top- level Makefile (line 153) propagates into CFLAGS / CXXFLAGS / LDFLAGS for every TU and the final link. No new apt packages: libasan / libubsan ship with the default gcc on ubuntu-latest. - Smoke runs --help and --features. Both exit(0) directly after printing. Coverage scope is honest in the inline comment: libc init, main(), frontend driver bootstrap, argv duplication, the getopt walk over the full option table, the print functions themselves, and the static feature-table walks --features performs. Doesn't reach core loading, video init, or cleanup- on-shutdown -- a follow-up step running with --max-frames=N against a noop core can extend coverage to the full lifecycle, but adding the noop-core dependency to CI is a separate step better handled in its own patch. - Sanitizer settings: ASAN_OPTIONS=abort_on_error=1:detect_leaks=0: print_stacktrace=1:strict_string_checks=1 UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0 LSAN_OPTIONS=exitcode=0 ASan in abort-on-error: any heap corruption fails the job immediately. Leak detection off for now -- libGL / Mesa / Wayland symbol-resolution drives a non-trivial baseline of "leaks" at process shutdown that aren't RetroArch bugs; can revisit with a suppression file once the rest is clean. UBSan in report-only: the first iteration is a discovery run. Pre-existing signed-overflow / alignment / shift-too-large diagnostics in audio resampling, color math, and vendored deps are likely to be present. The "Summarize UBSan baseline" step prints the deduplicated list so follow-up patches have a target to drive to zero. Once that count reaches zero, flip halt_on_error=1 in the smoke steps to enforce. Each smoke step independently greps stderr for "AddressSanitizer:" and exits 1 on any match -- belt-and- suspenders to abort_on_error in case a future ASan release changes its default exit semantics. - Kept as its own workflow rather than folded into Linux-Headless so the existing green-path build smoke stays fast (no ~3-5x ASan slowdown), and so this one can own its longer 25-minute timeout independently. - Trigger matches the per-sample workflows: push to master, pull_request to master, workflow_dispatch. --- .github/workflows/Linux-asan-ubsan.yml | 170 +++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 .github/workflows/Linux-asan-ubsan.yml diff --git a/.github/workflows/Linux-asan-ubsan.yml b/.github/workflows/Linux-asan-ubsan.yml new file mode 100644 index 000000000000..c0085071176e --- /dev/null +++ b/.github/workflows/Linux-asan-ubsan.yml @@ -0,0 +1,170 @@ +name: CI Linux ASan + UBSan [No Menu] + +# Builds full RetroArch with -fsanitize=address,undefined and runs +# headless smoke invocations (--help, --features) so AddressSanitizer +# and UndefinedBehaviorSanitizer instrument the startup config-loading, +# argument parsing, default-driver init, and cleanup-on-exit paths. +# +# The per-sample tests under .github/workflows/Linux-samples-gfx.yml, +# Linux-samples-tasks.yml, and Linux-libretro-{db,common}-samples.yml +# regression-test specific predicates that previously had bugs. This +# job is complementary -- it covers everything those harnesses can't +# reach because the code only runs from main(), and catches future +# heap-corruption / UB regressions across the whole code base for +# free as a side effect of any change that can be exercised by a +# headless run. +# +# Build configuration matches Linux-Headless.yml (--disable-menu) with +# additional --disable-discord --disable-cheevos --disable-networking +# to shrink the third-party surface on this first iteration. Each can +# be re-enabled once the baseline is green. + +on: + push: + branches: + - master + pull_request: + branches: + - master + workflow_dispatch: + +permissions: + contents: read + +env: + ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true + +jobs: + asan-ubsan: + name: Build with ASan+UBSan and run headless smoke + runs-on: ubuntu-latest + timeout-minutes: 25 + + steps: + - name: Install dependencies + # Mirrors Linux-Headless.yml's apt set so the build matches a + # known-good headless configuration. No sanitizer-specific + # packages required; libasan / libubsan ship with the gcc that + # ubuntu-latest has installed by default. + run: | + sudo apt-get update -y + sudo apt-get install -y \ + build-essential \ + libxkbcommon-dev libx11-xcb-dev \ + zlib1g-dev libfreetype6-dev \ + libegl1-mesa-dev libgles2-mesa-dev libgbm-dev \ + nvidia-cg-toolkit nvidia-cg-dev \ + libavcodec-dev libsdl2-dev libsdl-image1.2-dev \ + libxml2-dev yasm + + - name: Checkout + uses: actions/checkout@v3 + + - name: Configure (no menu, no discord/cheevos/networking) + # Trim the build surface for the first iteration so any + # sanitizer hit is a RetroArch-internal bug rather than noise + # from a vendored third-party subsystem. The disabled + # subsystems will be re-enabled in follow-up patches as the + # baseline stays green. + run: | + ./configure \ + --disable-menu \ + --disable-discord \ + --disable-cheevos \ + --disable-networking + + - name: Build with -fsanitize=address,undefined + # The top-level Makefile (line 153) propagates SANITIZER into + # CFLAGS / CXXFLAGS / LDFLAGS for every translation unit and + # the final link. ASan defaults to abort-on-heap-corruption; + # UBSan recovery is controlled at runtime via UBSAN_OPTIONS + # (see the smoke-run steps below). + run: | + make -j$(getconf _NPROCESSORS_ONLN) \ + SANITIZER=address,undefined + test -x retroarch + file retroarch + + - name: Smoke run --help (ASan strict, UBSan soft-fail) + # `--help` exits cleanly via exit(0) after printing the usage + # banner. Coverage scope: libc init, main(), frontend + # driver bootstrap, argv duplication, the getopt walk over + # the full option table, and retroarch_print_help() itself. + # Doesn't reach into core loading / video init / cleanup-on- + # shutdown -- a follow-up step running with --max-frames=N + # against a noop core will extend coverage to the full + # lifecycle. ASan runs in abort-on-error mode so any heap + # corruption fails the job; UBSan reports stacktraces but + # does not fail (first iteration -- we want to see the + # baseline before deciding which UBSan classes to enforce). + env: + ASAN_OPTIONS: abort_on_error=1:detect_leaks=0:print_stacktrace=1:strict_string_checks=1 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=0 + # Avoid noise from libGL / Mesa / Wayland symbol-resolution + # leaks at process shutdown; those are not RetroArch bugs. + LSAN_OPTIONS: exitcode=0 + run: | + set -eu + timeout 30 ./retroarch --help > /tmp/help.out 2> /tmp/help.err || rc=$? + echo "exit=${rc:-0}" + echo "=== stdout (head) ===" + head -40 /tmp/help.out + echo "=== stderr ===" + cat /tmp/help.err + # ASan abort_on_error sends the process to exit code 1 + a + # SUMMARY: line on stderr. Treat any "AddressSanitizer:" + # marker as fatal regardless of exit code. + if grep -q "AddressSanitizer:" /tmp/help.err; then + echo "[FAIL] AddressSanitizer reported a finding" + exit 1 + fi + echo "[pass] retroarch --help under ASan+UBSan" + + - name: Smoke run --features (ASan strict, UBSan soft-fail) + # `--features` exits cleanly via exit(0) after printing the + # compile-time feature list. Coverage scope is the same as + # --help (libc init / main / frontend bootstrap / arg parse) + # plus retroarch_print_features(), which walks the static + # video / audio / input / camera / location / record / cheats + # / network / database / overlay / hid feature tables. Each + # such walk reads pointers into a static-string table -- low + # corruption surface but a useful sanity check that the link- + # time feature flags resolve consistently. Doesn't reach + # core loading or cleanup-on-shutdown. + env: + ASAN_OPTIONS: abort_on_error=1:detect_leaks=0:print_stacktrace=1:strict_string_checks=1 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=0 + LSAN_OPTIONS: exitcode=0 + run: | + set -eu + timeout 30 ./retroarch --features > /tmp/features.out 2> /tmp/features.err || rc=$? + echo "exit=${rc:-0}" + echo "=== stdout (head) ===" + head -40 /tmp/features.out + echo "=== stderr ===" + cat /tmp/features.err + if grep -q "AddressSanitizer:" /tmp/features.err; then + echo "[FAIL] AddressSanitizer reported a finding" + exit 1 + fi + echo "[pass] retroarch --features under ASan+UBSan" + + - name: Summarize UBSan baseline + # Both runtime steps are non-fatal on UBSan diagnostics. This + # step prints a count of distinct UBSan messages observed so + # follow-up patches have a numerical target to drive to zero. + # If this step ever shows zero, flip + # UBSAN_OPTIONS=halt_on_error=1 in the runtime steps above to + # enforce the clean baseline. + if: always() + run: | + set +e + ub_count=$( { + grep -h "runtime error:" /tmp/help.err /tmp/features.err 2>/dev/null + } | sort -u | wc -l) + echo "UBSan distinct diagnostics: ${ub_count}" + if [ "${ub_count}" != "0" ]; then + echo "=== unique UBSan diagnostics ===" + grep -h "runtime error:" /tmp/help.err /tmp/features.err 2>/dev/null \ + | sort -u + fi From 60c7e3971006807923895ef7c8f36b2e1b80efcf Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:46:14 +0200 Subject: [PATCH 11/17] menu_displaylist: Hide History/Images/Music/Videos entries when history is disabled When 'Settings -> Playlists -> History' (history_list_enable) is OFF, CMD_EVENT_HISTORY_INIT early-returns and leaves g_defaults.content_history, image_history, music_history, and video_history all NULL. The Playlists tab generator was still appending entries for these with paths obtained via playlist_get_conf_path(NULL), which returns NULL and renders as a blank title. The icons and localized sublabels remained, producing four "empty" rows in Main Menu -> Playlists. Gate the four append calls additionally on the corresponding g_defaults.*_history pointer being non-NULL. This also covers the secondary early-return path in CMD_EVENT_HISTORY_INIT when content_history_size is 0. Mirror the same guard for the History entry in the rgui/glui (no nav bar) branch. Favorites is unaffected: g_defaults.content_favorites is initialized independently of history_list_enable. --- menu/menu_displaylist.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/menu/menu_displaylist.c b/menu/menu_displaylist.c index b64637f81fb0..3cb0434f8729 100644 --- a/menu/menu_displaylist.c +++ b/menu/menu_displaylist.c @@ -4229,7 +4229,7 @@ static unsigned menu_displaylist_parse_playlists( MENU_SETTING_ACTION, 0, 0, NULL)) count++; - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && g_defaults.content_history) if (menu_entries_append(info_list, playlist_get_conf_path(g_defaults.content_history), MENU_ENUM_LABEL_LOAD_CONTENT_HISTORY_STR, @@ -4239,7 +4239,7 @@ static unsigned menu_displaylist_parse_playlists( } else { - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && g_defaults.content_history) if (menu_entries_append(info_list, playlist_get_conf_path(g_defaults.content_history), MENU_ENUM_LABEL_LOAD_CONTENT_HISTORY_STR, @@ -4258,7 +4258,7 @@ static unsigned menu_displaylist_parse_playlists( } #ifdef HAVE_IMAGEVIEWER - if (settings->bools.menu_content_show_images) + if (settings->bools.menu_content_show_images && g_defaults.image_history) if (menu_entries_append(info_list, playlist_get_conf_path(g_defaults.image_history), MENU_ENUM_LABEL_GOTO_IMAGES_STR, @@ -4267,7 +4267,7 @@ static unsigned menu_displaylist_parse_playlists( count++; #endif - if (settings->bools.menu_content_show_music) + if (settings->bools.menu_content_show_music && g_defaults.music_history) if (menu_entries_append(info_list, playlist_get_conf_path(g_defaults.music_history), MENU_ENUM_LABEL_GOTO_MUSIC_STR, @@ -4276,7 +4276,7 @@ static unsigned menu_displaylist_parse_playlists( count++; #if defined(HAVE_FFMPEG) || defined(HAVE_MPV) - if (settings->bools.menu_content_show_video) + if (settings->bools.menu_content_show_video && g_defaults.video_history) if (menu_entries_append(info_list, playlist_get_conf_path(g_defaults.video_history), MENU_ENUM_LABEL_GOTO_VIDEO_STR, @@ -15368,7 +15368,7 @@ bool menu_displaylist_ctl(enum menu_displaylist_ctl_state type, MENU_SETTING_ACTION, 0, 0, NULL)) count++; - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && g_defaults.content_history) if (menu_entries_append(info->list, playlist_get_conf_path(g_defaults.content_history), MENU_ENUM_LABEL_LOAD_CONTENT_HISTORY_STR, @@ -15378,7 +15378,7 @@ bool menu_displaylist_ctl(enum menu_displaylist_ctl_state type, } else { - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && g_defaults.content_history) if (menu_entries_append(info->list, playlist_get_conf_path(g_defaults.content_history), MENU_ENUM_LABEL_LOAD_CONTENT_HISTORY_STR, From 70f24be5a57a107f8be2cdc2c59880c6761c3c7b Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:53:40 +0200 Subject: [PATCH 12/17] CI: enforce UBSan strict on ASan+UBSan workflow after clean baseline Follow-up to b9777c8 (the ASan+UBSan workflow). Run #1 of the new job came back clean: zero distinct UBSan diagnostics across the --help and --features smoke invocations. https://github.com/libretro/RetroArch/actions/runs/25056073385 * .github/workflows/Linux-asan-ubsan.yml - Flip UBSAN_OPTIONS=halt_on_error=0 to halt_on_error=1 in both smoke steps. A future change that introduces signed-overflow / alignment / shift-too-large UB along the option-parsing or print paths will now fail the run instead of being silently logged in a discovery summary. - Add a parallel grep -q "runtime error:" check after each timeout invocation so the failure is visible at exit code level even if a future sanitizer release changes the default abort semantics, mirroring the existing "AddressSanitizer:" check. - Drop the "Summarize UBSan baseline" step. Its job was to surface diagnostics during the discovery phase; with strict enforcement above it any non-zero count would have already aborted the run, so the summary is dead weight that suggested the workflow was still in soft-fail mode. - Step name updates: "ASan strict, UBSan soft-fail" -> "ASan + UBSan strict" on both smoke runs. Honest scoping note unchanged from b9777c8: --help and --features both exit(0) directly without going through the normal shutdown path, so this enforces UBSan-cleanliness only for libc init / main / frontend bootstrap / option parsing / the print functions. Audio resampling, color math, parsers, and full-lifecycle cleanup still aren't covered. The follow-up step running with --max-frames=N against a noop core (deferred from b9777c8) will extend coverage, and that step's first run will start in soft-fail mode the same way this one did. --- .github/workflows/Linux-asan-ubsan.yml | 56 ++++++++++++-------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/.github/workflows/Linux-asan-ubsan.yml b/.github/workflows/Linux-asan-ubsan.yml index c0085071176e..f0a7c165f577 100644 --- a/.github/workflows/Linux-asan-ubsan.yml +++ b/.github/workflows/Linux-asan-ubsan.yml @@ -85,7 +85,7 @@ jobs: test -x retroarch file retroarch - - name: Smoke run --help (ASan strict, UBSan soft-fail) + - name: Smoke run --help (ASan + UBSan strict) # `--help` exits cleanly via exit(0) after printing the usage # banner. Coverage scope: libc init, main(), frontend # driver bootstrap, argv duplication, the getopt walk over @@ -93,13 +93,17 @@ jobs: # Doesn't reach into core loading / video init / cleanup-on- # shutdown -- a follow-up step running with --max-frames=N # against a noop core will extend coverage to the full - # lifecycle. ASan runs in abort-on-error mode so any heap - # corruption fails the job; UBSan reports stacktraces but - # does not fail (first iteration -- we want to see the - # baseline before deciding which UBSan classes to enforce). + # lifecycle. ASan and UBSan both run in halt-on-error mode: + # the first run of this workflow (Apr 28 2026, run #1) + # reported zero distinct UBSan diagnostics across both + # smoke invocations, so the baseline for these two surfaces + # is clean and we enforce it. If a future change introduces + # signed-overflow / alignment / shift-too-large UB along + # the option-parsing or print paths, this step will fail + # and the diagnostic line will be in the captured stderr. env: ASAN_OPTIONS: abort_on_error=1:detect_leaks=0:print_stacktrace=1:strict_string_checks=1 - UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=0 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 # Avoid noise from libGL / Mesa / Wayland symbol-resolution # leaks at process shutdown; those are not RetroArch bugs. LSAN_OPTIONS: exitcode=0 @@ -113,14 +117,21 @@ jobs: cat /tmp/help.err # ASan abort_on_error sends the process to exit code 1 + a # SUMMARY: line on stderr. Treat any "AddressSanitizer:" - # marker as fatal regardless of exit code. + # or UBSan "runtime error:" marker as fatal regardless of + # exit code -- belt-and-suspenders to the runtime options + # in case a future sanitizer release changes its default + # exit semantics. if grep -q "AddressSanitizer:" /tmp/help.err; then echo "[FAIL] AddressSanitizer reported a finding" exit 1 fi + if grep -q "runtime error:" /tmp/help.err; then + echo "[FAIL] UBSan reported a finding" + exit 1 + fi echo "[pass] retroarch --help under ASan+UBSan" - - name: Smoke run --features (ASan strict, UBSan soft-fail) + - name: Smoke run --features (ASan + UBSan strict) # `--features` exits cleanly via exit(0) after printing the # compile-time feature list. Coverage scope is the same as # --help (libc init / main / frontend bootstrap / arg parse) @@ -130,10 +141,11 @@ jobs: # such walk reads pointers into a static-string table -- low # corruption surface but a useful sanity check that the link- # time feature flags resolve consistently. Doesn't reach - # core loading or cleanup-on-shutdown. + # core loading or cleanup-on-shutdown. Like the --help step + # this enforces UBSan strict because the baseline is clean. env: ASAN_OPTIONS: abort_on_error=1:detect_leaks=0:print_stacktrace=1:strict_string_checks=1 - UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=0 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 LSAN_OPTIONS: exitcode=0 run: | set -eu @@ -147,24 +159,8 @@ jobs: echo "[FAIL] AddressSanitizer reported a finding" exit 1 fi - echo "[pass] retroarch --features under ASan+UBSan" - - - name: Summarize UBSan baseline - # Both runtime steps are non-fatal on UBSan diagnostics. This - # step prints a count of distinct UBSan messages observed so - # follow-up patches have a numerical target to drive to zero. - # If this step ever shows zero, flip - # UBSAN_OPTIONS=halt_on_error=1 in the runtime steps above to - # enforce the clean baseline. - if: always() - run: | - set +e - ub_count=$( { - grep -h "runtime error:" /tmp/help.err /tmp/features.err 2>/dev/null - } | sort -u | wc -l) - echo "UBSan distinct diagnostics: ${ub_count}" - if [ "${ub_count}" != "0" ]; then - echo "=== unique UBSan diagnostics ===" - grep -h "runtime error:" /tmp/help.err /tmp/features.err 2>/dev/null \ - | sort -u + if grep -q "runtime error:" /tmp/features.err; then + echo "[FAIL] UBSan reported a finding" + exit 1 fi + echo "[pass] retroarch --features under ASan+UBSan" From bdcf692d244e1969c4c74330629ffa77240cac5d Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 15:56:26 +0200 Subject: [PATCH 13/17] ozone+menu_setting: Hide History/Images/Music/Video tabs when history is disabled The ozone sidebar exposes History, Images, Music, and Video tabs whose content is populated exclusively from the playlists managed by CMD_EVENT_HISTORY_INIT. When history_list_enable is OFF those playlists are never initialized, so the tabs become permanent dead ends - selecting History, for example, just shows "No History Available" forever. Gate the four tabs in ozone_refresh_system_tabs_list on history_list_enable in addition to the existing menu_content_show_* flags. Favorites is unaffected (independent setting). Also wire MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into the MENU_ENVIRON_RESET_HORIZONTAL_LIST dispatch in general_write_handler so toggling the setting refreshes the tab list immediately rather than requiring a restart, matching the behaviour of the neighbouring menu_content_show_* toggles. --- menu/drivers/ozone.c | 10 +++++----- menu/menu_setting.c | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/menu/drivers/ozone.c b/menu/drivers/ozone.c index 04c089556469..37b0fb48405e 100644 --- a/menu/drivers/ozone.c +++ b/menu/drivers/ozone.c @@ -5283,27 +5283,27 @@ static void ozone_refresh_system_tabs_list(ozone_handle_t * ozone) { if (settings->bools.menu_content_show_favorites) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_FAVORITES; - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && settings->bools.history_list_enable) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_HISTORY; } else { - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && settings->bools.history_list_enable) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_HISTORY; if (settings->bools.menu_content_show_favorites) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_FAVORITES; } #ifdef HAVE_IMAGEVIEWER - if (settings->bools.menu_content_show_images) + if (settings->bools.menu_content_show_images && settings->bools.history_list_enable) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_IMAGES; #endif - if (settings->bools.menu_content_show_music) + if (settings->bools.menu_content_show_music && settings->bools.history_list_enable) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_MUSIC; #if defined(HAVE_FFMPEG) || defined(HAVE_MPV) - if (settings->bools.menu_content_show_video) + if (settings->bools.menu_content_show_video && settings->bools.history_list_enable) ozone->tabs[++ozone->system_tab_end] = OZONE_SYSTEM_TAB_VIDEO; #endif diff --git a/menu/menu_setting.c b/menu/menu_setting.c index 1fc6a6401d8e..6e3146a9130a 100644 --- a/menu/menu_setting.c +++ b/menu/menu_setting.c @@ -9322,6 +9322,7 @@ static void general_write_handler(rarch_setting_t *setting) case MENU_ENUM_LABEL_CONTENT_SHOW_PLAYLIST_TABS: case MENU_ENUM_LABEL_CONTENT_SHOW_EXPLORE: case MENU_ENUM_LABEL_CONTENT_SHOW_CONTENTLESS_CORES: + case MENU_ENUM_LABEL_HISTORY_LIST_ENABLE: { struct menu_state *menu_st = menu_state_get_ptr(); if (menu_st->driver_ctx->environ_cb) From 5f0afac80f5117c5042812ec2c5ac8d264e004c0 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 16:09:27 +0200 Subject: [PATCH 14/17] xmb+menu_setting: Hide History/Images/Music/Video tabs when history is disabled, refresh on toggle Apply the same fix as ozone (bdcf692) to the XMB driver: gate the History, Images, Music, and Video system tabs in xmb_refresh_system_tabs_list on history_list_enable in addition to the existing menu_content_show_* flags. Without history tracking these tabs are permanent dead ends with no possibility of content. Favorites is unaffected (independent setting). Additionally, fix the live-toggle behaviour. The previous patch wired MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into the MENU_ENVIRON_RESET_HORIZONTAL_LIST dispatch but stale state in g_defaults.{content,image,music,video}_history meant that the gating logic in menu_displaylist.c (which checks both the menu_content_show_* flag and the corresponding playlist pointer) read the wrong values until the next launch. As a result, the materialui Playlists screen did not visibly update when toggling the setting. Split MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into its own case in general_write_handler that fires CMD_EVENT_HISTORY_INIT (which internally deinits first, then early-returns when the new setting value is OFF), keeping g_defaults pointers in sync with the setting. Also set MENU_ST_FLAG_ENTRIES_NEED_REFRESH so the currently-displayed displaylist is rebuilt - this is what makes materialui's Playlists screen update on toggle, since materialui has no horizontal tab bar of its own to refresh. --- menu/drivers/xmb.c | 10 +++++----- menu/menu_setting.c | 13 +++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/menu/drivers/xmb.c b/menu/drivers/xmb.c index f8e415c2e075..6a7acd681e4c 100644 --- a/menu/drivers/xmb.c +++ b/menu/drivers/xmb.c @@ -3179,27 +3179,27 @@ static void xmb_refresh_system_tabs_list(xmb_handle_t *xmb) { if (settings->bools.menu_content_show_favorites) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_FAVORITES; - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && settings->bools.history_list_enable) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_HISTORY; } else { - if (settings->bools.menu_content_show_history) + if (settings->bools.menu_content_show_history && settings->bools.history_list_enable) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_HISTORY; if (settings->bools.menu_content_show_favorites) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_FAVORITES; } #ifdef HAVE_IMAGEVIEWER - if (settings->bools.menu_content_show_images) + if (settings->bools.menu_content_show_images && settings->bools.history_list_enable) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_IMAGES; #endif - if (settings->bools.menu_content_show_music) + if (settings->bools.menu_content_show_music && settings->bools.history_list_enable) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_MUSIC; #if defined(HAVE_FFMPEG) || defined(HAVE_MPV) - if (settings->bools.menu_content_show_video) + if (settings->bools.menu_content_show_video && settings->bools.history_list_enable) xmb->tabs[++xmb->system_tab_end] = XMB_SYSTEM_TAB_VIDEO; #endif diff --git a/menu/menu_setting.c b/menu/menu_setting.c index 6e3146a9130a..690fa44fcbf7 100644 --- a/menu/menu_setting.c +++ b/menu/menu_setting.c @@ -9322,12 +9322,25 @@ static void general_write_handler(rarch_setting_t *setting) case MENU_ENUM_LABEL_CONTENT_SHOW_PLAYLIST_TABS: case MENU_ENUM_LABEL_CONTENT_SHOW_EXPLORE: case MENU_ENUM_LABEL_CONTENT_SHOW_CONTENTLESS_CORES: + { + struct menu_state *menu_st = menu_state_get_ptr(); + if (menu_st->driver_ctx->environ_cb) + menu_st->driver_ctx->environ_cb(MENU_ENVIRON_RESET_HORIZONTAL_LIST, + NULL, menu_st->userdata); + } + break; case MENU_ENUM_LABEL_HISTORY_LIST_ENABLE: { struct menu_state *menu_st = menu_state_get_ptr(); + /* Sync history playlist init state to the new setting value + * (HISTORY_INIT internally calls HISTORY_DEINIT first, then + * early-returns if history_list_enable is now OFF). */ + command_event(CMD_EVENT_HISTORY_INIT, NULL); if (menu_st->driver_ctx->environ_cb) menu_st->driver_ctx->environ_cb(MENU_ENVIRON_RESET_HORIZONTAL_LIST, NULL, menu_st->userdata); + menu_st->flags |= MENU_ST_FLAG_PREVENT_POPULATE + | MENU_ST_FLAG_ENTRIES_NEED_REFRESH; } break; case MENU_ENUM_LABEL_SUSPEND_SCREENSAVER_ENABLE: From d1228e6854469b3049fd6bbd39f1148e8bf74aea Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 17:08:41 +0200 Subject: [PATCH 15/17] win32+core_updater: fix taskbar progress overlay during downloads The Win32 taskbar progress overlay (the green bar drawn on the RetroArch taskbar icon during downloads) never appeared. Two independent bugs were keeping it dark: 1) wndproc: the 'TaskbarButtonCreated' message check was nested inside unrelated 'case' arms (WM_MOUSEMOVE..WM_NCLBUTTONDBLCLK, WM_DROPFILES..WM_COMMAND, and the WM_PAINT branch of the GDI wrappers). Since 'TaskbarButtonCreated' is a registered window message with its own dynamically-allocated value, those arms could never match it, so WIN32_CMN_FLAG_TASKBAR_CREATED was never set and dispserv_win32's set_window_progress() always early-outed without calling ITaskbarList3::SetProgressValue. Move the comparison to the top of each dispatcher (wnd_proc_common, wnd_proc_common_internal, wnd_proc_winraw_common_internal, wnd_proc_common_dinput_internal) so it sees every incoming message. Short-circuit on the flag bit so the comparison only runs until the message arrives once. The HAVE_TASKBAR gate is unchanged; pre-Win7 builds either don't define it or fall back through the existing taskbar_list NULL guard in dispserv_win32. 2) task_core_updater: even with the flag set, the five outer aggregating tasks (get_list, single download, update_installed_cores, play_feature_delivery_install, play_feature_delivery_switch) never set a progress_cb. They spawn inner http transfers with mute=true to suppress per-core on-screen toasts, which also suppresses the inner tasks' progress_cb invocation in task_queue_push_progress(). Net result: video_display_server_set_window_progress() was never being called during a Core Updater run. Promote the existing http progress forwarder (http_transfer_progress_cb in task_http.c) to a public helper named task_window_progress_cb, declare it in tasks_internal.h, and wire it as the progress_cb of all five outer tasks. The aggregating tasks already drive task->progress in 0..100 across their state machines, so no other plumbing is needed. Single-zip update flows (Update Assets, Update Cheats, Update Databases, etc.) were already working because their underlying http transfer is unmuted and titled, and the existing progress_cb fired for those. --- gfx/common/win32_common.c | 74 +++++++++++++++------------------------ tasks/task_core_updater.c | 5 +++ tasks/task_http.c | 9 +++-- tasks/tasks_internal.h | 8 +++++ 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/gfx/common/win32_common.c b/gfx/common/win32_common.c index 432972decec4..ceaaa7014dcc 100644 --- a/gfx/common/win32_common.c +++ b/gfx/common/win32_common.c @@ -508,6 +508,14 @@ static LRESULT CALLBACK wnd_proc_common( bool *quit, HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) { +#ifdef HAVE_TASKBAR + win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; + if ( !(g_win32_flags & WIN32_CMN_FLAG_TASKBAR_CREATED) + && g_win32->taskbar_message + && message == g_win32->taskbar_message) + g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; +#endif + switch (message) { case WM_SYSCOMMAND: @@ -709,6 +717,13 @@ static LRESULT CALLBACK wnd_proc_common_internal(HWND hwnd, bool quit = false; win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; +#ifdef HAVE_TASKBAR + if ( !(g_win32_flags & WIN32_CMN_FLAG_TASKBAR_CREATED) + && g_win32->taskbar_message + && message == g_win32->taskbar_message) + g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; +#endif + switch (message) { case WM_KEYUP: /* Key released */ @@ -771,10 +786,6 @@ static LRESULT CALLBACK wnd_proc_common_internal(HWND hwnd, case WM_MOUSEWHEEL: case WM_MOUSEHWHEEL: case WM_NCLBUTTONDBLCLK: -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif break; case WM_DROPFILES: case WM_SYSCOMMAND: @@ -793,10 +804,6 @@ static LRESULT CALLBACK wnd_proc_common_internal(HWND hwnd, ret = wnd_proc_common(&quit, hwnd, message, wparam, lparam); if (quit) return ret; -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif break; case WM_SETFOCUS: #ifdef HAVE_CLIP_WINDOW @@ -830,6 +837,13 @@ static LRESULT CALLBACK wnd_proc_winraw_common_internal(HWND hwnd, bool quit = false; win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; +#ifdef HAVE_TASKBAR + if ( !(g_win32_flags & WIN32_CMN_FLAG_TASKBAR_CREATED) + && g_win32->taskbar_message + && message == g_win32->taskbar_message) + g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; +#endif + switch (message) { case WM_KEYUP: /* Key released */ @@ -857,10 +871,6 @@ static LRESULT CALLBACK wnd_proc_winraw_common_internal(HWND hwnd, case WM_MOUSEWHEEL: case WM_MOUSEHWHEEL: case WM_NCLBUTTONDBLCLK: -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif break; case WM_DROPFILES: case WM_SYSCOMMAND: @@ -879,10 +889,6 @@ static LRESULT CALLBACK wnd_proc_winraw_common_internal(HWND hwnd, ret = wnd_proc_common(&quit, hwnd, message, wparam, lparam); if (quit) return ret; -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif break; case WM_SETFOCUS: #ifdef HAVE_CLIP_WINDOW @@ -936,6 +942,13 @@ static LRESULT CALLBACK wnd_proc_common_dinput_internal(HWND hwnd, bool quit = false; win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; +#ifdef HAVE_TASKBAR + if ( !(g_win32_flags & WIN32_CMN_FLAG_TASKBAR_CREATED) + && g_win32->taskbar_message + && message == g_win32->taskbar_message) + g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; +#endif + switch (message) { case WM_IME_ENDCOMPOSITION: @@ -1074,10 +1087,6 @@ static LRESULT CALLBACK wnd_proc_common_dinput_internal(HWND hwnd, case WM_MOUSEWHEEL: case WM_MOUSEHWHEEL: case WM_NCLBUTTONDBLCLK: -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif #if !defined(_XBOX) { void* input_data = (void*)(LONG_PTR)GetWindowLongPtr(main_window.hwnd, GWLP_USERDATA); @@ -1104,10 +1113,6 @@ static LRESULT CALLBACK wnd_proc_common_dinput_internal(HWND hwnd, ret = wnd_proc_common(&quit, hwnd, message, wparam, lparam); if (quit) return ret; -#ifdef HAVE_TASKBAR - if (g_win32->taskbar_message && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif break; case WM_SETFOCUS: #ifdef HAVE_CLIP_WINDOW @@ -1342,7 +1347,6 @@ LRESULT CALLBACK wnd_proc_gdi_dinput(HWND hwnd, UINT message, return wnd_proc_wm_gdi_create(hwnd); else if (message == WM_PAINT) { - win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; gdi_t *gdi = (gdi_t*)video_driver_get_ptr(); if (gdi && gdi->memDC) @@ -1365,12 +1369,6 @@ LRESULT CALLBACK wnd_proc_gdi_dinput(HWND hwnd, UINT message, SelectObject(gdi->memDC, gdi->bmp_old); } - -#ifdef HAVE_TASKBAR - if ( g_win32->taskbar_message - && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif } return wnd_proc_common_dinput_internal(hwnd, message, wparam, lparam); @@ -1385,7 +1383,6 @@ LRESULT CALLBACK wnd_proc_gdi_winraw(HWND hwnd, UINT message, return wnd_proc_wm_gdi_create(hwnd); else if (message == WM_PAINT) { - win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; gdi_t *gdi = (gdi_t*)video_driver_get_ptr(); if (gdi && gdi->memDC) @@ -1408,12 +1405,6 @@ LRESULT CALLBACK wnd_proc_gdi_winraw(HWND hwnd, UINT message, SelectObject(gdi->memDC, gdi->bmp_old); } - -#ifdef HAVE_TASKBAR - if ( g_win32->taskbar_message - && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif } return wnd_proc_winraw_common_internal(hwnd, message, wparam, lparam); @@ -1427,7 +1418,6 @@ LRESULT CALLBACK wnd_proc_gdi_common(HWND hwnd, UINT message, return wnd_proc_wm_gdi_create(hwnd); else if (message == WM_PAINT) { - win32_common_state_t *g_win32 = (win32_common_state_t*)&win32_st; gdi_t *gdi = (gdi_t*)video_driver_get_ptr(); if (gdi && gdi->memDC) @@ -1450,12 +1440,6 @@ LRESULT CALLBACK wnd_proc_gdi_common(HWND hwnd, UINT message, SelectObject(gdi->memDC, gdi->bmp_old); } - -#ifdef HAVE_TASKBAR - if ( g_win32->taskbar_message - && message == g_win32->taskbar_message) - g_win32_flags |= WIN32_CMN_FLAG_TASKBAR_CREATED; -#endif } return wnd_proc_common_internal(hwnd, message, wparam, lparam); diff --git a/tasks/task_core_updater.c b/tasks/task_core_updater.c index 90b897b08434..33170b76225f 100644 --- a/tasks/task_core_updater.c +++ b/tasks/task_core_updater.c @@ -476,6 +476,7 @@ void *task_push_get_core_updater_list( task->state = list_handle; task->title = strdup(msg_hash_to_str(MSG_FETCHING_CORE_LIST)); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; if (mute) task->flags |= RETRO_TASK_FLG_MUTE; @@ -1118,6 +1119,7 @@ void *task_push_core_updater_download( task->state = download_handle; task->title = strdup(task_title); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->callback = cb_task_core_updater_download; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; if (mute) @@ -1547,6 +1549,7 @@ void task_push_update_installed_cores( task->state = update_installed_handle; task->title = strdup(msg_hash_to_str(MSG_FETCHING_CORE_LIST)); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; /* Push task */ @@ -1869,6 +1872,7 @@ void *task_push_play_feature_delivery_core_install( task->state = pfd_install_handle; task->title = strdup(task_title); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->callback = cb_task_core_updater_download; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; if (mute) @@ -2239,6 +2243,7 @@ void task_push_play_feature_delivery_switch_installed_cores( task->state = pfd_switch_cores_handle; task->title = strdup(msg_hash_to_str(MSG_SCANNING_CORES)); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; /* Push task */ diff --git a/tasks/task_http.c b/tasks/task_http.c index 713b99a05795..205eb474495c 100644 --- a/tasks/task_http.c +++ b/tasks/task_http.c @@ -243,7 +243,12 @@ static bool task_http_finder(retro_task_t *task, void *user_data) return false; } -static void http_transfer_progress_cb(retro_task_t *task) +/* Forward task progress to the platform's window/taskbar progress + * indicator (e.g. ITaskbarList3 on Win32). Exposed via tasks_internal.h + * so non-HTTP tasks (such as the Core Updater's outer aggregating tasks) + * can wire it as their progress_cb and have the desktop reflect their + * progress, the same way titled HTTP transfers already do. */ +void task_window_progress_cb(retro_task_t *task) { #ifdef RARCH_INTERNAL if (task) @@ -312,7 +317,7 @@ static void *task_push_http_transfer_generic_titled( t->handler = task_http_transfer_handler; t->state = http; t->callback = cb; - t->progress_cb = http_transfer_progress_cb; + t->progress_cb = task_window_progress_cb; t->cleanup = task_http_transfer_cleanup; t->user_data = user_data; t->progress = -1; diff --git a/tasks/tasks_internal.h b/tasks/tasks_internal.h index 0f9bbd532377..6ed3fbc03dae 100644 --- a/tasks/tasks_internal.h +++ b/tasks/tasks_internal.h @@ -61,6 +61,14 @@ typedef struct int status; } http_transfer_data_t; +/* Generic progress_cb that forwards a task's progress (0-100) to the + * platform's window/taskbar progress indicator. Set this on any task + * whose progress should be reflected on the taskbar -- aggregating + * tasks (e.g. the Core Updater's outer task) need to do this manually + * because their inner http transfers run muted and so their own + * progress callbacks never fire. */ +void task_window_progress_cb(retro_task_t *task); + void *task_push_http_transfer(const char *url, bool mute, const char *type, retro_task_callback_t cb, void *userdata); From 1f6c709ae6846fab946a0e16d99450ff94c45584 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 17:28:32 +0200 Subject: [PATCH 16/17] tasks: extend window/taskbar progress coverage Build on the task_window_progress_cb helper introduced in d1228e6 and propagate the Win32 taskbar progress overlay (and any other platform window progress hook) to more long-running user-visible tasks. Helper relocation: - Move the implementation from task_http.c (HAVE_NETWORKING-gated) to task_file_transfer.c (always built), so non-network tasks can use it without pulling in HAVE_NETWORKING. - Move the prototype in tasks_internal.h out of the HAVE_NETWORKING block for the same reason. - Drop the duplicated copy in task_database.c (task_database_progress_cb) in favour of the shared helper, and the now-unused video_display_server.h include alongside it. New coverage: - task_pl_thumbnail_download.c: the system-wide bulk thumbnail download task. Long, user-initiated, already drives task->progress monotonically across two outer phases. - task_cloudsync.c: cloud sync, which can be very long and reports progress as files are reconciled. - task_core_backup.c: both the backup and restore outer tasks, so user-initiated backup/restore of cores reflects on the taskbar. The single-entry pl_thumbnail outer task and short-running tasks (decompress, disc scan, playlist manager, menu explore) are left unchanged for now -- they finish too quickly for the taskbar indicator to be useful, and adding them is a trivial follow-up if that turns out to be wrong. --- tasks/task_cloudsync.c | 1 + tasks/task_core_backup.c | 3 +++ tasks/task_database.c | 12 ++---------- tasks/task_file_transfer.c | 21 +++++++++++++++++++++ tasks/task_http.c | 17 ----------------- tasks/task_pl_thumbnail_download.c | 1 + tasks/tasks_internal.h | 20 ++++++++++++-------- 7 files changed, 40 insertions(+), 35 deletions(-) diff --git a/tasks/task_cloudsync.c b/tasks/task_cloudsync.c index 3df3699053c5..849366a5e5e0 100644 --- a/tasks/task_cloudsync.c +++ b/tasks/task_cloudsync.c @@ -1439,6 +1439,7 @@ static void task_push_cloud_sync_with_mode(int conflict_resolution) task->title = strdup(task_title); task->handler = task_cloud_sync_task_handler; task->callback = task_cloud_sync_cb; + task->progress_cb = task_window_progress_cb; task_queue_push(task); } diff --git a/tasks/task_core_backup.c b/tasks/task_core_backup.c index 56beacb2281b..51f9ab9fed87 100644 --- a/tasks/task_core_backup.c +++ b/tasks/task_core_backup.c @@ -35,6 +35,7 @@ #include "../verbosity.h" #include "../core_info.h" #include "../core_backup.h" +#include "tasks_internal.h" #if defined(RARCH_INTERNAL) && defined(HAVE_MENU) #include "../menu/menu_driver.h" @@ -631,6 +632,7 @@ void *task_push_core_backup( task->state = backup_handle; task->title = strdup(task_title); task->progress = 0; + task->progress_cb = task_window_progress_cb; if (mute) task->flags |= RETRO_TASK_FLG_MUTE; @@ -1085,6 +1087,7 @@ bool task_push_core_restore(const char *backup_path, const char *dir_libretro, task->state = backup_handle; task->title = strdup(task_title); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->callback = cb_task_core_restore; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; diff --git a/tasks/task_database.c b/tasks/task_database.c index 3562217c2a4a..9ca3a6efa089 100644 --- a/tasks/task_database.c +++ b/tasks/task_database.c @@ -39,7 +39,6 @@ #include "../playlist.h" #include "../configuration.h" #include "../ui/ui_companion_driver.h" -#include "../gfx/video_display_server.h" #ifdef HAVE_MENU #include "../menu/menu_driver.h" #endif @@ -1667,13 +1666,6 @@ static void scan_results_batch_update_playlists(scan_results_t *sr, } #ifdef HAVE_LIBRETRODB -static void task_database_progress_cb(retro_task_t *task) -{ - if (task) - video_display_server_set_window_progress(task->progress, - ((task->flags & RETRO_TASK_FLG_FINISHED) > 0)); -} - bool task_push_dbscan( const char *playlist_directory, /* always from settings */ const char *content_database, /* always from settings */ @@ -2607,8 +2599,8 @@ bool task_push_manual_content_scan( task->state = manual_scan; task->title = strdup(task_title); task->progress = 0; -#ifdef HAVE_LIBRETRODB - task->progress_cb = task_database_progress_cb; +#ifdef HAVE_LIBRETRODB + task->progress_cb = task_window_progress_cb; #else task->progress_cb = NULL; #endif diff --git a/tasks/task_file_transfer.c b/tasks/task_file_transfer.c index 9cf378e669fd..bbf3415a8aca 100644 --- a/tasks/task_file_transfer.c +++ b/tasks/task_file_transfer.c @@ -24,8 +24,29 @@ #include "task_file_transfer.h" #include "tasks_internal.h" +#ifdef RARCH_INTERNAL +#include "../gfx/video_display_server.h" +#endif + bool task_image_load_handler(retro_task_t *task); +/* Forward task progress to the platform's window/taskbar progress + * indicator (e.g. ITaskbarList3 on Win32). Wire this as a task's + * progress_cb to have the desktop reflect that task's progress. + * + * Lives in this always-built TU (rather than the network-gated + * task_http.c where it originated) so non-network tasks -- core + * backup, manual content scan, etc. -- can use it without pulling + * in HAVE_NETWORKING. */ +void task_window_progress_cb(retro_task_t *task) +{ +#ifdef RARCH_INTERNAL + if (task) + video_display_server_set_window_progress(task->progress, + ((task->flags & RETRO_TASK_FLG_FINISHED) > 0)); +#endif +} + /* Default number of nbio_iterate() calls per handler tick. * Callers may override by setting nbio->pos_increment to a * non-zero value before queuing the task. A value of 0 (the diff --git a/tasks/task_http.c b/tasks/task_http.c index 205eb474495c..07ebd020aeca 100644 --- a/tasks/task_http.c +++ b/tasks/task_http.c @@ -23,9 +23,6 @@ #include #include -#ifdef RARCH_INTERNAL -#include "../gfx/video_display_server.h" -#endif #include "task_file_transfer.h" #include "tasks_internal.h" @@ -243,20 +240,6 @@ static bool task_http_finder(retro_task_t *task, void *user_data) return false; } -/* Forward task progress to the platform's window/taskbar progress - * indicator (e.g. ITaskbarList3 on Win32). Exposed via tasks_internal.h - * so non-HTTP tasks (such as the Core Updater's outer aggregating tasks) - * can wire it as their progress_cb and have the desktop reflect their - * progress, the same way titled HTTP transfers already do. */ -void task_window_progress_cb(retro_task_t *task) -{ -#ifdef RARCH_INTERNAL - if (task) - video_display_server_set_window_progress(task->progress, - ((task->flags & RETRO_TASK_FLG_FINISHED) > 0)); -#endif -} - static void *task_push_http_transfer_generic_titled( struct http_connection_t *conn, const char *url, bool mute, bool headers_accept_err, diff --git a/tasks/task_pl_thumbnail_download.c b/tasks/task_pl_thumbnail_download.c index 9afe48e70f0e..8f755d538bec 100644 --- a/tasks/task_pl_thumbnail_download.c +++ b/tasks/task_pl_thumbnail_download.c @@ -617,6 +617,7 @@ bool task_push_pl_thumbnail_download( task->state = pl_thumb; task->title = strdup(system); task->progress = 0; + task->progress_cb = task_window_progress_cb; task->flags |= RETRO_TASK_FLG_ALTERNATIVE_LOOK; task_queue_push(task); diff --git a/tasks/tasks_internal.h b/tasks/tasks_internal.h index 6ed3fbc03dae..5876be816bb1 100644 --- a/tasks/tasks_internal.h +++ b/tasks/tasks_internal.h @@ -52,6 +52,18 @@ typedef struct nbio_buf unsigned bufsize; } nbio_buf_t; +/* Generic progress_cb that forwards a task's progress (0-100) to the + * platform's window/taskbar progress indicator (e.g. ITaskbarList3 on + * Win32). Set this on any task whose progress should be reflected on + * the taskbar -- aggregating tasks (e.g. the Core Updater's outer + * task) need to do this manually because their inner http transfers + * run muted and so their own progress callbacks never fire. + * + * Available regardless of HAVE_NETWORKING: implementation lives in + * task_file_transfer.c so non-network long-running tasks (manual + * content scan, core backup, ...) can use it too. */ +void task_window_progress_cb(retro_task_t *task); + #ifdef HAVE_NETWORKING typedef struct { @@ -61,14 +73,6 @@ typedef struct int status; } http_transfer_data_t; -/* Generic progress_cb that forwards a task's progress (0-100) to the - * platform's window/taskbar progress indicator. Set this on any task - * whose progress should be reflected on the taskbar -- aggregating - * tasks (e.g. the Core Updater's outer task) need to do this manually - * because their inner http transfers run muted and so their own - * progress callbacks never fire. */ -void task_window_progress_cb(retro_task_t *task); - void *task_push_http_transfer(const char *url, bool mute, const char *type, retro_task_callback_t cb, void *userdata); From 832250be0aad44aebd192aa0903a7d10d4acd91f Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-SPFP6AQ\\twistedtechre" Date: Tue, 28 Apr 2026 17:37:11 +0200 Subject: [PATCH 17/17] Buildfix for CI --- samples/tasks/database/main.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/samples/tasks/database/main.c b/samples/tasks/database/main.c index 09771fe7bb08..9feedf5302e0 100644 --- a/samples/tasks/database/main.c +++ b/samples/tasks/database/main.c @@ -72,6 +72,18 @@ void video_display_server_set_window_progress(int progress, bool finished) (void)progress; (void)finished; } +/* task_database.c's progress_cb is now the shared task_window_progress_cb, + * whose definition lives in tasks/task_file_transfer.c. Pulling that + * file in would drag in nbio, the audio mixer, and the image-task + * machinery, none of which the dbscan path exercises. Stub it here + * to satisfy the linker; the function is never called on this code + * path because no progress_cb is invoked unless a worker thread + * publishes progress, and this sample never reaches that state. */ +void task_window_progress_cb(retro_task_t *task) +{ + (void)task; +} + uint64_t frontend_driver_get_free_memory(void) { return 0;