pixman: Branch 'master'
Søren Sandmann Pedersen
sandmann at kemper.freedesktop.org
Mon Jul 2 09:22:25 PDT 2007
TODO | 90 +++++++++++++++++++++++++++++++++++++-----------
pixman/pixman-compose.c | 2 -
pixman/pixman-image.c | 4 +-
pixman/pixman-pict.c | 2 -
pixman/pixman-private.h | 3 +
pixman/pixman-region.c | 68 +++++++++++++++++++++++++++---------
pixman/pixman-utils.c | 24 ++++++++++++
pixman/pixman.h | 2 -
8 files changed, 155 insertions(+), 40 deletions(-)
New commits:
diff-tree bbef73192e558695933d7f05befaa8c18550bb63 (from 2e61f30e4c8d0e01e175495e13a5f132521ad6f2)
Author: Søren Sandmann <sandmann at redhat.com>
Date: Mon Jul 2 12:18:42 2007 -0400
Port Vlad's fixes for integer overflows with malloc().
diff --git a/TODO b/TODO
index 92beb0b..6649c69 100644
--- a/TODO
+++ b/TODO
@@ -1,8 +1,15 @@
+ - Go through things marked FIXME
+
+ - Add calls to prepare and finish access where necessary. grep for
+ ACCESS_MEM, and make sure they are correctly wrapped in prepare
+ and finish.
+
+ - restore READ/WRITE in the fbcompose combiners since they sometimes
+ store directly to destination drawables.
+
- It probably makes sense to move the more strange X region API
into pixman as well, but guarded with PIXMAN_XORG_COMPATIBILITY
- - Go through things marked FIXME
-
- Reinstate the FbBits typedef? At the moment we don't
even have the FbBits type; we just use uint32_t everywhere.
@@ -14,33 +21,78 @@
this, I suggest just using 32-bit datatypes by setting
IC_SHIFT to 5 for all machines.
-- Consider whether calling regions region16 is really such a great idea
+ - Consider whether calling regions region16 is really such a great
+ idea Vlad wants 32 bit regions for Cairo. This will break X server
+ ABI, but should otherwise be mostly harmless, though a
+ pixman_region_get_boxes16() may be useful.
+
+ - Make source clipping optional.
+ - done: source clipping happens through an indirection.
+ still needs to make the indirection settable. (And call it
+ from X)
+
+ - Consider optimizing the 8/16 bit solid fills in pixman-util.c by
+ storing more than one value at a time.
+
+ - Add an image cache to prevent excessive malloc/free. Note that pixman
+ needs to be thread safe when used from cairo.
+
+ - Review the pixman_format_code_t enum to make sure it will support
+ future formats. Some formats we will probably need:
+
+ ARGB/ABGR with 16/32/64 bit integer/floating channels
+ YUV2,
+ YV12
+
+ Also we may need the ability to distinguish between PICT_c8 and
+ PICT_x4c4. (This could be done by interpreting the A channel as
+ the depth for TYPE_COLOR and TYPE_GRAY formats).
+
+ A possibility may be to reserve the two top bits and make them
+ encode "number of places to shift the channel widths given" Since
+ these bits are 00 at the moment everything will continue to work,
+ but these additional widths will be allowed:
+
+ All even widths between 18-32
+ All multiples of four widths between 33 and 64
+ All multiples of eight between 64 and 128
+
+ This means things like r21g22b21 won't work - is that worth
+ worrying about? I don't think so. And of course the bpp field
+ can't handle a depth of over 256, so > 64 bit channels arent'
+ really all that useful.
+
+ We could reserve one extra bit to indicate floating point, but
+ we may also just add
+
+ PIXMAN_TYPE_ARGB_FLOAT
+ PIXMAN_TYPE_BGRA_FLOAT
+ PIXMAN_TYPE_A_FLOAT
+
+ image types. With five bits we can support up to 32 different
+ format types, which should be enough for everybody, even if we
+ decide to support all the various video formats here:
+
+ http://www.fourcc.org/yuv.php
+
+ It may make sense to have a PIXMAN_TYPE_YUV, and then use the
+ channel bits to specify the exact subtype.
+
+ What about color spaces such a linear vs. srGB etc.?
+
+
+done:
- Run cairo test suite; fix bugs
- one bug in source-scale-clip
- Remove the warning suppression in the ACCESS_MEM macro and fix the
warnings that are real
-
-- Add calls to prepare and finish access where necessary.
- grep for ACCESS_MEM, and make sure they are correctly wrapped in
- prepare and finish
-
-- Make source clipping optional.
- - done: source clipping happens through an indirection.
- still needs to make the indirection settable.
+ - irrelevant now.
- make the wrapper functions global instead of image specific
- this won't work since pixman is linked to both fb and wfb
-- restore READ/WRITE in the fbcompose combiners since they sometimes
- store directly to destination drawables.
-
-- Consider optimizing the 8/16 bit solid fills in pixman-util.c by
- storing more than one value at a time.
-
-done:
-
- Add non-mmx solid fill
- Make sure the endian-ness macros are defined correctly.
diff --git a/pixman/pixman-compose.c b/pixman/pixman-compose.c
index e336e04..e7f80f8 100644
--- a/pixman/pixman-compose.c
+++ b/pixman/pixman-compose.c
@@ -4074,7 +4074,7 @@ static void fbFetchExternalAlpha(bits_im
return;
}
if (width > SCANLINE_BUFFER_LENGTH)
- alpha_buffer = (uint32_t *) malloc(width*sizeof(uint32_t));
+ alpha_buffer = (uint32_t *) pixman_malloc_ab (width, sizeof(uint32_t));
fbFetchTransformed(pict, x, y, width, buffer, mask, maskBits);
fbFetchTransformed((bits_image_t *)pict->common.alpha_map, x - pict->common.alpha_origin.x,
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index f22d2c0..ca186a3 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -44,7 +44,7 @@ init_gradient (gradient_t *gradient,
init_source_image (&gradient->common);
- gradient->stops = malloc (n_stops * sizeof (pixman_gradient_stop_t));
+ gradient->stops = pixman_malloc_ab (n_stops, sizeof (pixman_gradient_stop_t));
if (!gradient->stops)
return FALSE;
@@ -443,7 +443,7 @@ pixman_image_set_filter (pixman_image_t
new_params = NULL;
if (params)
{
- new_params = malloc (n_params * sizeof (pixman_fixed_t));
+ new_params = pixman_malloc_ab (n_params, sizeof (pixman_fixed_t));
if (!new_params)
return FALSE;
diff --git a/pixman/pixman-pict.c b/pixman/pixman-pict.c
index cad11dd..b8b7b76 100644
--- a/pixman/pixman-pict.c
+++ b/pixman/pixman-pict.c
@@ -1359,7 +1359,7 @@ pixman_image_composite_rect (pixman_op_
if (width > SCANLINE_BUFFER_LENGTH)
{
- scanline_buffer = (uint32_t *)malloc (width * 3 * sizeof (uint32_t));
+ scanline_buffer = (uint32_t *)pixman_malloc_abc (width, 3, sizeof (uint32_t));
if (!scanline_buffer)
return;
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 9b89dee..89abf8f 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -44,6 +44,9 @@
#define FB_MASK (FB_UNIT - 1)
#define FB_ALLONES ((uint32_t) -1)
+/* Memory allocation helpers */
+void *pixman_malloc_ab (unsigned int n, unsigned int b);
+void *pixman_malloc_abc (unsigned int a, unsigned int b, unsigned int c);
#if DEBUG
diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c
index 08ce2e2..ffd92d6 100644
--- a/pixman/pixman-region.c
+++ b/pixman/pixman-region.c
@@ -69,7 +69,6 @@ typedef struct pixman_region16_point {
#define PIXREGION_BOX(reg,i) (&PIXREGION_BOXPTR(reg)[i])
#define PIXREGION_TOP(reg) PIXREGION_BOX(reg, (reg)->data->numRects)
#define PIXREGION_END(reg) PIXREGION_BOX(reg, (reg)->data->numRects - 1)
-#define PIXREGION_SZOF(n) (sizeof(pixman_region16_data_t) + ((n) * sizeof(pixman_box16_t)))
#undef assert
@@ -186,7 +185,29 @@ pixman_break (pixman_region16_t *pReg);
((r1)->y1 <= (r2)->y1) && \
((r1)->y2 >= (r2)->y2) )
-#define allocData(n) malloc(PIXREGION_SZOF(n))
+static size_t
+PIXREGION_SZOF(size_t n)
+{
+ size_t size = n * sizeof(pixman_box16_t);
+ if (n > UINT32_MAX / sizeof(pixman_box16_t))
+ return 0;
+
+ if (sizeof(pixman_region16_data_t) > UINT32_MAX - size)
+ return 0;
+
+ return size + sizeof(pixman_region16_data_t);
+}
+
+static void *
+allocData(size_t n)
+{
+ size_t sz = PIXREGION_SZOF(n);
+ if (!sz)
+ return NULL;
+
+ return malloc(sz);
+}
+
#define freeData(reg) if ((reg)->data && (reg)->data->size) free((reg)->data)
#define RECTALLOC_BAIL(pReg,n,bail) \
@@ -219,17 +240,21 @@ if (!(pReg)->data || (((pReg)->data->num
assert(pReg->data->numRects<=pReg->data->size); \
}
-#define DOWNSIZE(reg,numRects) \
-if (((numRects) < ((reg)->data->size >> 1)) && ((reg)->data->size > 50)) \
-{ \
- pixman_region16_data_t * NewData; \
- NewData = (pixman_region16_data_t *)realloc((reg)->data, PIXREGION_SZOF(numRects)); \
- if (NewData) \
- { \
- NewData->size = (numRects); \
- (reg)->data = NewData; \
- } \
-}
+#define DOWNSIZE(reg,numRects) \
+ if (((numRects) < ((reg)->data->size >> 1)) && ((reg)->data->size > 50)) \
+ { \
+ pixman_region16_data_t * NewData; \
+ size_t data_size = PIXREGION_SZOF(numRects); \
+ if (!data_size) \
+ NewData = NULL; \
+ else \
+ NewData = (pixman_region16_data_t *)realloc((reg)->data, data_size); \
+ if (NewData) \
+ { \
+ NewData->size = (numRects); \
+ (reg)->data = NewData; \
+ } \
+ }
pixman_bool_t
pixman_region_equal(reg1, reg2)
@@ -365,6 +390,7 @@ pixman_rect_alloc (pixman_region16_t * r
}
else
{
+ size_t data_size;
if (n == 1)
{
n = region->data->numRects;
@@ -372,7 +398,11 @@ pixman_rect_alloc (pixman_region16_t * r
n = 250;
}
n += region->data->numRects;
- data = (pixman_region16_data_t *)realloc(region->data, PIXREGION_SZOF(n));
+ data_size = PIXREGION_SZOF(n);
+ if (!data_size)
+ data = NULL;
+ else
+ data = (pixman_region16_data_t *)realloc(region->data, PIXREGION_SZOF(n));
if (!data)
return pixman_break (region);
region->data = data;
@@ -1466,7 +1496,7 @@ pixman_region_validate(pixman_region16_t
/* Set up the first region to be the first rectangle in badreg */
/* Note that step 2 code will never overflow the ri[0].reg rects array */
- ri = (RegionInfo *) malloc(4 * sizeof(RegionInfo));
+ ri = (RegionInfo *) pixman_malloc_ab (4, sizeof(RegionInfo));
if (!ri)
return pixman_break (badreg);
sizeRI = 4;
@@ -1528,9 +1558,15 @@ pixman_region_validate(pixman_region16_t
/* Uh-oh. No regions were appropriate. Create a new one. */
if (sizeRI == numRI)
{
+ size_t data_size;
+
/* Oops, allocate space for new region information */
sizeRI <<= 1;
- rit = (RegionInfo *) realloc(ri, sizeRI * sizeof(RegionInfo));
+
+ data_size = sizeRI * sizeof(RegionInfo);
+ if (data_size / sizeRI != sizeof(RegionInfo))
+ goto bail;
+ rit = (RegionInfo *) realloc(ri, data_size);
if (!rit)
goto bail;
ri = rit;
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index aadb6e7..cdf115d 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -22,6 +22,7 @@
*/
#include <config.h>
+#include <stdlib.h>
#include "pixman.h"
#include "pixman-private.h"
#include "pixman-mmx.h"
@@ -366,3 +367,26 @@ pixman_line_fixed_edge_init (pixman_edge
bot->x + x_off_fixed,
bot->y + y_off_fixed);
}
+
+void *
+pixman_malloc_ab(unsigned int a,
+ unsigned int b)
+{
+ if (a >= INT32_MAX / b)
+ return NULL;
+
+ return malloc (a * b);
+}
+
+void *
+pixman_malloc_abc (unsigned int a,
+ unsigned int b,
+ unsigned int c)
+{
+ if (a >= INT32_MAX / b)
+ return NULL;
+ else if (a * b >= INT32_MAX / c)
+ return NULL;
+ else
+ return malloc (a * b * c);
+}
diff --git a/pixman/pixman.h b/pixman/pixman.h
index cd64c8d..bd6045f 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -363,7 +363,7 @@ struct pixman_indexed
PIXMAN_FORMAT_B(f))
#define PIXMAN_TYPE_OTHER 0
-#define PIXMAN_TYPE_A 1
+#define PIXMAN_TYPE_A 1
#define PIXMAN_TYPE_ARGB 2
#define PIXMAN_TYPE_ABGR 3
#define PIXMAN_TYPE_COLOR 4
More information about the xorg-commit
mailing list