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.

Author: LibretroAdmin

.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"

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);
 

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))

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