]> granicus.if.org Git - libvpx/commitdiff
Fix potential buffer over-read in highbd d117 predictor Neon
authorGeorge Steed <george.steed@arm.com>
Mon, 6 Mar 2023 13:24:47 +0000 (13:24 +0000)
committerGeorge Steed <george.steed@arm.com>
Mon, 6 Mar 2023 13:34:35 +0000 (13:34 +0000)
The load of `left[bs]` in the standard bitdepth d117 Neon implementation
triggered an address-sanitizer failure.

The highbd equivalent does not appear to trigger any asan failures when
running the VP9/ExternalFrameBufferMD5Test or
VP9/TestVectorTest.MD5Match tests, but for consistency with the standard
bitdepth implementation we adjust it to avoid the over-read.

Performance is roughly identical, with a 0.8% performance improvement on
average over the previous optimised code.

Change-Id: I05dc4d43f244f4915c0ccc52cc0af999bbacb018

vpx_dsp/arm/highbd_intrapred_neon.c

index d1e335c263874e987f79a5c84894bb56b60b5dbc..dc1b27dc105753ab75504c31a1ddacc05ca48335 100644 (file)
@@ -465,7 +465,11 @@ void vpx_highbd_d117_predictor_4x4_neon(uint16_t *dst, ptrdiff_t stride,
   l0az = vext_u16(vld1_dup_u16(left), az, 3);
 
   l0 = vld1_u16(left + 0);
-  l1 = vld1_u16(left + 1);
+  // The last lane here is unused, reading left[4] could cause a buffer
+  // over-read, so just fill with a duplicate of left[0] to avoid needing to
+  // materialize a zero:
+  // [ left[1], left[2], left[3], x ]
+  l1 = vext_u16(l0, l0, 1);
   // [ above[-1], left[0], left[1], left[2] ]
   azl0 = vext_u16(vld1_dup_u16(above - 1), l0, 3);
 
@@ -494,7 +498,11 @@ void vpx_highbd_d117_predictor_8x8_neon(uint16_t *dst, ptrdiff_t stride,
   l0az = vextq_u16(vld1q_dup_u16(left), az, 7);
 
   l0 = vld1q_u16(left + 0);
-  l1 = vld1q_u16(left + 1);
+  // The last lane here is unused, reading left[8] could cause a buffer
+  // over-read, so just fill with a duplicate of left[0] to avoid needing to
+  // materialize a zero:
+  // [ left[1], ... , left[7], x ]
+  l1 = vextq_u16(l0, l0, 1);
   // [ above[-1], left[0], ..., left[6] ]
   azl0 = vextq_u16(vld1q_dup_u16(above - 1), l0, 7);
 
@@ -565,7 +573,11 @@ void vpx_highbd_d117_predictor_16x16_neon(uint16_t *dst, ptrdiff_t stride,
   l1 = vld1q_u16(left + 1);
   l7 = vld1q_u16(left + 7);
   l8 = vld1q_u16(left + 8);
-  l9 = vld1q_u16(left + 9);
+  // The last lane here is unused, reading left[16] could cause a buffer
+  // over-read, so just fill with a duplicate of left[8] to avoid needing to
+  // materialize a zero:
+  // [ left[9], ... , left[15], x ]
+  l9 = vextq_u16(l8, l8, 1);
   // [ above[-1], left[0], ..., left[6] ]
   azl0 = vextq_u16(vld1q_dup_u16(above - 1), l0, 7);
 
@@ -658,6 +670,11 @@ void vpx_highbd_d117_predictor_32x32_neon(uint16_t *dst, ptrdiff_t stride,
   l23 = vld1q_u16(left + 23);
   l24 = vld1q_u16(left + 24);
   l25 = vld1q_u16(left + 25);
+  // The last lane here is unused, reading left[32] could cause a buffer
+  // over-read, so just fill with a duplicate of left[24] to avoid needing to
+  // materialize a zero:
+  // [ left[25], ... , left[31], x ]
+  l25 = vextq_u16(l24, l24, 1);
   // [ above[-1], left[0], ..., left[6] ]
   azl0 = vextq_u16(vld1q_dup_u16(above - 1), l0, 7);