From 1ffc9299be1b7908f653387b2b3c96adab233eae Mon Sep 17 00:00:00 2001 From: vasilito Date: Tue, 28 Jul 2026 07:19:01 +0900 Subject: [PATCH] mesa: fix batch pool follow-on defects (zero-byte BO, mutex init, oversized) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-on defects from the 5-lane review of commit 16f74ab87c: 1. Zero-byte BO allocation (redox_drm_bo.c) batch_bo_acquire() in redox_drm_cs.c passes a pipe_resource template with format = 0 (PIPE_FORMAT_NONE). redox_resource_byte_count() returned 0 for format = 0, producing a zero-byte GEM object. The original commit message acknowledged this but didn't fix it. Fix: in redox_resource_byte_count(), handle PIPE_BUFFER and format-NONE specially. PIPE_BUFFER uses width0 as the raw byte count; size by width0 * height0 * depth0 (which for buffers is just width0). This is the root-cause fix — it also benefits every other PIPE_BUFFER allocation, not just batch pools. 2. Unchecked mtx_init (redox_drm_winsys.c, .h) (void) mtx_init(...) silently swallowed initialization errors, leaving subsequent lock/unlock operations on an invalid mutex (undefined behavior). Fix: require thrd_success; on failure, return false from redox_drm_initialize(). Add a batch_pool_mutex_initialized flag to the winsys struct so redox_drm_destroy() only destroys an initialized mutex. Also clean up the mutex on the cs_create failure path. Initialize the flag and pool slots to safe defaults at the top of redox_drm_initialize(). 3. Oversized batch size truncation (redox_drm_cs.c) pipe_resource::width0 is uint32_t. byte_count was stored as width0 without overflow checks; requests > UINT32_MAX would allocate a truncated BO and then overflow on the memcpy. Fix: in batch_bo_acquire(), return NULL when byte_count > UINT32_MAX with a comment explaining the limit. No real batch exceeds 4 GiB. Verified by static review; the C code cross-compiles only for the Redox target (no host build). --- .../gallium/winsys/redox/drm/redox_drm_bo.c | 19 ++++++++++++++++++- .../gallium/winsys/redox/drm/redox_drm_cs.c | 7 +++++++ .../winsys/redox/drm/redox_drm_winsys.c | 16 ++++++++++++++-- .../winsys/redox/drm/redox_drm_winsys.h | 1 + 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_bo.c b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_bo.c index b7027f5cc4..d27873dde5 100644 --- a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_bo.c +++ b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_bo.c @@ -60,9 +60,26 @@ redox_format_is_compressed(enum pipe_format format) static uint64_t redox_resource_byte_count(const struct pipe_resource *templat) { - if (!templat || !templat->format) + if (!templat) return 0; + /* PIPE_BUFFER uses width0 as the size in bytes and typically leaves + * format as PIPE_FORMAT_NONE (0). The generic format-based path + * below would compute 0 bytes (util_format_get_blocksizebits(0) + * returns 0), producing a zero-byte GEM object — the root cause of + * the batch-pool zero-byte allocation defect. + * + * Size buffers by their raw geometry (width0 * height0 * depth0) + * instead. For PIPE_BUFFER height0 and depth0 are 1, so this + * reduces to width0 (= the byte count the caller requested). This + * also covers any other resource whose format is NONE. */ + if (templat->target == PIPE_BUFFER || !templat->format) { + uint32_t width = templat->width0 ? templat->width0 : 1; + uint32_t height = templat->height0 ? templat->height0 : 1; + uint32_t depth = templat->depth0 ? templat->depth0 : 1; + return (uint64_t)width * (uint64_t)height * (uint64_t)depth; + } + uint32_t width = templat->width0 ? templat->width0 : 1; uint32_t height = templat->height0 ? templat->height0 : 1; uint32_t depth = templat->depth0 ? templat->depth0 : 1; diff --git a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_cs.c b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_cs.c index 7679ccf13a..bf5e4b26ac 100644 --- a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_cs.c +++ b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_cs.c @@ -161,6 +161,13 @@ batch_bo_acquire(struct redox_drm_winsys *rws, size_t byte_count) if (byte_count == 0 || !rws) return NULL; + /* pipe_resource::width0 is uint32_t. A byte_count exceeding UINT32_MAX + * would be silently truncated when stored as width0, producing an + * undersized GEM object and a subsequent heap overflow on write. + * Reject such requests explicitly — no real batch can exceed 4 GiB. */ + if (byte_count > UINT32_MAX) + return NULL; + unsigned cls = batch_bo_size_class(byte_count); /* Requests exceeding the largest pool class (512 KiB) always get diff --git a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.c b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.c index 492a1613a1..e1f53e9ee1 100644 --- a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.c +++ b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.c @@ -398,14 +398,23 @@ redox_drm_initialize(struct redox_drm_winsys *rws) if (!rws) return false; + rws->cs = NULL; + rws->batch_pool_mutex_initialized = false; + for (unsigned i = 0; i < REDOX_DRM_BATCH_POOL_SIZE_CLASSES; i++) rws->batch_pool[i] = NULL; - (void) mtx_init(&rws->batch_pool_mutex, mtx_plain); + if (mtx_init(&rws->batch_pool_mutex, mtx_plain) != thrd_success) { + debug_printf("redox: mtx_init failed for batch_pool_mutex\n"); + return false; + } + rws->batch_pool_mutex_initialized = true; rws->cs = redox_drm_cs_create(rws); if (!rws->cs) { debug_printf("redox: failed to create cs state\n"); + mtx_destroy(&rws->batch_pool_mutex); + rws->batch_pool_mutex_initialized = false; return false; } return true; @@ -424,7 +433,10 @@ redox_drm_destroy(struct redox_drm_winsys *rws) } } - mtx_destroy(&rws->batch_pool_mutex); + if (rws->batch_pool_mutex_initialized) { + mtx_destroy(&rws->batch_pool_mutex); + rws->batch_pool_mutex_initialized = false; + } if (rws->cs) { redox_drm_cs_destroy(rws->cs); diff --git a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.h b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.h index 64ff75242f..027c8c772b 100644 --- a/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.h +++ b/local/recipes/libs/mesa/source/src/gallium/winsys/redox/drm/redox_drm_winsys.h @@ -39,6 +39,7 @@ struct redox_drm_winsys { * create several contexts per screen, and each can flush (submit) * independently from different threads. */ mtx_t batch_pool_mutex; + bool batch_pool_mutex_initialized; /* Batch BO ring-buffer pool. batch_pool[i] caches the largest * recently-recycled BO whose size class == i. Acquire pulls from