From 326953a662417ba998cdd0d570829cf72eee5bd1 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Wed, 12 Jan 2022 13:23:08 +0900 Subject: [PATCH] vrend: Check GL errors There are many combinations of invalid arguments for OpenGL functions and it is impractical to cover all of them. Even if nothing is wrong with the user, GL_CONTEXT_LOST and GL_OUT_OF_MEMORY can also occur in many GL functions due to hardware problems. They can leave the context in an invalid state which can result in a reliability or security issue. Check GL errors after an operation completes and prevent from using the GL context after a GL error occurred. spec@!opengl 1.5@draw-vertices, spec@!opengl 1.5@draw-vertices-user, and spec@!opengl 2.0@gl-2.0-vertexattribpointer are marked as crash in .gitlab-ci/expectations/host/piglit-virgl-gles-fails.txt because they require GL_DOUBLE specification for glVertexAttribPointer, which is not supported by OpenGL ES. Avoiding the crashes requires capability checks on the guest, which this change does not implement. Signed-off-by: Akihiko Odaki --- .../host/piglit-virgl-gles-fails.txt | 6 +++--- .../virt/piglit-virgl-gles-fails.txt | 1 - src/vrend_decode.c | 7 +++++-- src/vrend_renderer.c | 21 +++++++++++++++++++ src/vrend_renderer.h | 2 ++ tests/test_virgl_transfer.c | 2 +- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci/expectations/host/piglit-virgl-gles-fails.txt b/.gitlab-ci/expectations/host/piglit-virgl-gles-fails.txt index 53fa38a..f1604b9 100644 --- a/.gitlab-ci/expectations/host/piglit-virgl-gles-fails.txt +++ b/.gitlab-ci/expectations/host/piglit-virgl-gles-fails.txt @@ -3391,12 +3391,12 @@ spec@!opengl 1.4@gl-1.4-tex1d-2dborder,Fail spec@!opengl 1.4@tex-miplevel-selection-lod-bias,Fail spec@!opengl 1.5@depth-tex-compare,Fail spec@!opengl 1.5@draw-elements-user,Fail -spec@!opengl 1.5@draw-vertices,Fail -spec@!opengl 1.5@draw-vertices-user,Fail +spec@!opengl 1.5@draw-vertices,Crash +spec@!opengl 1.5@draw-vertices-user,Crash spec@!opengl 2.0@gl-2.0-edgeflag,Fail spec@!opengl 2.0@gl-2.0-edgeflag-immediate,Fail spec@!opengl 2.0@gl-2.0-large-point-fs,Fail -spec@!opengl 2.0@gl-2.0-vertexattribpointer,Fail +spec@!opengl 2.0@gl-2.0-vertexattribpointer,Crash spec@!opengl 2.0@occlusion-query-discard,Fail spec@!opengl 2.0@vertex-program-two-side enabled back2,Fail spec@!opengl 2.0@vertex-program-two-side enabled back2@vs and fs,Fail diff --git a/.gitlab-ci/expectations/virt/piglit-virgl-gles-fails.txt b/.gitlab-ci/expectations/virt/piglit-virgl-gles-fails.txt index c9f9260..1f214ba 100644 --- a/.gitlab-ci/expectations/virt/piglit-virgl-gles-fails.txt +++ b/.gitlab-ci/expectations/virt/piglit-virgl-gles-fails.txt @@ -3473,7 +3473,6 @@ spec@!opengl 1.5@draw-vertices-user,Fail spec@!opengl 2.0@gl-2.0-edgeflag,Fail spec@!opengl 2.0@gl-2.0-edgeflag-immediate,Fail spec@!opengl 2.0@gl-2.0-large-point-fs,Fail -spec@!opengl 2.0@gl-2.0-vertexattribpointer,Fail spec@!opengl 2.0@occlusion-query-discard,Fail spec@!opengl 2.0@vertex-program-two-side enabled back2,Fail spec@!opengl 2.0@vertex-program-two-side enabled back2@vs and fs,Fail diff --git a/src/vrend_decode.c b/src/vrend_decode.c index b734512..31b14f6 100644 --- a/src/vrend_decode.c +++ b/src/vrend_decode.c @@ -1570,8 +1570,9 @@ static int vrend_decode_ctx_transfer_3d(struct virgl_context *ctx, { TRACE_FUNC(); struct vrend_decode_ctx *dctx = (struct vrend_decode_ctx *)ctx; - return vrend_renderer_transfer_iov(dctx->grctx, res->res_id, info, - transfer_mode); + int ret = vrend_renderer_transfer_iov(dctx->grctx, res->res_id, info, + transfer_mode); + return vrend_check_no_error(dctx->grctx) || ret ? ret : EINVAL; } static int vrend_decode_ctx_get_blob(struct virgl_context *ctx, @@ -1734,6 +1735,8 @@ static int vrend_decode_ctx_submit_cmd(struct virgl_context *ctx, TRACE_SCOPE_SLOW(vrend_get_comand_name(cmd)); ret = decode_table[cmd](gdctx->grctx, buf, len); + if (!vrend_check_no_error(gdctx->grctx) && !ret) + ret = EINVAL; if (ret) { vrend_printf("context %d failed to dispatch %s: %d\n", gdctx->base.ctx_id, vrend_get_comand_name(cmd), ret); diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c index 10b3191..6e4ba85 100644 --- a/src/vrend_renderer.c +++ b/src/vrend_renderer.c @@ -6452,6 +6452,22 @@ static uint64_t vrend_pipe_resource_get_size(struct pipe_resource *pres, return res->size; } +bool vrend_check_no_error(struct vrend_context *ctx) +{ + GLenum err; + + err = glGetError(); + if (err == GL_NO_ERROR) + return true; + + while (err != GL_NO_ERROR) { + vrend_report_context_error(ctx, VIRGL_ERROR_CTX_UNKNOWN, err); + err = glGetError(); + } + + return false; +} + const struct virgl_resource_pipe_callbacks * vrend_renderer_get_pipe_callbacks(void) { @@ -6611,6 +6627,11 @@ int vrend_renderer_init(const struct vrend_if_cbs *cbs, uint32_t flags) vrend_state.use_egl_fence = virgl_egl_supports_fences(egl); #endif + if (!vrend_check_no_error(vrend_state.ctx0)) { + vrend_renderer_fini(); + return EINVAL; + } + return 0; } diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h index b40b943..e561dac 100644 --- a/src/vrend_renderer.h +++ b/src/vrend_renderer.h @@ -128,6 +128,8 @@ struct vrend_if_cbs { #define VREND_USE_EXTERNAL_BLOB (1 << 1) #define VREND_USE_ASYNC_FENCE_CB (1 << 2) +bool vrend_check_no_error(struct vrend_context *ctx); + const struct virgl_resource_pipe_callbacks * vrend_renderer_get_pipe_callbacks(void); diff --git a/tests/test_virgl_transfer.c b/tests/test_virgl_transfer.c index 3c53c3d..ad15f75 100644 --- a/tests/test_virgl_transfer.c +++ b/tests/test_virgl_transfer.c @@ -879,7 +879,7 @@ START_TEST(virgl_test_copy_transfer_to_staging_without_iov_fails) virgl_encoder_copy_transfer(&ctx, &dst_res, 0, 0, &box, &src_res, 0, synchronized); ret = virgl_renderer_submit_cmd(ctx.cbuf->buf, ctx.ctx_id, ctx.cbuf->cdw); - ck_assert_int_eq(ret, 0); + ck_assert_int_eq(ret, EINVAL); virgl_renderer_ctx_detach_resource(ctx.ctx_id, src_res.handle); virgl_renderer_ctx_detach_resource(ctx.ctx_id, dst_res.handle);