From 4f03fb0f488d8f528bdc0f9ecac07eea8f7b63ac Mon Sep 17 00:00:00 2001 From: Christoph Gohlke Date: Tue, 17 Mar 2026 20:56:39 -0700 Subject: [PATCH 1/6] Fix comparison operator for buffer size check (#2307) Signed-off-by: Christoph Gohlke --- .../plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_zip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_zip.c b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_zip.c index 628438c5c01..56df6a49c2f 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_zip.c +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_zip.c @@ -367,7 +367,7 @@ apply_zip_impl (exr_encode_pipeline_t* encode) if (rv == EXR_ERR_SUCCESS) { - if (compbufsz > encode->packed_bytes) + if (compbufsz >= encode->packed_bytes) { memcpy ( encode->compressed_buffer, From 0512b6c41b95e5c0ac9e01b2f87b56acd437d8e9 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 26 Mar 2026 16:35:49 -0700 Subject: [PATCH 2/6] Fix integer overflow in `srcbuffer` pointer arithmetic in `unpack_*` (#2321) Pointer arithmetic involving `srcbuffer` in the `unpack_*` functions can overflow for very wide images. Multiplying `int w` by a bytes-per-element constant (2, 4, 6, or 8) in `srcbuffer +=` expressions causes signed integer overflow when the image width is large. Promote the leading operand to `int64_t` at every affected multiplication so pointer arithmetic is performed in 64-bit signed arithmetic throughout, eliminating the undefined behavior without imposing any image size limit. Affected sites: `generic_unpack`, `unpack_16bit`, `unpack_32bit`, and all 3- and 4-channel interleave/planar specializations. Made-with: Cursor Signed-off-by: Cary Phillips --- .../hioOpenEXR/OpenEXR/OpenEXRCore/unpack.c | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/unpack.c b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/unpack.c index 1324508c5b4..8765bfa50d6 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/unpack.c +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/unpack.c @@ -229,7 +229,7 @@ unpack_16bit_3chan_interleave (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = one_to_native16 (in0[x]); @@ -269,7 +269,7 @@ unpack_16bit_3chan_interleave_rev (exr_decode_pipeline_t* decode) in1 = in0 + w; // G in2 = in1 + w; // R - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = one_to_native16 (in2[x]); @@ -309,7 +309,7 @@ unpack_half_to_float_3chan_interleave (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = half_to_float (one_to_native16 (in0[x])); @@ -349,7 +349,7 @@ unpack_half_to_float_3chan_interleave_rev (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = half_to_float (one_to_native16 (in2[x])); @@ -390,7 +390,7 @@ unpack_16bit_3chan_planar (exr_decode_pipeline_t* decode) in0 = (const uint16_t*) srcbuffer; in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion /* specialise to memcpy if we can */ #if EXR_HOST_IS_NOT_LITTLE_ENDIAN for (int x = 0; x < w; ++x) @@ -440,7 +440,7 @@ unpack_half_to_float_3chan_planar (exr_decode_pipeline_t* decode) in0 = (const uint16_t*) srcbuffer; in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion /* specialise to memcpy if we can */ half_to_float_buffer ((float*) out0, in0, w); half_to_float_buffer ((float*) out1, in1, w); @@ -485,7 +485,7 @@ unpack_16bit_3chan (exr_decode_pipeline_t* decode) in0 = (const uint16_t*) srcbuffer; in1 = in0 + w; in2 = in1 + w; - srcbuffer += w * 6; // 3 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 6; // 3 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) *((uint16_t*) (out0 + x * inc0)) = one_to_native16 (in0[x]); for (int x = 0; x < w; ++x) @@ -539,7 +539,7 @@ unpack_16bit_4chan_interleave (exr_decode_pipeline_t* decode) in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { combined.a = one_to_native16 (in0[x]); @@ -592,7 +592,7 @@ unpack_16bit_4chan_interleave_rev (exr_decode_pipeline_t* decode) in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { combined.a = one_to_native16 (in0[x]); @@ -633,7 +633,7 @@ unpack_half_to_float_4chan_interleave (exr_decode_pipeline_t* decode) in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = half_to_float (one_to_native16 (in3[x])); @@ -674,7 +674,7 @@ unpack_half_to_float_4chan_interleave_rev (exr_decode_pipeline_t* decode) in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) { out[0] = half_to_float (one_to_native16 (in0[x])); @@ -719,7 +719,7 @@ unpack_16bit_4chan_planar (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion /* specialize to memcpy if we can */ #if EXR_HOST_IS_NOT_LITTLE_ENDIAN for (int x = 0; x < w; ++x) @@ -775,7 +775,7 @@ unpack_half_to_float_4chan_planar (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion half_to_float_buffer ((float*) out0, in0, w); half_to_float_buffer ((float*) out1, in1, w); @@ -825,7 +825,7 @@ unpack_16bit_4chan (exr_decode_pipeline_t* decode) in1 = in0 + w; in2 = in1 + w; in3 = in2 + w; - srcbuffer += w * 8; // 4 * sizeof(uint16_t), avoid type conversion + srcbuffer += (int64_t) w * 8; // 4 * sizeof(uint16_t), avoid type conversion for (int x = 0; x < w; ++x) *((uint16_t*) (out0 + x * inc0)) = one_to_native16 (in0[x]); for (int x = 0; x < w; ++x) @@ -898,7 +898,7 @@ unpack_16bit (exr_decode_pipeline_t* decode) } } #endif - srcbuffer += w * 2; + srcbuffer += (int64_t) w * 2; } } return EXR_ERR_SUCCESS; @@ -963,7 +963,7 @@ unpack_32bit (exr_decode_pipeline_t* decode) } } #endif - srcbuffer += w * 4; + srcbuffer += (int64_t) w * 4; } } return EXR_ERR_SUCCESS; @@ -1118,7 +1118,7 @@ generic_unpack (exr_decode_pipeline_t* decode) (uint64_t) decc->user_line_stride); else { - srcbuffer += w * bpc; + srcbuffer += (int64_t) w * bpc; continue; } } @@ -1128,12 +1128,12 @@ generic_unpack (exr_decode_pipeline_t* decode) } else { - srcbuffer += w * bpc; + srcbuffer += (int64_t) w * bpc; continue; } UNPACK_SAMPLES (w) - srcbuffer += w * bpc; + srcbuffer += (int64_t) w * bpc; } } return EXR_ERR_SUCCESS; From f5f7f32d5761b8d28d61e391e9daf68bd62bc9e5 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Sun, 29 Mar 2026 19:19:12 -0700 Subject: [PATCH 3/6] Security: fix signed integer overflow in `undo_pxr24_impl()` (PXR24 decoder) (#2323) In the `EXR_PIXEL_FLOAT` branch of `undo_pxr24_impl()`, the expressions (uint64_t)(w * 3) compute the signed 32-bit product `w * 3` before the cast to `uint64_t`. When `w` is large this is undefined behavior under the C standard; on two's-complement builds without sanitizers the result wraps to a small positive value, which can cause the bounds check if (nDec + (uint64_t)(w * 3) > outSize) to pass incorrectly. If the check is bypassed the decode loop proceeds to write `4*w` bytes through `dout`, potentially far beyond the allocated output buffer. Fix: cast `w` to `uint64_t` before multiplying so that both the bounds check and the counter update are performed entirely in 64-bit unsigned arithmetic: (uint64_t)w * 3 (cast before multiply, not after) The `EXR_PIXEL_UINT` and `EXR_PIXEL_HALF` decode branches are unaffected: they reuse the pre-computed `nBytes` variable, which is already formed as `(uint64_t)(w) * (uint64_t)(bytes_per_element)`. Also fix the symmetric issue in `apply_pxr24_impl()` (the encoder): lastIn += w * 4 advances a pointer by a signed 32-bit product; corrected to lastIn += (uint64_t)w * 4 Made-with: Cursor Signed-off-by: Cary Phillips --- .../plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_pxr24.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_pxr24.c b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_pxr24.c index 43e52c982cf..30f71f4f846 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_pxr24.c +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_pxr24.c @@ -182,7 +182,7 @@ apply_pxr24_impl (exr_encode_pipeline_t* encode) if (nOut + nBytes > encode->scratch_alloc_size_1) return EXR_ERR_OUT_OF_MEMORY; nOut += nBytes; - lastIn += w * 4; + lastIn += (uint64_t) w * 4; ptr[0] = out; out += w; @@ -371,7 +371,7 @@ undo_pxr24_impl ( ptr[2] = lastIn; lastIn += w; - if (nDec + (uint64_t) (w * 3) > outSize) + if (nDec + (uint64_t) w * 3 > outSize) return EXR_ERR_CORRUPT_CHUNK; for (int x = 0; x < w; ++x) @@ -384,7 +384,7 @@ undo_pxr24_impl ( unaligned_store32 (dout, pixel); ++dout; } - nDec += (uint64_t) (w * 3); + nDec += (uint64_t) w * 3; break; } default: return EXR_ERR_INVALID_ARGUMENT; From d578381a503add330b3b96b30cf6a49461989f08 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Sun, 29 Mar 2026 19:19:49 -0700 Subject: [PATCH 4/6] fix integer overflow in PIZ wavelet buffer arithmetic (#2328) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three classes of signed integer overflow in the PIZ codec path, all reachable from corrupt `dataWindow` dimensions in the EXR file header. **`wav_2D_encode` / `wav_2D_decode` — wavelet loop pointer arithmetic** `oy` is passed as `int` (value `wcount * nx`, at most ~INT32_MAX after the guard below). Inside the hierarchical wavelet loop the expressions ey = in + oy * (ny - p2) // pointer end-of-row sentinel oy1 = oy * p // row stride at level p oy2 = oy * p2 // row stride at level p2 multiply two values that can each approach INT32_MAX, producing a signed 32-bit product that wraps to a small or negative value. The wrapped value is used as a pointer offset, causing reads and writes through `px` / `py` to land outside the allocated wavelet buffer. Fix: widen by introducing `int64_t oy64 = oy` and using it for all three expressions; `oy1` and `oy2` are also declared `int64_t`. **`wavbuf += nx * ny * wcount` — per-channel buffer advance** `nx`, `ny`, and `wcount` are all `int`. Their triple product overflows int32 for moderately large images, causing subsequent channels to be processed at an incorrect (too-small) offset into the wavelet buffer, corrupting both encode and decode output. Fix: cast to `(uint64_t)` before multiplying. **`wcount * nx` — call-site argument overflow** The fifth argument to `wav_2D_encode` / `wav_2D_decode` is `wcount * nx` (`oy` = y-stride = elements per row). `wcount` is 1 or 2 (`bytes_per_element / 2`); for `wcount = 2` the product overflows int32 when `nx > INT32_MAX / 2`. Fix: add an early bounds check `if (wcount > 0 && nx > INT_MAX / wcount)` that rejects such input as `EXR_ERR_CORRUPT_CHUNK` before any arithmetic is performed. This also keeps `wcount * nx` within int32 range at the call site, ensuring `oy` arrives in the wavelet functions with a valid non-overflowed value. Made-with: Cursor Signed-off-by: Cary Phillips --- .../OpenEXR/OpenEXRCore/internal_piz.c | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_piz.c b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_piz.c index 6c2750b166f..787d2752c61 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_piz.c +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_piz.c @@ -10,6 +10,7 @@ #include "internal_huf.h" #include "internal_xdr.h" +#include #include /**************************************/ @@ -171,10 +172,11 @@ wdec16 (uint16_t l, uint16_t h, uint16_t* a, uint16_t* b) static void wav_2D_encode (uint16_t* in, int nx, int ox, int ny, int oy, uint16_t mx) { - int w14 = (mx < (1 << 14)) ? 1 : 0; - int n = (nx > ny) ? ny : nx; - int p = 1; // == 1 << level - int p2 = 2; // == 1 << (level+1) + int w14 = (mx < (1 << 14)) ? 1 : 0; + int n = (nx > ny) ? ny : nx; + int p = 1; // == 1 << level + int p2 = 2; // == 1 << (level+1) + int64_t oy64 = oy; // // Hierarchical loop on smaller dimension n @@ -183,9 +185,9 @@ wav_2D_encode (uint16_t* in, int nx, int ox, int ny, int oy, uint16_t mx) while (p2 <= n) { uint16_t* py = in; - uint16_t* ey = in + oy * (ny - p2); - int oy1 = oy * p; - int oy2 = oy * p2; + uint16_t* ey = in + oy64 * (ny - p2); + int64_t oy1 = oy64 * p; + int64_t oy2 = oy64 * p2; int ox1 = ox * p; int ox2 = ox * p2; uint16_t i00, i01, i10, i11; @@ -284,10 +286,11 @@ wav_2D_decode ( int oy, // i : y offset uint16_t mx) // i : maximum in[x][y] value { - int w14 = (mx < (1 << 14)) ? 1 : 0; - int n = (nx > ny) ? ny : nx; - int p = 1; - int p2; + int w14 = (mx < (1 << 14)) ? 1 : 0; + int n = (nx > ny) ? ny : nx; + int p = 1; + int p2; + int64_t oy64 = oy; // // Search max level @@ -307,9 +310,9 @@ wav_2D_decode ( while (p >= 1) { uint16_t* py = in; - uint16_t* ey = in + oy * (ny - p2); - int oy1 = oy * p; - int oy2 = oy * p2; + uint16_t* ey = in + oy64 * (ny - p2); + int64_t oy1 = oy64 * p; + int64_t oy2 = oy64 * p2; int ox1 = ox * p; int ox2 = ox * p2; uint16_t i00, i01, i10, i11; @@ -502,11 +505,13 @@ internal_exr_apply_piz (exr_encode_pipeline_t* encode) nx = curc->width; ny = curc->height; wcount = (int) (curc->bytes_per_element / 2); + if (wcount > 0 && nx > INT_MAX / wcount) + return EXR_ERR_CORRUPT_CHUNK; for (int j = 0; j < wcount; ++j) { wav_2D_encode (wavbuf + j, nx, wcount, ny, wcount * nx, maxValue); } - wavbuf += nx * ny * wcount; + wavbuf += (uint64_t) nx * ny * wcount; } nBytes = 0; @@ -655,11 +660,13 @@ internal_exr_undo_piz ( nx = curc->width; ny = curc->height; wcount = (int) (curc->bytes_per_element / 2); + if (wcount > 0 && nx > INT_MAX / wcount) + return EXR_ERR_CORRUPT_CHUNK; for (int j = 0; j < wcount; ++j) { wav_2D_decode (wavbuf + j, nx, wcount, ny, wcount * nx, maxValue); } - wavbuf += nx * ny * wcount; + wavbuf += (uint64_t) nx * ny * wcount; } // From a76474285ddc24e9956b5e12fab658b908912ed4 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Sun, 29 Mar 2026 19:20:52 -0700 Subject: [PATCH 5/6] Fix signed integer overflow in `LossyDctDecoder_execute()` pointer arithmetic (#2329) `numBlocksX` and `numBlocksY` are declared as `int`. Two pointer-offset expressions in `LossyDctDecoder_execute()` multiplied them as signed 32-bit integers before using the result as a pointer offset: rowBlock[comp] = rowBlock[comp - 1] + numBlocksX * 64; currDcComp[comp] = currDcComp[comp - 1] + numBlocksX * numBlocksY; `dataWindow.max.x` is a signed 32-bit value in the EXR file format, so `numBlocksX` can reach `(INT32_MAX + 7) / 8 = 268,435,456`. At that point `numBlocksX * 64 = 17,179,869,184` overflows `int32`, and `numBlocksX * numBlocksY` overflows even sooner. The wraparound produces a small or negative pointer offset, causing `rowBlock[comp]` and `currDcComp[comp]` to point into already-used or pre-buffer memory rather than the intended component stride. Fix: cast `numBlocksX` to `size_t` before multiplying so the arithmetic is performed in pointer-sized unsigned arithmetic: rowBlock[comp] = rowBlock[comp - 1] + (size_t) numBlocksX * 64; currDcComp[comp] = currDcComp[comp - 1] + (size_t) numBlocksX * numBlocksY; This is consistent with the allocation on the line above, which already uses `(size_t) numComp * (size_t) numBlocksX * 64 * sizeof(uint16_t)`, and with the packed-DC count check, which uses explicit `uint64_t` casts. Made-with: Cursor Signed-off-by: Cary Phillips --- .../hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h index bcdebd985f9..e289b064dac 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h @@ -265,7 +265,7 @@ LossyDctDecoder_execute ( } for (int comp = 1; comp < numComp; ++comp) - rowBlock[comp] = rowBlock[comp - 1] + numBlocksX * 64; + rowBlock[comp] = rowBlock[comp - 1] + (size_t) numBlocksX * 64; // // Pack DC components together by common plane, so we can get @@ -275,7 +275,7 @@ LossyDctDecoder_execute ( currDcComp[0] = (uint16_t*) d->_packedDc; for (int comp = 1; comp < numComp; ++comp) - currDcComp[comp] = currDcComp[comp - 1] + numBlocksX * numBlocksY; + currDcComp[comp] = currDcComp[comp - 1] + (size_t) numBlocksX * numBlocksY; for (int blocky = 0; blocky < numBlocksY; ++blocky) { From 219a0870506f80c7353016800e467ad0de923424 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Tue, 31 Mar 2026 07:49:09 -0700 Subject: [PATCH 6/6] =?UTF-8?q?Fix=20misaligned=20memory=20access=20in=20`?= =?UTF-8?q?LossyDctDecoder=5Fexecute`=20HALF=E2=86=92FLOAT=20expansion=20(?= =?UTF-8?q?#2324)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix misaligned memory access in `LossyDctDecoder_execute` HALF→FLOAT expansion After DCT decoding, `LossyDctDecoder_execute()` expands FLOAT-type channels from their intermediate HALF (16-bit) XDR representation back to FLOAT (32-bit) XDR in place. The expansion was done by casting `_rows[y]` (a `uint8_t *`) directly to `float *` and `uint16_t *`, then reading and writing through those typed pointers. Because row buffers are assigned by advancing a byte pointer with no alignment padding (`outBufferEnd += chan->width * chan->bytes_per_element` in `internal_dwa_compressor.h`), a FLOAT channel that follows a HALF channel of odd width receives a `_rows[y]` pointer that is 2-byte aligned but not 4-byte aligned. Dereferencing a `float *` cast from such a pointer is undefined behavior under the C standard: - On ARM, RISC-V, and MIPS (strict alignment) this crashes immediately. - On x86 it is silently tolerated at the hardware level but remains UB: auto-vectorizing compilers (SSE/AVX) may assume aligned access and generate incorrect code. - UBSan reports: `store to misaligned address ... for type 'float', which requires 4 byte alignment` at `internal_dwa_decoder.h:749`. Fix: replace the cast-and-dereference pattern with the `unaligned_load16` / `memcpy` / `unaligned_store32` helpers already used throughout the rest of OpenEXRCore (`internal_xdr.h`, `unpack.c`, `pack.c`, `internal_pxr24.c`). These helpers use `memcpy` internally, which the C standard guarantees is safe for unaligned addresses and which compilers compile to a single load/store instruction on architectures that support it. The byte-order handling is preserved correctly: - `unaligned_load16` reads 2 bytes via `memcpy` and applies `one_to_native16` (XDR → native), returning a native-endian HALF value. - `half_to_float` converts native HALF → native float. - `memcpy(&bits, &f, 4)` reinterprets the float's bit pattern as `uint32_t` without numeric conversion (the correct type-pun idiom in C). - `unaligned_store32` applies `one_from_native32` (native → XDR) and writes 4 bytes via `memcpy`, storing the result in XDR float format. Made-with: Cursor Signed-off-by: Cary Phillips * add TODO comment Signed-off-by: Cary Phillips --------- Signed-off-by: Cary Phillips --- .../OpenEXR/OpenEXRCore/internal_dwa_decoder.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h index e289b064dac..046615ec5b8 100644 --- a/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h +++ b/pxr/imaging/plugin/hioOpenEXR/OpenEXR/OpenEXRCore/internal_dwa_decoder.h @@ -651,13 +651,22 @@ LossyDctDecoder_execute ( /* process in place in reverse to avoid temporary buffer */ for (int y = 0; y < d->_height; ++y) { - float* floatXdrPtr = (float*) chanData[chan]->_rows[y]; - uint16_t* halfXdr = (uint16_t*) floatXdrPtr; + uint8_t* rowBytes = chanData[chan]->_rows[y]; for (int x = d->_width - 1; x >= 0; --x) { - floatXdrPtr[x] = one_from_native_float ( - half_to_float (one_to_native16 (halfXdr[x]))); + // TODO: make an unaligned_store32f that takes the float and + // packages up a one_from_native_float and calls memcpy + // instead of the two memcpy. We should look at the metrics + // for dwa and see if there's a performance difference to do + // so at some point. See: + // https://github.com/AcademySoftwareFoundation/openexr/pull/2324 + + uint16_t h = unaligned_load16 (rowBytes + x * sizeof (uint16_t)); + float f = half_to_float (h); + uint32_t bits; + memcpy (&bits, &f, sizeof (bits)); + unaligned_store32 (rowBytes + x * sizeof (float), bits); } } }