glamor-egl, parameter checking

Seth Arnold seth.arnold at canonical.com
Wed Sep 25 18:29:07 PDT 2013


Hello, I recently gave glamor-egl a very quick audit, and found some
issues that I wanted to run past someone who would know the code better
than I do.

- _glamor_poly_lines():
  - unchecked malloc() return value
  - if n = 1 is passed to the function, the malloc might allocate zero
    bytes:
            rects = malloc(sizeof(xRectangle) * (n - 1));
  Can n = 1 be a realistic input to this function? It appears to be
  callable from outside the library with arbitrary inputs.
  Is this safe?

- _pixman_region_init_clipped_rectangles()
  - The unsigned int num_rects argument is passed, unchecked, to
    boxes = malloc(sizeof(pixman_box16_t) * num_rects);
    -- can a large-enough value of num_rects cause a multiplication
    overflow here, allocating less memory than necessary?
  It appears to be callable from outside the library with arbitrary
  inputs. Is this safe?

- glamor_create_composite_fs() appears to have two unguarded divisions
  in relocate_texture that might result in divide-by-zero, wh.x and wh.y
  -- are these guarded somewhere else?

- glamor_create_composite_fs() appears to have an unguarded division in
  rel_sampler that might result in divide-by-zero, wh.xy -- is this
  guarded somewhere else?

- glamor_pixmap_attach_fbo() has a switch statement that uses
  fall-through after a block of code, but there's no comment nearby to
  assure the reader that it is intentional. Is it intentional? :)

There are several unchecked memory allocations:
  - glamor_compile_glsl_prog() unchecked malloc() return value
  - glamor_egl_init() unchecked calloc() return value glamor_egl
  - glamor_compute_clipped_regions_ext() unchecked calloc() return value
    result_regions
  - __glamor_compute_clipped_regions() unchecked calloc() return value
    clipped_regions
  - glamor_composite_largepixmap_region() unchecked malloc() return value
    source_pixmap_priv

(While it's true malloc() will almost never return NULL, someone may
someday wish to run this code with overcommit turned off, and it'd be
better to be safe.)

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130925/5248b3f9/attachment.pgp>


More information about the xorg-devel mailing list