Skip to content

Commit 274e03c

Browse files
pixelflingerpixelflinger
committed
fix crash when validating an invalid EGL objects
the code that validated EGL objects dereferenced the object to access its EGLDisplay -- needed for validation (!). This was wrong for two reasons, first we dereferenced the object before validating it (potentially leading to a crash), secondly we didn't validate that the object existed in the right EGLDisplay. We now use the EGLDisplay passed by the user API. Change-Id: I66f9e851d4f8507892a6b1fee3065f124c4e7138
1 parent 6b228af commit 274e03c

File tree

6 files changed

+48
-41
lines changed

6 files changed

+48
-41
lines changed

‎opengl/libs/EGL/egl.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,16 +212,20 @@ egl_connection_t* validate_display_config(EGLDisplay dpy, EGLConfig config,
212212

213213
EGLImageKHR egl_get_image_for_current_context(EGLImageKHR image)
214214
{
215-
ImageRef _i(image);
216-
if (!_i.get())
217-
return EGL_NO_IMAGE_KHR;
218-
219215
EGLContext context = egl_tls_t::getContext();
220216
if (context == EGL_NO_CONTEXT || image == EGL_NO_IMAGE_KHR)
221217
return EGL_NO_IMAGE_KHR;
222218

223219
egl_context_t const * const c = get_context(context);
224-
if (c == NULL) // this should never happen
220+
if (c == NULL) // this should never happen, by construction
221+
return EGL_NO_IMAGE_KHR;
222+
223+
egl_display_t* display = egl_display_t::get(c->dpy);
224+
if (display == NULL) // this should never happen, by construction
225+
return EGL_NO_IMAGE_KHR;
226+
227+
ImageRef _i(display, image);
228+
if (!_i.get())
225229
return EGL_NO_IMAGE_KHR;
226230

227231
// here we don't validate the context because if it's been marked for

‎opengl/libs/EGL/eglApi.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ EGLBoolean eglDestroySurface(EGLDisplay dpy, EGLSurface surface)
451451
egl_display_t const * const dp = validate_display(dpy);
452452
if (!dp) return EGL_FALSE;
453453

454-
SurfaceRef _s(surface);
454+
SurfaceRef _s(dp, surface);
455455
if (!_s.get())
456456
return setError(EGL_BAD_SURFACE, EGL_FALSE);
457457

@@ -472,7 +472,7 @@ EGLBoolean eglQuerySurface( EGLDisplay dpy, EGLSurface surface,
472472
egl_display_t const * const dp = validate_display(dpy);
473473
if (!dp) return EGL_FALSE;
474474

475-
SurfaceRef _s(surface);
475+
SurfaceRef _s(dp, surface);
476476
if (!_s.get())
477477
return setError(EGL_BAD_SURFACE, EGL_FALSE);
478478

@@ -541,7 +541,7 @@ EGLBoolean eglDestroyContext(EGLDisplay dpy, EGLContext ctx)
541541
if (!dp)
542542
return EGL_FALSE;
543543

544-
ContextRef _c(ctx);
544+
ContextRef _c(dp, ctx);
545545
if (!_c.get())
546546
return setError(EGL_BAD_CONTEXT, EGL_FALSE);
547547

@@ -592,9 +592,9 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw,
592592
}
593593

594594
// get a reference to the object passed in
595-
ContextRef _c(ctx);
596-
SurfaceRef _d(draw);
597-
SurfaceRef _r(read);
595+
ContextRef _c(dp, ctx);
596+
SurfaceRef _d(dp, draw);
597+
SurfaceRef _r(dp, read);
598598

599599
// validate the context (if not EGL_NO_CONTEXT)
600600
if ((ctx != EGL_NO_CONTEXT) && !_c.get()) {
@@ -696,7 +696,7 @@ EGLBoolean eglQueryContext( EGLDisplay dpy, EGLContext ctx,
696696
egl_display_t const * const dp = validate_display(dpy);
697697
if (!dp) return EGL_FALSE;
698698

699-
ContextRef _c(ctx);
699+
ContextRef _c(dp, ctx);
700700
if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
701701

702702
egl_context_t * const c = get_context(ctx);
@@ -944,7 +944,7 @@ EGLBoolean eglSwapBuffers(EGLDisplay dpy, EGLSurface draw)
944944
egl_display_t const * const dp = validate_display(dpy);
945945
if (!dp) return EGL_FALSE;
946946

947-
SurfaceRef _s(draw);
947+
SurfaceRef _s(dp, draw);
948948
if (!_s.get())
949949
return setError(EGL_BAD_SURFACE, EGL_FALSE);
950950

@@ -960,7 +960,7 @@ EGLBoolean eglCopyBuffers( EGLDisplay dpy, EGLSurface surface,
960960
egl_display_t const * const dp = validate_display(dpy);
961961
if (!dp) return EGL_FALSE;
962962

963-
SurfaceRef _s(surface);
963+
SurfaceRef _s(dp, surface);
964964
if (!_s.get())
965965
return setError(EGL_BAD_SURFACE, EGL_FALSE);
966966

@@ -1002,7 +1002,7 @@ EGLBoolean eglSurfaceAttrib(
10021002
egl_display_t const * const dp = validate_display(dpy);
10031003
if (!dp) return EGL_FALSE;
10041004

1005-
SurfaceRef _s(surface);
1005+
SurfaceRef _s(dp, surface);
10061006
if (!_s.get())
10071007
return setError(EGL_BAD_SURFACE, EGL_FALSE);
10081008

@@ -1022,7 +1022,7 @@ EGLBoolean eglBindTexImage(
10221022
egl_display_t const * const dp = validate_display(dpy);
10231023
if (!dp) return EGL_FALSE;
10241024

1025-
SurfaceRef _s(surface);
1025+
SurfaceRef _s(dp, surface);
10261026
if (!_s.get())
10271027
return setError(EGL_BAD_SURFACE, EGL_FALSE);
10281028

@@ -1042,7 +1042,7 @@ EGLBoolean eglReleaseTexImage(
10421042
egl_display_t const * const dp = validate_display(dpy);
10431043
if (!dp) return EGL_FALSE;
10441044

1045-
SurfaceRef _s(surface);
1045+
SurfaceRef _s(dp, surface);
10461046
if (!_s.get())
10471047
return setError(EGL_BAD_SURFACE, EGL_FALSE);
10481048

@@ -1201,7 +1201,7 @@ EGLBoolean eglLockSurfaceKHR(EGLDisplay dpy, EGLSurface surface,
12011201
egl_display_t const * const dp = validate_display(dpy);
12021202
if (!dp) return EGL_FALSE;
12031203

1204-
SurfaceRef _s(surface);
1204+
SurfaceRef _s(dp, surface);
12051205
if (!_s.get())
12061206
return setError(EGL_BAD_SURFACE, EGL_FALSE);
12071207

@@ -1220,7 +1220,7 @@ EGLBoolean eglUnlockSurfaceKHR(EGLDisplay dpy, EGLSurface surface)
12201220
egl_display_t const * const dp = validate_display(dpy);
12211221
if (!dp) return EGL_FALSE;
12221222

1223-
SurfaceRef _s(surface);
1223+
SurfaceRef _s(dp, surface);
12241224
if (!_s.get())
12251225
return setError(EGL_BAD_SURFACE, EGL_FALSE);
12261226

@@ -1241,7 +1241,7 @@ EGLImageKHR eglCreateImageKHR(EGLDisplay dpy, EGLContext ctx, EGLenum target,
12411241
if (!dp) return EGL_NO_IMAGE_KHR;
12421242

12431243
if (ctx != EGL_NO_CONTEXT) {
1244-
ContextRef _c(ctx);
1244+
ContextRef _c(dp, ctx);
12451245
if (!_c.get())
12461246
return setError(EGL_BAD_CONTEXT, EGL_NO_IMAGE_KHR);
12471247
egl_context_t * const c = get_context(ctx);
@@ -1310,7 +1310,7 @@ EGLBoolean eglDestroyImageKHR(EGLDisplay dpy, EGLImageKHR img)
13101310
egl_display_t const * const dp = validate_display(dpy);
13111311
if (!dp) return EGL_FALSE;
13121312

1313-
ImageRef _i(img);
1313+
ImageRef _i(dp, img);
13141314
if (!_i.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE);
13151315

13161316
egl_image_t* image = get_image(img);
@@ -1349,7 +1349,7 @@ EGLSyncKHR eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_l
13491349
if (!dp) return EGL_NO_SYNC_KHR;
13501350

13511351
EGLContext ctx = eglGetCurrentContext();
1352-
ContextRef _c(ctx);
1352+
ContextRef _c(dp, ctx);
13531353
if (!_c.get())
13541354
return setError(EGL_BAD_CONTEXT, EGL_NO_SYNC_KHR);
13551355

@@ -1372,12 +1372,12 @@ EGLBoolean eglDestroySyncKHR(EGLDisplay dpy, EGLSyncKHR sync)
13721372
egl_display_t const * const dp = validate_display(dpy);
13731373
if (!dp) return EGL_FALSE;
13741374

1375-
SyncRef _s(sync);
1375+
SyncRef _s(dp, sync);
13761376
if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE);
13771377
egl_sync_t* syncObject = get_sync(sync);
13781378

13791379
EGLContext ctx = syncObject->context;
1380-
ContextRef _c(ctx);
1380+
ContextRef _c(dp, ctx);
13811381
if (!_c.get())
13821382
return setError(EGL_BAD_CONTEXT, EGL_FALSE);
13831383

@@ -1399,12 +1399,12 @@ EGLint eglClientWaitSyncKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint flags, EGLTi
13991399
egl_display_t const * const dp = validate_display(dpy);
14001400
if (!dp) return EGL_FALSE;
14011401

1402-
SyncRef _s(sync);
1402+
SyncRef _s(dp, sync);
14031403
if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE);
14041404
egl_sync_t* syncObject = get_sync(sync);
14051405

14061406
EGLContext ctx = syncObject->context;
1407-
ContextRef _c(ctx);
1407+
ContextRef _c(dp, ctx);
14081408
if (!_c.get())
14091409
return setError(EGL_BAD_CONTEXT, EGL_FALSE);
14101410

@@ -1424,13 +1424,13 @@ EGLBoolean eglGetSyncAttribKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint attribute
14241424
egl_display_t const * const dp = validate_display(dpy);
14251425
if (!dp) return EGL_FALSE;
14261426

1427-
SyncRef _s(sync);
1427+
SyncRef _s(dp, sync);
14281428
if (!_s.get())
14291429
return setError(EGL_BAD_PARAMETER, EGL_FALSE);
14301430

14311431
egl_sync_t* syncObject = get_sync(sync);
14321432
EGLContext ctx = syncObject->context;
1433-
ContextRef _c(ctx);
1433+
ContextRef _c(dp, ctx);
14341434
if (!_c.get())
14351435
return setError(EGL_BAD_CONTEXT, EGL_FALSE);
14361436

@@ -1455,7 +1455,7 @@ EGLBoolean eglSetSwapRectangleANDROID(EGLDisplay dpy, EGLSurface draw,
14551455
egl_display_t const * const dp = validate_display(dpy);
14561456
if (!dp) return EGL_FALSE;
14571457

1458-
SurfaceRef _s(draw);
1458+
SurfaceRef _s(dp, draw);
14591459
if (!_s.get())
14601460
return setError(EGL_BAD_SURFACE, EGL_FALSE);
14611461

‎opengl/libs/EGL/egl_display.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ void egl_display_t::removeObject(egl_object_t* object) {
6262
objects.remove(object);
6363
}
6464

65-
bool egl_display_t::getObject(egl_object_t* object) {
65+
bool egl_display_t::getObject(egl_object_t* object) const {
6666
Mutex::Autolock _l(lock);
6767
if (objects.indexOf(object) >= 0) {
68-
object->incRef();
69-
return true;
68+
if (object->getDisplay() == this) {
69+
object->incRef();
70+
return true;
71+
}
7072
}
7173
return false;
7274
}

‎opengl/libs/EGL/egl_display.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class EGLAPI egl_display_t { // marked as EGLAPI for testing purposes
8181
// remove object from this display's list
8282
void removeObject(egl_object_t* object);
8383
// add reference to this object. returns true if this is a valid object.
84-
bool getObject(egl_object_t* object);
84+
bool getObject(egl_object_t* object) const;
8585

8686

8787
static egl_display_t* get(EGLDisplay dpy);
@@ -119,9 +119,9 @@ class EGLAPI egl_display_t { // marked as EGLAPI for testing purposes
119119
egl_config_t* configs;
120120

121121
private:
122-
uint32_t refs;
123-
Mutex lock;
124-
SortedVector<egl_object_t*> objects;
122+
uint32_t refs;
123+
mutable Mutex lock;
124+
SortedVector<egl_object_t*> objects;
125125
};
126126

127127
// ----------------------------------------------------------------------------

‎opengl/libs/EGL/egl_object.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ void egl_object_t::destroy() {
5555
}
5656
}
5757

58-
bool egl_object_t::get() {
58+
bool egl_object_t::get(egl_display_t const* display, egl_object_t* object) {
5959
// used by LocalRef, this does an incRef() atomically with
6060
// checking that the object is valid.
61-
return display->getObject(this);
61+
return display->getObject(object);
6262
}
6363

6464
// ----------------------------------------------------------------------------

‎opengl/libs/EGL/egl_object.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,11 @@ class egl_object_t {
5252

5353
inline int32_t incRef() { return android_atomic_inc(&count); }
5454
inline int32_t decRef() { return android_atomic_dec(&count); }
55+
inline egl_display_t* getDisplay() const { return display; }
5556

5657
private:
5758
void terminate();
58-
bool get();
59+
static bool get(egl_display_t const* display, egl_object_t* object);
5960

6061
public:
6162
template <typename N, typename T>
@@ -66,9 +67,9 @@ class egl_object_t {
6667
public:
6768
~LocalRef();
6869
explicit LocalRef(egl_object_t* rhs);
69-
explicit LocalRef(T o) : ref(0) {
70+
explicit LocalRef(egl_display_t const* display, T o) : ref(0) {
7071
egl_object_t* native = reinterpret_cast<N*>(o);
71-
if (o && native->get()) {
72+
if (o && egl_object_t::get(display, native)) {
7273
ref = native;
7374
}
7475
}

0 commit comments

Comments
 (0)