]> git.ipfire.org Git - thirdparty/zlib-ng.git/commitdiff
crc_folding: Fix potential out-of-bounds access
authorNicolas Trangez <nicolas.trangez@scality.com>
Thu, 3 Mar 2016 17:17:43 +0000 (18:17 +0100)
committerHans Kristian Rosbach <hk-github@circlestorm.org>
Wed, 4 Sep 2019 06:55:05 +0000 (08:55 +0200)
In some (very rare) scenarios, the SIMD code in the `crc_folding` module
can perform out-of-bounds reads or writes, which could lead to GPF
crashes.

Here's the deal: when the `crc_fold_copy` function is called with a
non-zero `len` argument of less then 16, `src` is read through
`_mm_loadu_si128` which always reads 16 bytes. If the `src` pointer
points to a location which contains `len` bytes, but any of the `16 -
len` out-of-bounds bytes falls in unmapped memory, this operation will
trigger a GPF.

The same goes for the `dst` pointer when written to through
`_mm_storeu_si128`.

With this patch applied, the crash no longer occurs.

We first discovered this issue though Valgrind reporting an
out-of-bounds access while running a unit-test for some code derived
from `crc_fold_copy`. In general, the out-of-bounds read is not an issue
because reads only occur in sections which are definitely mapped
(assuming page size is a multiple of 16), and garbage bytes are ignored.
While giving this some more thought we realized for small `len` values
and `src` or `dst` pointers at a very specific place in the address
space can lead to GPFs.

- Minor tweaks to merge request by Jim Kukunas <james.t.kukunas@linux.intel.com>
- removed C11-isms
- use unaligned load
- better integrated w/ zlib (use zalign)
- removed full example code from commit msg

arch/x86/crc_folding.c

index e86b44939090cda66d498c545336ef154e6791d1..c80117c5d780f11ff6ebeed72e524fac71bc5462 100644 (file)
@@ -239,9 +239,14 @@ ZLIB_INTERNAL void crc_fold_copy(deflate_state *const s, unsigned char *dst, con
     __m128i xmm_crc_part;
 
     if (len < 16) {
+        char ALIGNED_(16) partial_buf[16] = { 0 };
+
         if (len == 0)
             return;
-        xmm_crc_part = _mm_loadu_si128((__m128i *)src);
+
+        memcpy(partial_buf, src, len);
+        xmm_crc_part = _mm_loadu_si128((const __m128i *)partial_buf);
+        memcpy(dst, partial_buf, len);
         goto partial;
     }
 
@@ -348,8 +353,8 @@ ZLIB_INTERNAL void crc_fold_copy(deflate_state *const s, unsigned char *dst, con
         xmm_crc_part = _mm_load_si128((__m128i *)src);
     }
 
-partial:
     _mm_storeu_si128((__m128i *)dst, xmm_crc_part);
+partial:
     partial_fold(len, &xmm_crc0, &xmm_crc1, &xmm_crc2, &xmm_crc3, &xmm_crc_part);
 done:
     /* CRC_SAVE */