From bc8c3ae8bbf2b38da79b587602184a1663f7f8c7 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 15 Aug 2026 02:22:36 -0700 Subject: [PATCH] fix(simd): NEON vfloat4::load(values,n) read past the end of the buffer The NEON branch loaded all 4 lanes with vld1q_f32 and then zeroed the ones it didn't want, so a partial load of n<4 elements read up to 12 bytes past the caller's buffer. ASan flags it as a heap-buffer-overflow of size 16. The SSE branch gets this right by switching on n before loading. Within OIIO the over-read has been harmless, because imagebuf.cpp and imagecache.cpp deliberately pad their allocations by OIIO_SIMD_MAX_SIZE_BYTES to permit exactly this. But simd.h is a public header, and a downstream caller has no reason to expect a 4-element load to touch 16 bytes. Also, n==0 fell through to `default:` after the load without zeroing anything, returning the over-read garbage instead of zeros. Assisted-by: Claude Code / claude-opus-5 Signed-off-by: Larry Gritz --- src/include/OpenImageIO/simd.h | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/include/OpenImageIO/simd.h b/src/include/OpenImageIO/simd.h index 24e821402c..5c6f4fe986 100644 --- a/src/include/OpenImageIO/simd.h +++ b/src/include/OpenImageIO/simd.h @@ -6701,18 +6701,25 @@ OIIO_FORCEINLINE void vfloat4::load (const float *values, int n) { break; } #elif OIIO_SIMD_NEON - //switch (n) { - //case 1: m_simd = vdupq_n_f32(0); m_simd[0] = values[0]; break; - //case 2: load (values[0], values[1], 0.0f, 0.0f); break; - //case 3: load (values[0], values[1], values[2], 0.0f); break; - //case 4: m_simd = vld1q_f32 (values); break; - //default: break; - m_simd = vld1q_f32(values); + // Note: must not read past values[n-1], so switch before loading rather + // than loading all 4 lanes and masking afterwards. switch (n) { - case 1: m_simd = vsetq_lane_f32(0.0f, m_simd, 1); - case 2: m_simd = vsetq_lane_f32(0.0f, m_simd, 2); - case 3: m_simd = vsetq_lane_f32(0.0f, m_simd, 3); - default: break; + case 1: + m_simd = vld1q_lane_f32 (values, vdupq_n_f32(0.0f), 0); + break; + case 2: + m_simd = vcombine_f32 (vld1_f32(values), vdup_n_f32(0.0f)); + break; + case 3: + m_simd = vld1q_lane_f32 (values+2, + vcombine_f32 (vld1_f32(values), vdup_n_f32(0.0f)), 2); + break; + case 4: + m_simd = vld1q_f32 (values); + break; + default: + clear(); + break; } #else for (int i = 0; i < n; ++i)