drm/sunxi: fix RGUI menu color conversion and per-frame leak

The RGUI menu blit path in drm_gfx.c and sunxi_gfx.c (audit
findings #29 and #34, byte-for-byte clones of each other) had
several bugs that compounded into wrong menu colors, an
unbounded VLA, and -- on drm_gfx -- a malloc per menu frame
with no matching free.

Bugs in the bit extraction
==========================

The conversion treated the uint16_t source as if its layout
were RGB565 with naive 8-bit-channel shifts:

    R = (src_pix << 8) & 0x00FF0000;
    G = (src_pix << 4) & 0x0000FF00;
    B = (src_pix << 0) & 0x000000FF;
    line[j] = R | G | B;

Two problems with that:

1. The RGUI default output on these platforms is RGBA4444, not
   RGB565 (rgui_set_pixel_format_function in menu/drivers/rgui.c
   selects RGBA4444 for the unmatched-driver-name fallback that
   covers both drm and sunxi). The channel layout is
   R=15..12, G=11..8, B=7..4, A=3..0.

2. Even taken as RGB565, the shifts are wrong: they place 4-bit
   channel data into the low nibble of each 8-bit destination
   byte, so e.g. 0x000F (RGBA black, fully opaque) decodes to
   0x0000000F -- alpha bleeds into the blue byte. 0x00FF (RGBA
   pure blue) decodes to 0x00000FFF, leaking alpha across the
   bottom of green into blue.

Concretely, with the old code:

    rgba4444 input | old output  | correct XRGB8888
    ---------------+-------------+-----------------
    0xF00F (red)   | 0x00F0000F  | 0x00FF0000
    0x0F0F (green) | 0x000FF00F  | 0x0000FF00
    0x00FF (blue)  | 0x00000FFF  | 0x000000FF
    0x000F (black) | 0x0000000F  | 0x00000000
    0x888F (gray)  | 0x0088888F  | 0x00888888
    0xFFFF (white) | 0x00FFFFFF  | 0x00FFFFFF  (only correct case)

Pure white was the one input that came out right, which is
likely why the bug survived: a quick visual sanity check on
fully-saturated menu chrome could look plausible while every
other color was off and had stray bits in the unused alpha
byte of XRGB8888.

Fix: treat src_pix as RGBA4444, extract each 4-bit channel,
and expand to 8 bits via nibble replication (n | (n << 4)),
which is the standard 4-bit-to-8-bit upscaling that maps 0
to 0x00 and 0xF to 0xFF linearly.

The (currently ignored) rgb32 parameter
=======================================

Both functions accepted a `bool rgb32` from the caller and
discarded it. The set_texture_frame contract is: when rgb32
is true, the source is XRGB8888 already and no conversion
is required. The new code branches on rgb32 and does a
per-row memcpy in that case. RGUI does not currently exercise
this path on these drivers, but other set_texture_frame
callers do, and the parameter is part of the interface.

Per-frame malloc leak (drm only)
================================

drm_set_texture_frame did:

    char *frame_output = malloc(dst_pitch * height);
    /* ... fill it ... */
    drm_surface_update(_drmvars, frame_output, _drmvars->menu_surface);

drm_surface_update memcpys row-by-row from frame_output into
the dumb buffer at surface->pages[surface->flip_page].buf.map
(drm_gfx.c:328-331) and returns. frame_output is never freed
-- one full XRGB8888-frame leak per menu refresh.

The intermediate buffer is unnecessary. The destination is a
CPU-mapped DRM dumb buffer; we can write the converted pixels
directly into it. surface->pitch is the visible bytes per row
(src_width * bpp, set at drm_gfx.c:243) and matches the dst
stride for the menu surface, which was created with
pitch == width * 4 == surface->pitch. After writing in place,
call drm_page_flip directly -- this is the same sequence
drm_surface_update performs minus the now-redundant memcpy.

Stack-allocated VLA
===================

Both files declared

    uint32_t line[dst_width];

as a per-row scratch buffer, where dst_width was the menu
output width (drm) or the display xres (sunxi). On a 4K-wide
output that's 16 KiB on the stack per call, on platforms
(embedded ARM Linux) where thread stacks may be small. The
direct-into-mapped-buffer rewrite removes the need for any
scratch line entirely; pixels are produced one at a time
into their final destination slot.

sunxi: stale-stack read on width < xres
=======================================

sunxi_set_texture_frame additionally had a smaller bug: the
inner loop only filled line[0..width-1], but the row memcpy
copied dst_pitch == xres*4 bytes from line[]. When the source
was narrower than the display (e.g. RGUI rendering at a lower
resolution upscaled by the layer), the copy read uninitialized
stack bytes for pixels [width..xres-1] and pushed them into
the framebuffer. With the new direct-write path, only the
`width` actually-meaningful pixels per row are written; pixels
beyond `width` retain whatever was previously in the dumb
buffer (typically the prior frame's same-row content, which is
the correct behavior for a partial-width menu blit).

Defensive clamps on width/height
================================

The destination buffers (drm dumb buffer, sunxi framebuffer
page) are sized once and never resized while the menu is
active. The original drm code was structured around an
intermediate malloc whose size was driven by the surface's
src_height and total_pitch, which silently truncated rows if
the caller passed a larger frame than the surface was created
for. The original sunxi code allocated nothing and would have
walked off the end of pages[0] in the same scenario.

To preserve the no-overrun property of the new direct-write
path, both functions now clamp `width` and `height` against
the destination's known bounds (drm: surface->src_width /
surface->src_height; sunxi: sunxi_disp->xres / src_height)
before the loops. In normal operation these clamps are no-ops;
they exist purely so a future caller passing oversized
dimensions writes a truncated frame rather than overrunning
the mapped region.

Validation
==========

- Both functions compile cleanly under gcc -Wall -Wextra in
  isolation (no unused vars, no unused parameters beyond the
  pre-existing `alpha`).
- Conversion math validated against a tabulated reference for
  the six characteristic RGBA4444 inputs above, including the
  diagnostic that pure-white is the unique input where the old
  and new implementations agree.
- drm path: surface->pitch and surface->total_pitch semantics
  verified against drm_surface_setup (drm_gfx.c:222-278) and
  drm_surface_update (316-338). For the menu surface, both are
  width*4 because drm_surface_setup is called with
  pitch=width*4, bpp=4, src_width=width.
- drm flip semantics verified: drm_page_flip uses
  pages[flip_page] then toggles flip_page, so writing into
  pages[flip_page].buf.map before calling drm_page_flip puts
  the new pixels on the page that is about to be committed,
  matching what drm_surface_update did.
- sunxi flip semantics unchanged: same
  sunxi_layer_set_rgb_input_buffer call follows the in-place
  write, just as before.
- Behavioral validation via a synthetic harness that mocks the
  DRM dumb-buffer and sunxi page interfaces, drops in both the
  pre- and post-patch versions of each function side-by-side,
  and runs them against identical inputs (41 assertions across
  12 cases, including: exhaustive 4096-combination nibble
  matrix, 1000-frame leak counter, red-zone canaries to detect
  oversized-frame overruns, and direct reproduction of the
  original alpha-bleed and stack-garbage bugs against the
  unfixed code). All assertions pass on the post-patch code;
  the original-code reproductions all fail in the expected
  ways.
- Not testable in this sandbox: actual on-device rendering on
  Raspberry Pi (drm) or Allwinner SoC (sunxi). Both drivers
  are conditionally compiled and have no automated tests.

Author: LibretroAdmin

gfx/drivers/drm_gfx.c

@@ -816,8 +816,11 @@ static void drm_set_texture_enable(void *data, bool state, bool full_screen)
 static void drm_set_texture_frame(void *data, const void *frame, bool rgb32,
       unsigned width, unsigned height, float alpha)
 {
-   unsigned int i, j;
-   struct drm_video *_drmvars = data;
+   unsigned int i;
+   struct drm_video    *_drmvars = data;
+   struct drm_surface  *surface  = NULL;
+   uint8_t             *dst_base = NULL;
+   unsigned int         dst_pitch;
 
    if (!_drmvars->menu_active)
       return;
@@ -843,36 +846,61 @@ static void drm_set_texture_frame(void *data, const void *frame, bool rgb32,
       drm_plane_setup(_drmvars->menu_surface);
    }
 
-   /* We have to go on a pixel format conversion adventure
-    * for now, until we can convince RGUI to output
-    * in an 8888 format. */
-   unsigned int src_pitch        = width * 2;
-   unsigned int dst_pitch        = width * 4;
-   unsigned int dst_width        = width;
-   uint32_t line[dst_width];
-
-   /* The output pixel array with the converted pixels. */
-   char *frame_output = (char *) malloc (dst_pitch * height);
+   surface   = _drmvars->menu_surface;
+   dst_base  = (uint8_t*)surface->pages[surface->flip_page].buf.map;
+   dst_pitch = surface->pitch;
 
-   /* Remember, memcpy() works with 8bits pointers for increments. */
-   char *dst_base_addr           = frame_output;
+   /* Defensive clamps: the dumb buffer was sized at the first
+    * menu frame's dimensions and is never resized within an
+    * active menu session. If the caller hands us something
+    * bigger anyway, write only what fits rather than running
+    * off the end of the mapped region. */
+   {
+      unsigned int max_w = (unsigned int)surface->src_width;
+      unsigned int max_h = (unsigned int)surface->src_height;
+      if (width  > max_w) width  = max_w;
+      if (height > max_h) height = max_h;
+   }
 
-   for (i = 0; i < height; i++)
+   if (rgb32)
    {
-      for (j = 0; j < src_pitch / 2; j++)
+      /* Source is already XRGB8888 -- just copy row by row to handle
+       * any difference between source stride and dst stride. */
+      const uint8_t *src      = (const uint8_t*)frame;
+      unsigned int   src_pitch = width * 4;
+      unsigned int   row_bytes = (src_pitch < dst_pitch) ? src_pitch : dst_pitch;
+
+      for (i = 0; i < height; i++)
+         memcpy(dst_base + (dst_pitch * i), src + (src_pitch * i), row_bytes);
+   }
+   else
+   {
+      /* RGUI default output is RGBA4444 with channel layout
+       *   R = bits 15..12, G = 11..8, B = 7..4, A = 3..0
+       * Expand each 4-bit channel to 8 bits via nibble replication
+       * (x | (x << 4)) and pack into XRGB8888 for the dumb buffer. */
+      for (i = 0; i < height; i++)
       {
-         uint16_t src_pix = *((uint16_t*)frame + (src_pitch / 2 * i) + j);
-         /* The hex AND is for keeping only the part we need for each component. */
-         uint32_t R = (src_pix << 8) & 0x00FF0000;
-         uint32_t G = (src_pix << 4) & 0x0000FF00;
-         uint32_t B = (src_pix << 0) & 0x000000FF;
-         line[j] = (0 | R | G | B);
+         const uint16_t *src_row = (const uint16_t*)frame + (width * i);
+         uint32_t       *dst_row = (uint32_t*)(dst_base + (dst_pitch * i));
+         unsigned int    j;
+
+         for (j = 0; j < width; j++)
+         {
+            uint16_t src_pix = src_row[j];
+            uint32_t r4      = (src_pix >> 12) & 0xF;
+            uint32_t g4      = (src_pix >>  8) & 0xF;
+            uint32_t b4      = (src_pix >>  4) & 0xF;
+            uint32_t r8      = (r4 << 4) | r4;
+            uint32_t g8      = (g4 << 4) | g4;
+            uint32_t b8      = (b4 << 4) | b4;
+            dst_row[j]       = (r8 << 16) | (g8 << 8) | b8;
+         }
       }
-      memcpy(dst_base_addr + (dst_pitch * i), (char*)line, dst_pitch);
    }
 
-   /* We update the menu surface if menu is active. */
-   drm_surface_update(_drmvars, frame_output, _drmvars->menu_surface);
+   /* The bytes are in place in the dumb buffer; commit the page flip. */
+   drm_page_flip(surface);
 }
 
 static void drm_set_nonblock_state(void *a, bool b, bool c, unsigned d) { }

gfx/drivers/sunxi_gfx.c

@@ -846,40 +846,70 @@ static void sunxi_set_texture_frame(void *data, const void *frame, bool rgb32,
       unsigned width, unsigned height, float alpha)
 {
    struct sunxi_video *_dispvars = (struct sunxi_video*)data;
+   uint8_t            *dst_base;
+   unsigned int        dst_pitch;
+   unsigned int        i;
 
-   if (_dispvars->menu_active)
-   {
-      unsigned int i, j;
+   if (!_dispvars->menu_active)
+      return;
 
-      /* We have to go on a pixel format conversion adventure for now, until we can
-       * convince RGUI to output in an 8888 format. */
-      unsigned int src_pitch        = width * 2;
-      unsigned int dst_pitch        = _dispvars->sunxi_disp->xres * 4;
-      unsigned int dst_width        = _dispvars->sunxi_disp->xres;
-      uint32_t line[dst_width];
+   /* The display layer is xres pixels wide; we write `width` pixels
+    * per row into the leading bytes of each destination row, leaving
+    * any pixels beyond `width` untouched. RGUI typically renders at
+    * the full display width, in which case the whole row is written. */
+   dst_base  = (uint8_t*)_dispvars->pages[0].address;
+   dst_pitch = _dispvars->sunxi_disp->xres * 4;
 
-      /* Remember, memcpy() works with 8bits pointers for increments. */
-      char *dst_base_addr           = (char*)(_dispvars->pages[0].address);
+   /* Defensive clamps: the page is xres wide and src_height tall.
+    * Don't run off the end if the caller's frame is bigger. */
+   {
+      unsigned int max_w = _dispvars->sunxi_disp->xres;
+      unsigned int max_h = (unsigned int)_dispvars->src_height;
+      if (width  > max_w) width  = max_w;
+      if (height > max_h) height = max_h;
+   }
+
+   if (rgb32)
+   {
+      /* Source is already XRGB8888 -- per-row memcpy handles the
+       * difference between source stride (width*4) and dst stride. */
+      const uint8_t *src       = (const uint8_t*)frame;
+      unsigned int   src_pitch = width * 4;
+      unsigned int   row_bytes = (src_pitch < dst_pitch) ? src_pitch : dst_pitch;
 
+      for (i = 0; i < height; i++)
+         memcpy(dst_base + (dst_pitch * i), src + (src_pitch * i), row_bytes);
+   }
+   else
+   {
+      /* RGUI default output is RGBA4444 with channel layout
+       *   R = bits 15..12, G = 11..8, B = 7..4, A = 3..0
+       * Expand each 4-bit channel to 8 bits via nibble replication
+       * (x | (x << 4)) and pack into XRGB8888 for the display layer. */
       for (i = 0; i < height; i++)
       {
-         for (j = 0; j < src_pitch / 2; j++)
+         const uint16_t *src_row = (const uint16_t*)frame + (width * i);
+         uint32_t       *dst_row = (uint32_t*)(dst_base + (dst_pitch * i));
+         unsigned int    j;
+
+         for (j = 0; j < width; j++)
          {
-            uint16_t src_pix = *((uint16_t*)frame + (src_pitch / 2 * i) + j);
-            /* The hex AND is for keeping only the part we need for each component. */
-            uint32_t R = (src_pix << 8) & 0x00FF0000;
-            uint32_t G = (src_pix << 4) & 0x0000FF00;
-            uint32_t B = (src_pix << 0) & 0x000000FF;
-            line[j] = (0 | R | G | B);
+            uint16_t src_pix = src_row[j];
+            uint32_t r4      = (src_pix >> 12) & 0xF;
+            uint32_t g4      = (src_pix >>  8) & 0xF;
+            uint32_t b4      = (src_pix >>  4) & 0xF;
+            uint32_t r8      = (r4 << 4) | r4;
+            uint32_t g8      = (g4 << 4) | g4;
+            uint32_t b8      = (b4 << 4) | b4;
+            dst_row[j]       = (r8 << 16) | (g8 << 8) | b8;
          }
-         memcpy(dst_base_addr + (dst_pitch * i), (char*)line, dst_pitch);
       }
-
-      /* Issue pageflip. Will flip on next vsync. */
-      sunxi_layer_set_rgb_input_buffer(_dispvars->sunxi_disp,
-            _dispvars->sunxi_disp->bits_per_pixel,
-            _dispvars->pages[0].offset, width, height, _dispvars->sunxi_disp->xres);
    }
+
+   /* Issue pageflip. Will flip on next vsync. */
+   sunxi_layer_set_rgb_input_buffer(_dispvars->sunxi_disp,
+         _dispvars->sunxi_disp->bits_per_pixel,
+         _dispvars->pages[0].offset, width, height, _dispvars->sunxi_disp->xres);
 }
 
 static void sunxi_set_aspect_ratio(void *data, unsigned aspect_ratio_idx)