[Pixman] [PATCH] Avoid out-of-bounds read when accessing individual bytes from mask

Jonathan Kew jfkthame at gmail.com
Tue Mar 16 12:43:57 UTC 2021


The attached patch fixes an out-of-bounds read error detected by ASAN.

The important changes here are a handful of places where we replace

             memcpy(&m, mask++, sizeof(uint32_t));

or similar code with

             uint8_t m = *mask++;

because we're only supposed to be reading a single byte from *mask,
and accessing a 32-bit value may read out of bounds (besides that
it reads values we don't actually want; whether this matters would
depend exactly how the value in m is subsequently used).

I've also changed a bunch of other places to use this same pattern
(a local 8-bit variable) when reading individual bytes from the mask;
the code was inconsistent about this, sometimes casting the byte to
a uint32_t instead. This makes no actual difference, it just seemed
better to use a consistent pattern throughout the file.
-------------- next part --------------
From 54162f5ef27bb56e307d35e72c8c9b63b512c575 Mon Sep 17 00:00:00 2001
From: Jonathan Kew <jfkthame at gmail.com>
Date: Tue, 16 Mar 2021 12:25:01 +0000
Subject: [PATCH] Avoid out-of-bounds read when accessing individual bytes from
 mask.

The important changes here are a handful of places where we replace

            memcpy(&m, mask++, sizeof(uint32_t));

or similar code with

            uint8_t m = *mask++;

because we're only supposed to be reading a single byte from *mask,
and accessing a 32-bit value may read out of bounds (besides that
it reads values we don't actually want; whether this matters would
depend exactly how the value in m is subsequently used).

I've also changed a bunch of other places to use this same pattern
(a local 8-bit variable) when reading individual bytes from the mask;
the code was inconsistent about this, sometimes casting the byte to
a uint32_t instead. This makes no actual difference, it just seemed
better to use a consistent pattern throughout the file.
---
 pixman/pixman-sse2.c | 45 ++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/pixman/pixman-sse2.c b/pixman/pixman-sse2.c
index 2644b0a..ce4e75f 100644
--- a/pixman/pixman-sse2.c
+++ b/pixman/pixman-sse2.c
@@ -3202,7 +3202,7 @@ sse2_composite_over_n_8_8888 (pixman_implementation_t *imp,
     uint8_t *mask_line, *mask;
     int dst_stride, mask_stride;
     int32_t w;
-    uint32_t m, d;
+    uint32_t d;
 
     __m128i xmm_src, xmm_alpha, xmm_def;
     __m128i xmm_dst, xmm_dst_lo, xmm_dst_hi;
@@ -3257,6 +3257,7 @@ sse2_composite_over_n_8_8888 (pixman_implementation_t *imp,
 
 	while (w >= 4)
 	{
+            uint32_t m;
             memcpy(&m, mask, sizeof(uint32_t));
 
 	    if (srca == 0xff && m == 0xffffffff)
@@ -3477,7 +3478,6 @@ sse2_composite_src_n_8_8888 (pixman_implementation_t *imp,
     uint8_t     *mask_line, *mask;
     int dst_stride, mask_stride;
     int32_t w;
-    uint32_t m;
 
     __m128i xmm_src, xmm_def;
     __m128i xmm_mask, xmm_mask_lo, xmm_mask_hi;
@@ -3529,6 +3529,7 @@ sse2_composite_src_n_8_8888 (pixman_implementation_t *imp,
 
 	while (w >= 4)
 	{
+            uint32_t m;
             memcpy(&m, mask, sizeof(uint32_t));
 
 	    if (srca == 0xff && m == 0xffffffff)
@@ -3595,7 +3596,6 @@ sse2_composite_over_n_8_0565 (pixman_implementation_t *imp,
     uint8_t     *mask_line, *mask;
     int dst_stride, mask_stride;
     int32_t w;
-    uint32_t m;
     __m128i mmx_src, mmx_alpha, mmx_mask, mmx_dest;
 
     __m128i xmm_src, xmm_alpha;
@@ -3627,7 +3627,7 @@ sse2_composite_over_n_8_0565 (pixman_implementation_t *imp,
 
 	while (w && (uintptr_t)dst & 15)
 	{
-	    m = *mask++;
+	    uint8_t m = *mask++;
 
 	    if (m)
 	    {
@@ -3647,6 +3647,8 @@ sse2_composite_over_n_8_0565 (pixman_implementation_t *imp,
 
 	while (w >= 8)
 	{
+            uint32_t m;
+
 	    xmm_dst = load_128_aligned ((__m128i*) dst);
 	    unpack_565_128_4x128 (xmm_dst,
 				  &xmm_dst0, &xmm_dst1, &xmm_dst2, &xmm_dst3);
@@ -3700,7 +3702,7 @@ sse2_composite_over_n_8_0565 (pixman_implementation_t *imp,
 
 	while (w)
 	{
-	    m = *mask++;
+	    uint8_t m = *mask++;
 
 	    if (m)
 	    {
@@ -4062,7 +4064,7 @@ sse2_composite_in_n_8_8 (pixman_implementation_t *imp,
     uint8_t     *dst_line, *dst;
     uint8_t     *mask_line, *mask;
     int dst_stride, mask_stride;
-    uint32_t d, m;
+    uint32_t d;
     uint32_t src;
     int32_t w;
 
@@ -4089,7 +4091,7 @@ sse2_composite_in_n_8_8 (pixman_implementation_t *imp,
 
 	while (w && ((uintptr_t)dst & 15))
 	{
-	    m = (uint32_t) *mask++;
+	    uint8_t m = *mask++;
 	    d = (uint32_t) *dst;
 
 	    *dst++ = (uint8_t) pack_1x128_32 (
@@ -4126,7 +4128,7 @@ sse2_composite_in_n_8_8 (pixman_implementation_t *imp,
 
 	while (w)
 	{
-	    m = (uint32_t) *mask++;
+	    uint8_t m = *mask++;
 	    d = (uint32_t) *dst;
 
 	    *dst++ = (uint8_t) pack_1x128_32 (
@@ -4303,7 +4305,7 @@ sse2_composite_add_n_8_8 (pixman_implementation_t *imp,
     int dst_stride, mask_stride;
     int32_t w;
     uint32_t src;
-    uint32_t m, d;
+    uint32_t d;
 
     __m128i xmm_alpha;
     __m128i xmm_mask, xmm_mask_lo, xmm_mask_hi;
@@ -4328,7 +4330,7 @@ sse2_composite_add_n_8_8 (pixman_implementation_t *imp,
 
 	while (w && ((uintptr_t)dst & 15))
 	{
-	    m = (uint32_t) *mask++;
+	    uint8_t m = *mask++;
 	    d = (uint32_t) *dst;
 
 	    *dst++ = (uint8_t) pack_1x128_32 (
@@ -4364,7 +4366,7 @@ sse2_composite_add_n_8_8 (pixman_implementation_t *imp,
 
 	while (w)
 	{
-	    m = (uint32_t) *mask++;
+	    uint8_t m = (uint32_t) *mask++;
 	    d = (uint32_t) *dst;
 
 	    *dst++ = (uint8_t) pack_1x128_32 (
@@ -4832,7 +4834,6 @@ sse2_composite_over_x888_8_8888 (pixman_implementation_t *imp,
     uint32_t    *src, *src_line, s;
     uint32_t    *dst, *dst_line, d;
     uint8_t         *mask, *mask_line;
-    uint32_t m;
     int src_stride, mask_stride, dst_stride;
     int32_t w;
     __m128i ms;
@@ -4861,8 +4862,8 @@ sse2_composite_over_x888_8_8888 (pixman_implementation_t *imp,
 
         while (w && (uintptr_t)dst & 15)
         {
+            uint8_t m = *mask++;
             s = 0xff000000 | *src++;
-            memcpy(&m, mask++, sizeof(uint32_t));
             d = *dst;
             ms = unpack_32_1x128 (s);
 
@@ -4880,6 +4881,7 @@ sse2_composite_over_x888_8_8888 (pixman_implementation_t *imp,
 
         while (w >= 4)
         {
+            uint32_t m;
             memcpy(&m, mask, sizeof(uint32_t));
             xmm_src = _mm_or_si128 (
 		load_128_unaligned ((__m128i*)src), mask_ff000000);
@@ -4916,7 +4918,7 @@ sse2_composite_over_x888_8_8888 (pixman_implementation_t *imp,
 
         while (w)
         {
-            memcpy(&m, mask++, sizeof(uint32_t));
+            uint8_t m = *mask++;
 
             if (m)
             {
@@ -4957,7 +4959,6 @@ sse2_composite_over_8888_8_8888 (pixman_implementation_t *imp,
     uint32_t    *src, *src_line, s;
     uint32_t    *dst, *dst_line, d;
     uint8_t         *mask, *mask_line;
-    uint32_t m;
     int src_stride, mask_stride, dst_stride;
     int32_t w;
 
@@ -4986,9 +4987,9 @@ sse2_composite_over_8888_8_8888 (pixman_implementation_t *imp,
         while (w && (uintptr_t)dst & 15)
         {
 	    uint32_t sa;
+            uint8_t m = *mask++;
 
             s = *src++;
-            m = (uint32_t) *mask++;
             d = *dst;
 
 	    sa = s >> 24;
@@ -5019,6 +5020,7 @@ sse2_composite_over_8888_8_8888 (pixman_implementation_t *imp,
 
         while (w >= 4)
         {
+            uint32_t m;
             memcpy(&m, mask, sizeof(uint32_t));
 
 	    if (m)
@@ -5058,9 +5060,9 @@ sse2_composite_over_8888_8_8888 (pixman_implementation_t *imp,
         while (w)
         {
 	    uint32_t sa;
+            uint8_t m = *mask++;
 
             s = *src++;
-            m = (uint32_t) *mask++;
             d = *dst;
 
 	    sa = s >> 24;
@@ -5927,13 +5929,11 @@ scaled_bilinear_scanline_sse2_8888_8_8888_OVER (uint32_t *       dst,
     intptr_t unit_x = unit_x_;
     BILINEAR_DECLARE_VARIABLES;
     uint32_t pix1, pix2;
-    uint32_t m;
 
     while (w && ((uintptr_t)dst & 15))
     {
 	uint32_t sa;
-
-	m = (uint32_t) *mask++;
+	uint8_t m = *mask++;
 
 	if (m)
 	{
@@ -5969,6 +5969,8 @@ scaled_bilinear_scanline_sse2_8888_8_8888_OVER (uint32_t *       dst,
 
     while (w >= 4)
     {
+        uint32_t m;
+
 	__m128i xmm_src, xmm_src_lo, xmm_src_hi, xmm_srca_lo, xmm_srca_hi;
 	__m128i xmm_dst, xmm_dst_lo, xmm_dst_hi;
 	__m128i xmm_mask, xmm_mask_lo, xmm_mask_hi;
@@ -6015,8 +6017,7 @@ scaled_bilinear_scanline_sse2_8888_8_8888_OVER (uint32_t *       dst,
     while (w)
     {
 	uint32_t sa;
-
-	m = (uint32_t) *mask++;
+	uint8_t m = *mask++;
 
 	if (m)
 	{
-- 
2.30.0



More information about the Pixman mailing list