[Mesa-dev,12/15] egl: add eglCreateImage

Submitted by Marek Olšák on May 28, 2015, 12:27 p.m.

Details

Message ID CAAxE2A7YgeNuFRgb9fetorvk=e5qx-krKC=e8jWwt8VWCAU9-w@mail.gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák May 28, 2015, 12:27 p.m.
A new patch is attached. Please review.

Marek

On Wed, May 27, 2015 at 9:07 PM, Chad Versace <chad.versace@intel.com> wrote:
> On Fri 15 May 2015, Emil Velikov wrote:
>> On 12/05/15 22:54, Marek Olšák wrote:
>> > From: Marek Olšák <marek.olsak@amd.com>
>> >
>> > ---
>> >  src/egl/main/eglapi.c | 38 ++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 38 insertions(+)
>> >
>> > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>> > index 6457798..34a113b 100644
>> > --- a/src/egl/main/eglapi.c
>> > +++ b/src/egl/main/eglapi.c
>> > @@ -251,6 +251,30 @@ _eglUnlockDisplay(_EGLDisplay *dpy)
>> >  }
>> >
>> >
>> > +static EGLint *
>> > +_eglConvertAttribsToInt(const EGLAttrib *attr_list)
>> > +{
>> > +   EGLint *int_attribs = NULL;
>> > +
>> > +   /* Convert attributes from EGLAttrib[] to EGLint[] */
>> > +   if (attr_list) {
>> > +      int i, size = 0;
>> > +
>> > +      while (attr_list[size] != EGL_NONE)
>> > +         size += 2;
>> > +
>> > +      if (size) {
>> > +         size += 1; /* add space for EGL_NONE */
>> > +         int_attribs = malloc(size * sizeof(int_attribs[0]));
>> > +
>> > +         for (i = 0; i < size; i++)
>> > +            int_attribs[i] = attr_list[i];
>
>> In the unlikely event that malloc fails, it'll be nice to not crash.
>
> NAK.
>
> There is a stack overflow vulnerability here, even when malloc succeeds.
> An attacker can pass a very large but valid `EGLint *attrib_list` into
> an EGL entry point, forcing the size calculation given to malloc to
> overflow to a small positive integer.  Then _eglConvertAttribsToInt will
> blithely copy a portion (perhaps most) of the attacker's attrib list onto
> the stack!
>
> To prevent the stack overflow, _eglConvertAttribsToInt should use
> calloc() and abort if allocation fails.

Patch hide | download patch | download mbox

From acaa7cefac908d31f50c197f02413249aa3bf517 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <marek.olsak@amd.com>
Date: Tue, 12 May 2015 20:42:05 +0200
Subject: [PATCH 1/2] egl: add eglCreateImage (v2)

v2: - use calloc
    - return BAD_ALLOC if calloc fails
---
 src/egl/main/eglapi.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 9696869..e4fd44e 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -251,6 +251,31 @@  _eglUnlockDisplay(_EGLDisplay *dpy)
 }
 
 
+static EGLint *
+_eglConvertAttribsToInt(const EGLAttrib *attr_list)
+{
+   EGLint *int_attribs = NULL;
+
+   /* Convert attributes from EGLAttrib[] to EGLint[] */
+   if (attr_list) {
+      int i, size = 0;
+
+      while (attr_list[size] != EGL_NONE)
+         size += 2;
+
+      size += 1; /* add space for EGL_NONE */
+
+      int_attribs = calloc(size, sizeof(int_attribs[0]));
+      if (!int_attribs)
+         return NULL;
+
+      for (i = 0; i < size; i++)
+         int_attribs[i] = attr_list[i];
+   }
+   return int_attribs;
+}
+
+
 /**
  * This is typically the first EGL function that an application calls.
  * It associates a private _EGLDisplay object to the native display.
@@ -1255,6 +1280,22 @@  eglCreateImageKHR(EGLDisplay dpy, EGLContext ctx, EGLenum target,
 }
 
 
+EGLImage EGLAPIENTRY
+eglCreateImage(EGLDisplay dpy, EGLContext ctx, EGLenum target,
+               EGLClientBuffer buffer, const EGLAttrib *attr_list)
+{
+   EGLImage image;
+   EGLint *int_attribs = _eglConvertAttribsToInt(attr_list);
+
+   if (attr_list && !int_attribs)
+      RETURN_EGL_ERROR(NULL, EGL_BAD_ALLOC, EGL_NO_IMAGE);
+
+   image = eglCreateImageKHR(dpy, ctx, target, buffer, int_attribs);
+   free(int_attribs);
+   return image;
+}
+
+
 EGLBoolean EGLAPIENTRY
 eglDestroyImage(EGLDisplay dpy, EGLImage image)
 {
@@ -1751,6 +1792,7 @@  eglGetProcAddress(const char *procname)
       { "eglClientWaitSync", (_EGLProc) eglClientWaitSync },
       { "eglGetSyncAttrib", (_EGLProc) eglGetSyncAttrib },
       { "eglWaitSync", (_EGLProc) eglWaitSync },
+      { "eglCreateImage", (_EGLProc) eglCreateImage },
       { "eglDestroyImage", (_EGLProc) eglDestroyImage },
 #ifdef EGL_MESA_drm_display
       { "eglGetDRMDisplayMESA", (_EGLProc) eglGetDRMDisplayMESA },
-- 
2.1.0


Comments

On Thu 28 May 2015, Marek Olšák wrote:
> A new patch is attached. Please review.

LGTM.
Reviewed-by: Chad Versace <chad.versace@intel.com>