diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 11ec47ca..5a891fff 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,7 +17,7 @@ variables: # # Pick a pipeline on https://gitlab.freedesktop.org/mesa/mesa/-/pipelines/ # - MESA_PIPELINE_ID: 1646089 + MESA_PIPELINE_ID: 1669832 MESA_PROJECT_PATH: mesa/mesa S3_JWT_FILE: /s3_jwt @@ -59,7 +59,7 @@ include: # IMPORTANT: Use a recent Mesa Git revision # The commit ref must be in sync with the pipeline picked above # It can be found on the pipeline page below the commit message - ref: cc4492204894c0488e96a93f51b546345ae32f88 + ref: 52d57d98a2948214a2d029f2f4e8b476bf39ada3 file: - '/.gitlab-ci/image-tags.yml' diff --git a/.gitlab-ci/expectations/virt/traces-virgl.yml b/.gitlab-ci/expectations/virt/traces-virgl.yml index 7ce54d5a..2841e1f1 100644 --- a/.gitlab-ci/expectations/virt/traces-virgl.yml +++ b/.gitlab-ci/expectations/virt/traces-virgl.yml @@ -109,7 +109,7 @@ traces: label: [crash] godot/Material Testers.x86_64_2020.04.08_13.38_frame799.rdc: gl-virgl: - checksum: 7ba4f0c52a719d94b805653d31ce22d7 + checksum: 465410a06d966b377bc8c41e52b21b0f ror/ror-default.trace: gl-virgl: label: [crash] diff --git a/.gitlab-ci/expectations/virt/venus-flakes.txt b/.gitlab-ci/expectations/virt/venus-flakes.txt index efe1ae51..66445d07 100644 --- a/.gitlab-ci/expectations/virt/venus-flakes.txt +++ b/.gitlab-ci/expectations/virt/venus-flakes.txt @@ -35,9 +35,12 @@ dEQP-VK.ray_query.acceleration_structures.operations.traditional_structures.comp dEQP-VK.ray_tracing_pipeline.acceleration_structures.flags.sparse_binding_structures.gpu_built.aabbs.identical_instances.padded.fastbuild_update_0_0_bothgeneric dEQP-VK.ray_tracing_pipeline.acceleration_structures.flags.traditional_structures.gpu_built.triangles.different_instances.padded.0_update_0_0_bothgeneric dEQP-VK.ray_tracing_pipeline.shader_binding_table.indexing_hit.sbt_offset_0.no_shaderrecord.1_5_extraSBTRecordStrideBits +dEQP-VK.reconvergence.maximal.compute.nesting2.5.18 +dEQP-VK.reconvergence.maximal.compute.nesting2.6.22 dEQP-VK.renderpasses.dynamic_rendering.primary_cmd_buff.random.seed88_geometry dEQP-VK.renderpasses.renderpass2.suballocation.attachment_allocation.roll.56 dEQP-VK.robustness.robustness2.push.notemplate.r32ui.unroll.volatile.storage_texel_buffer.no_fmt_qual.len_36.samples_1.1d.comp +dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24 dEQP-VK.sparse_resources.image_sparse_residency.mutable.2d_array.r16_unorm_r8g8_sint_r8g8_snorm dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.uniform_float_16_to_32.uniform_buffer_block_vector_float_const_idx_6_tesse dEQP-VK.spirv_assembly.instruction.graphics.16bit_storage.uniform_float_16_to_64.uniform_buffer_block_vector_float_geom @@ -46,6 +49,7 @@ dEQP-VK.spirv_assembly.type.scalar.u8.bit_field_s_extract_offset64_count64_comp dEQP-VK.synchronization.cross_instance.suballocated.write_clear_color_image_read_image_compute_indirect.image_64x64x8_r32_sfloat_timeline_semaphore_zircon_handle dEQP-VK.synchronization.op.multi_queue.fence.write_draw_read_image_vertex.image_128x128_r16_uint_concurrent_maintenance9 dEQP-VK.texture.swizzle.component_mapping.color.r16_snorm_2d_pot_bbbb +dEQP-VK.wsi.direct_drm.swapchain.create.image_array_layers dEQP-VK.api.info.image_format_properties2.2d.optimal.g16_b16r16_2plane_422_unorm dEQP-VK.api.info.image_format_properties2.2d.optimal.g8_b8r8_2plane_420_unorm diff --git a/config.h.meson b/config.h.meson index 55497ffb..3f894fd4 100644 --- a/config.h.meson +++ b/config.h.meson @@ -46,6 +46,7 @@ #mesondefine ENABLE_DRM_I915 #mesondefine ENABLE_LIBDRM #mesondefine ENABLE_RENDER_SERVER +#mesondefine ENABLE_SAME_PROCESS_RENDER_SERVER #mesondefine ENABLE_RENDER_SERVER_WORKER_PROCESS #mesondefine ENABLE_RENDER_SERVER_WORKER_THREAD #mesondefine ENABLE_RENDER_SERVER_WORKER_MINIJAIL diff --git a/meson.build b/meson.build index 94d19410..71139fef 100644 --- a/meson.build +++ b/meson.build @@ -346,8 +346,14 @@ if with_check_gl_errors endif with_venus = get_option('venus') -with_render_server = with_venus +with_render_server = '' +if with_venus + with_render_server = get_option('render-server-mode') +endif with_render_server_worker = get_option('render-server-worker') +if with_render_server_worker == 'auto' + with_render_server_worker = with_render_server +endif render_server_install_dir = get_option('prefix') / get_option('libexecdir') if with_venus and with_host_darwin add_languages('objc', required: true) @@ -361,7 +367,7 @@ if with_venus if get_option('vulkan-dload') conf_data.set('ENABLE_VULKAN_DLOAD', 1) else - venus_dep = dependency('vulkan') + venus_dep += dependency('vulkan') endif if host_machine.system() in ['freebsd', 'openbsd'] @@ -412,6 +418,13 @@ if with_venus else error('unknown render server worker ' + with_render_server_worker) endif + + if with_render_server == 'thread' + conf_data.set('ENABLE_SAME_PROCESS_RENDER_SERVER', 1) + if with_render_server_worker != 'thread' + error('render-server-worker must be \'thread\' if render-server-mode is \'thread\'') + endif + endif endif with_video = get_option('video') @@ -455,7 +468,7 @@ if not with_host_windows subdir('vtest') endif -if with_render_server +if with_render_server == 'process' subdir('server') endif @@ -477,7 +490,8 @@ summary({'c_args': (' ').join(get_option('c_args')), 'drm-i915': with_drm_i915, 'drm-panfrost': with_drm_panfrost, 'drm-msm': with_drm_msm, - 'render server worker': with_render_server ? with_render_server_worker : 'none', + 'render server mode': with_render_server == '' ? 'none' : with_render_server, + 'render server worker': with_render_server == '' ? 'none' : with_render_server_worker, 'video': with_video, 'tests': with_tests, 'fuzzer': with_fuzzer, diff --git a/meson_options.txt b/meson_options.txt index e7e67d91..fdf27c48 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -86,10 +86,18 @@ option( ) option( - 'render-server-worker', + 'render-server-mode', type : 'combo', value : 'process', - choices : ['process', 'thread', 'minijail'], + choices : ['process', 'thread'], + description : 'how render server itself is spawned' +) + +option( + 'render-server-worker', + type : 'combo', + value : 'auto', + choices : ['auto', 'process', 'thread', 'minijail'], description : 'how a context in render server is serviced' ) diff --git a/server/render_common.c b/server/render_common.c index e51bb88c..688d1a0f 100644 --- a/server/render_common.c +++ b/server/render_common.c @@ -4,6 +4,9 @@ */ #include "render_common.h" +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER +#include "virgl_util.h" +#endif #include #include @@ -21,6 +24,10 @@ render_log(const char *fmt, ...) va_list va; va_start(va, fmt); +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER + virgl_prefixed_logv("server", VIRGL_LOG_LEVEL_INFO, fmt, va); +#else vsyslog(LOG_DEBUG, fmt, va); +#endif va_end(va); } diff --git a/server/render_state.c b/server/render_state.c index 715f85e6..bc7f8e16 100644 --- a/server/render_state.c +++ b/server/render_state.c @@ -93,6 +93,7 @@ render_state_lookup_context(uint32_t ctx_id) return ctx; } +#ifndef ENABLE_SAME_PROCESS_RENDER_SERVER static void render_state_cb_debug_logger(UNUSED enum virgl_log_level_flags log_level, const char *message, @@ -100,6 +101,7 @@ render_state_cb_debug_logger(UNUSED enum virgl_log_level_flags log_level, { render_log(message); } +#endif static void render_state_cb_retire_fence(uint32_t ctx_id, uint32_t ring_idx, uint64_t fence_id) @@ -112,7 +114,9 @@ render_state_cb_retire_fence(uint32_t ctx_id, uint32_t ring_idx, uint64_t fence_ } static const struct vkr_renderer_callbacks render_state_cbs = { +#ifndef ENABLE_SAME_PROCESS_RENDER_SERVER .debug_logger = render_state_cb_debug_logger, +#endif .retire_fence = render_state_cb_retire_fence, }; diff --git a/server/render_worker.c b/server/render_worker.c index b8df351d..3e1cb6ee 100644 --- a/server/render_worker.c +++ b/server/render_worker.c @@ -33,7 +33,7 @@ #include #include #ifdef ENABLE_RENDER_SERVER_WORKER_THREAD -#include +#include "c11/threads.h" #endif #include diff --git a/src/drm/amdgpu/amdgpu_renderer.c b/src/drm/amdgpu/amdgpu_renderer.c index e7f47c34..80baf960 100644 --- a/src/drm/amdgpu/amdgpu_renderer.c +++ b/src/drm/amdgpu/amdgpu_renderer.c @@ -259,7 +259,7 @@ amdgpu_renderer_attach_resource(struct virgl_context *vctx, struct virgl_resourc obj->bo = import.buf_handle; amdgpu_bo_export(obj->bo, amdgpu_bo_handle_type_kms, &obj->base.handle); amdgpu_object_set_res_id(ctx, obj, res->res_id); - print(1, "imported dmabuf -> res_id=%u" PRIx64, res->res_id); + print(1, "imported dmabuf -> res_id=%u", res->res_id); } else { print(2, "Ignored res_id: %d (fd_type = %d)", res->res_id, fd_type); if (fd_type != VIRGL_RESOURCE_FD_INVALID) @@ -411,7 +411,7 @@ amdgpu_renderer_get_blob(struct virgl_context *vctx, uint32_t res_id, uint64_t b /* If GEM_NEW fails, we can end up here without a backing obj or if it's a dumb buffer. */ if (!obj) { - print(0, "No object with blob_id=%ld", blob_id); + print(0, "No object with blob_id=%" PRIu64, blob_id); return -ENOENT; } @@ -424,7 +424,7 @@ amdgpu_renderer_get_blob(struct virgl_context *vctx, uint32_t res_id, uint64_t b * to the same storage. */ if (obj->exported) { - print(0, "Already exported! blob_id:%ld", blob_id); + print(0, "Already exported! blob_id:%" PRIu64, blob_id); return -EINVAL; } @@ -436,7 +436,7 @@ amdgpu_renderer_get_blob(struct virgl_context *vctx, uint32_t res_id, uint64_t b ret = amdgpu_bo_export(obj->bo, amdgpu_bo_handle_type_dma_buf_fd, (uint32_t *)&fd); if (ret) { - print(0, "Export to fd failed for blob_id:%ld r=%d (%s)", blob_id, ret, strerror(errno)); + print(0, "Export to fd failed for blob_id:%" PRIu64 " r=%d (%s)", blob_id, ret, strerror(errno)); return ret; } @@ -470,7 +470,7 @@ amdgpu_ccmd_query_info(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) struct amdgpu_ccmd_query_info_rsp *rsp; unsigned rsp_len; if (__builtin_add_overflow(sizeof(*rsp), req->info.return_size, &rsp_len)) { - print(1, "%s: Request size overflow: %zu + %" PRIu32 " > %u", + print(1, "%s: Request size overflow: %zu + %u > %u", __FUNCTION__, sizeof(*rsp), req->info.return_size, UINT_MAX); return -EINVAL; } @@ -510,12 +510,12 @@ amdgpu_ccmd_gem_new(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) if (req->r.__pad) { print(0, "Invalid value for struct %s_req::r::__pad: " - "0x%" PRIx32, __FUNCTION__, req->r.__pad); + "0x%x", __FUNCTION__, req->r.__pad); ret = -EINVAL; goto alloc_failed; } if (!drm_context_blob_id_valid(dctx, req->blob_id)) { - print(0, "Invalid blob_id %ld", req->blob_id); + print(0, "Invalid blob_id %" PRIu64, req->blob_id); ret = -EINVAL; goto alloc_failed; } @@ -553,7 +553,7 @@ amdgpu_ccmd_gem_new(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) drm_context_object_set_blob_id(dctx, &obj->base, req->blob_id); - print(2, "new object blob_id: %ld heap: %08x flags: %lx size: %ld", + print(2, "new object blob_id: %" PRIu64 " heap: %08x flags: %" PRIx64" size: %" PRIu64, req->blob_id, req->r.preferred_heap, req->r.flags, req->r.alloc_size); return 0; @@ -562,7 +562,7 @@ amdgpu_ccmd_gem_new(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) amdgpu_bo_free(bo_handle); alloc_failed: - print(2, "ERROR blob_id: %ld heap: %08x flags: %lx", + print(2, "ERROR blob_id: %" PRIu64 " heap: %08x flags: %" PRIx64, req->blob_id, req->r.preferred_heap, req->r.flags); if (ctx->shmem) ctx->shmem->async_error++; @@ -593,7 +593,10 @@ amdgpu_ccmd_bo_va_op(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) } else { obj = amdgpu_get_object_from_res_id(ctx, req->res_id, __FUNCTION__); if (!obj) { - print(0, "amdgpu_bo_va_op_raw failed: op: %d res_id: %d offset: 0x%lx size: 0x%lx va: %" PRIx64 " r=%d", + print(0, + "amdgpu_bo_va_op_raw failed: " + "op: %d res_id: %d offset: 0x%" PRIx64 " " + "size: 0x%" PRIx64 " va: %" PRIx64 " r=%d", req->op, obj->base.res_id, req->offset, req->vm_map_size, req->va, rsp->ret); /* This is ok. This means the guest closed the GEM already. */ @@ -609,11 +612,16 @@ amdgpu_ccmd_bo_va_op(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) if (ctx->shmem) ctx->shmem->async_error++; - print(0, "amdgpu_bo_va_op_raw failed: op: %d res_id: %d offset: 0x%lx size: 0x%lx va: %" PRIx64 " r=%d", + print(0, + "amdgpu_bo_va_op_raw failed: " + "op: %d res_id: %d offset: 0x%" PRIx64 " " + "size: 0x%" PRIx64 " va: %" PRIx64 " r=%d", req->op, req->res_id, req->offset, req->vm_map_size, req->va, rsp->ret); } else { - print(2, "va_op %d res_id: %u va: [0x%" PRIx64 ", 0x%" PRIx64 "] @offset 0x%" PRIx64, - req->op, req->res_id, req->va, req->va + req->vm_map_size - 1, req->offset); + print(2, + "va_op %d res_id: %u " + "va: [0x%" PRIx64 ", 0x%" PRIx64 "] @offset 0x%" PRIx64, + req->op, req->res_id, req->va, req->va + req->vm_map_size - 1, req->offset); } return 0; @@ -646,7 +654,7 @@ amdgpu_ccmd_set_metadata(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) metadata.size_metadata = req->size_metadata; if (req->size_metadata) { if (req->size_metadata > sizeof(metadata.umd_metadata)) { - print(0, "Metadata size is too large for target buffer: %" PRIu32 " > %zu", + print(0, "Metadata size is too large for target buffer: %u > %zu", req->size_metadata, sizeof(metadata.umd_metadata)); rsp->ret = -EINVAL; return -1; @@ -655,7 +663,7 @@ amdgpu_ccmd_set_metadata(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) offsetof(struct amdgpu_ccmd_set_metadata_req, umd_metadata)); if (requested_size > hdr->len) { - print(0, "Metadata size is too large for source buffer: %zu > %" PRIu32, + print(0, "Metadata size is too large for source buffer: %zu > %u", requested_size, hdr->len); rsp->ret = -EINVAL; return -1; @@ -736,7 +744,7 @@ amdgpu_ccmd_create_ctx(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) } if (req->flags & ~AMDGPU_CCMD_CREATE_CTX_DESTROY) { - print(0, "Invalid flags 0x%" PRIu32, req->flags); + print(0, "Invalid flags 0x%u", req->flags); rsp->hdr.ret = -EINVAL; return -1; } @@ -827,7 +835,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) } /* Do not allocate arbitrarily large buffer. */ if (req->num_chunks > AMDGPU_CCMD_CS_SUBMIT_MAX_NUM_CHUNKS) { - print(1, "%s: Invalid num_chunks: %" PRIu32 " > %d", + print(1, "%s: Invalid num_chunks: %u > %d", __FUNCTION__, req->num_chunks, AMDGPU_CCMD_CS_SUBMIT_MAX_NUM_CHUNKS); rsp->ret = -EINVAL; return -1; @@ -842,7 +850,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) chunks = malloc((req->num_chunks + 1 /* syncobj_in */ + 1 /* syncobj_out */) * sizeof(*chunks)); if (chunks == NULL) { - print(0, "Failed to allocate %" PRIu32 " chunks", req->num_chunks + 2); + print(0, "Failed to allocate %u chunks", req->num_chunks + 2); r = -EINVAL; goto end; } @@ -858,7 +866,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) size_t descriptors_len = size_add(offsetof(struct amdgpu_ccmd_cs_submit_req, payload), size_mul(req->num_chunks, sizeof(struct desc))); if (descriptors_len > hdr->len) { - print(0, "Descriptors are out of bounds: %zu + %zu * %" PRIu32 " > %" PRIu32, + print(0, "Descriptors are out of bounds: %zu + %zu * %u > %u", offsetof(struct amdgpu_ccmd_cs_submit_req, payload), sizeof(struct desc), req->num_chunks, hdr->len); r = -EINVAL; @@ -875,7 +883,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) chunks[num_chunks].chunk_id = chunk_id; /* Validate input. */ if (end > hdr->len) { - print(0, "Descriptors are out of bounds: %zu > %" PRIu32, end, hdr->len); + print(0, "Descriptors are out of bounds: %zu > %u", end, hdr->len); r = -EINVAL; goto end; } @@ -936,7 +944,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) } in = input; if (in->offset % sizeof(uint64_t)) { - print(0, "Invalid chunk offset %" PRIu32 " (not multiple of 8)", in->offset); + print(0, "Invalid chunk offset %u (not multiple of 8)", in->offset); r = -EINVAL; goto end; } @@ -1043,7 +1051,7 @@ amdgpu_ccmd_cs_submit(struct drm_context *dctx, struct vdrm_ccmd_req *hdr) drmSyncobjDestroy(amdgpu_device_get_fd(ctx->dev), syncobj_out.handle); - print(3, "ctx: %d -> seqno={v=%d a=%ld} r=%d", req->ctx_id, hdr->seqno, seqno, r); + print(3, "ctx: %d -> seqno={v=%d a=%" PRIu64 "} r=%d", req->ctx_id, hdr->seqno, seqno, r); end: if (bo_list) @@ -1158,7 +1166,7 @@ amdgpu_renderer_submit_fence(struct virgl_context *vctx, uint32_t flags, /* timeline is ring_idx-1 (because ring_idx 0 is host CPU timeline) */ if (ring_idx > AMDGPU_HW_IP_NUM) { - print(0, "invalid ring_idx: %" PRIu32, ring_idx); + print(0, "invalid ring_idx: %u", ring_idx); return -EINVAL; } /* ring_idx zero is used for the guest to synchronize with host CPU, @@ -1170,7 +1178,7 @@ amdgpu_renderer_submit_fence(struct virgl_context *vctx, uint32_t flags, return 0; } - print(3, "ring_idx: %d fence_id: %lu", ring_idx, fence_id); + print(3, "ring_idx: %d fence_id: %" PRIu64, ring_idx, fence_id); return drm_timeline_submit_fence(&ctx->timelines[ring_idx - 1], flags, fence_id); } diff --git a/src/drm/drm_context.c b/src/drm/drm_context.c index d76fec74..570deea2 100644 --- a/src/drm/drm_context.c +++ b/src/drm/drm_context.c @@ -52,13 +52,28 @@ drm_context_unmap_shmem_blob(struct drm_context *dctx) if (!dctx->shmem) return; - uint32_t blob_size = dctx->rsp_mem_sz + dctx->shmem->rsp_mem_offset; - - munmap(dctx->shmem, blob_size); + munmap(dctx->shmem, dctx->blob_size); dctx->shmem = NULL; dctx->rsp_mem = NULL; dctx->rsp_mem_sz = 0; + dctx->blob_size = 0; +} + +static bool +drm_check_shm_bounds(struct drm_context *dctx, const struct vdrm_ccmd_req *hdr, + size_t len) +{ + size_t rsp_mem_sz = dctx->rsp_mem_sz; + size_t off = hdr->rsp_off; + + if ((off > rsp_mem_sz) || (len > rsp_mem_sz - off)) { + drm_err("invalid shm offset: off=%zu, len=%zu (shmem_size=%zu)", + off, len, rsp_mem_sz); + return false; + } + + return true; } static int @@ -108,24 +123,36 @@ drm_context_submit_cmd_dispatch(struct drm_context *dctx, const struct vdrm_ccmd if (ret) { drm_err("%s: dispatch failed: %d (%s)", ccmd->name, ret, strerror(errno)); - return ret; + goto free_response_buffer; } /* Commands with no response, like SET_DEBUGINFO, could be sent before * the shmem buffer is allocated. */ if (!dctx->shmem) - return 0; + goto free_response_buffer; - /* If the response length from the guest is smaller than the - * expected size, ie. newer host and older guest, then a shadow - * copy is used, and we need to copy back to the actual rsp - * buffer. - */ - struct vdrm_ccmd_rsp *rsp = (struct vdrm_ccmd_rsp *)&dctx->rsp_mem[hdr->rsp_off]; if (dctx->current_rsp) { - uint32_t len = *(volatile uint32_t *)&rsp->len; - len = MIN2(len, dctx->current_rsp->len); + uint32_t len = dctx->current_rsp->len; + + /* To prevent TOCTOU attacks, a shadow buffer is always used. + * We need to copy back to the actual rsp buffer. Add a bounds + * check for defense in depth. + */ + if (!drm_check_shm_bounds(dctx, hdr, len)) { + assert(!"Failed bounds check when copying to response buffer"); + ret = -EFAULT; + goto free_response_buffer; + } + + struct vdrm_ccmd_rsp *rsp = (struct vdrm_ccmd_rsp *)&dctx->rsp_mem[hdr->rsp_off]; + uint32_t untrusted_len = *(volatile uint32_t *)&rsp->len; + + /* WARNING: MIN2() evaluates its arguments more than once! + * DO NOT inline the above volatile load into it! That would + * introduce a double fetch vulnerability. + */ + len = MIN2(len, untrusted_len); memcpy(rsp, dctx->current_rsp, len); rsp->len = len; free(dctx->current_rsp); @@ -135,6 +162,16 @@ drm_context_submit_cmd_dispatch(struct drm_context *dctx, const struct vdrm_ccmd p_atomic_xchg(&dctx->shmem->seqno, hdr->seqno); return 0; + +free_response_buffer: + /* Free the response buffer. This both prevents leaks + * and (more importantly) ensures that if a response + * is about to be copied to shared memory, its offset + * was validated by the *current* command's handler. + */ + free(dctx->current_rsp); + dctx->current_rsp = NULL; + return ret; } static int @@ -314,14 +351,8 @@ void * drm_context_rsp(struct drm_context *dctx, const struct vdrm_ccmd_req *hdr, size_t len) { - size_t rsp_mem_sz = dctx->rsp_mem_sz; - size_t off = hdr->rsp_off; - - if ((off > rsp_mem_sz) || (len > rsp_mem_sz - off)) { - drm_err("invalid shm offset: off=%zu, len=%zu (shmem_size=%zu)", - off, len, rsp_mem_sz); + if (!drm_check_shm_bounds(dctx, hdr, len)) return NULL; - } /* The shared buffer might be writable by the guest. To avoid TOCTOU, * data races, and other security problems, always allocate a shadow buffer. @@ -357,12 +388,12 @@ drm_context_get_shmem_blob(struct drm_context *dctx, return -EINVAL; } - if (dctx->shmem) { + if (dctx->shmem || dctx->blob_size) { drm_err("there can be only one!"); return -EINVAL; } - if ((blob_size < sizeof(*dctx->shmem)) || (blob_size > UINT32_MAX)) { + if ((blob_size < shmem_size) || (blob_size > UINT32_MAX)) { drm_err("invalid blob size 0x%" PRIx64, blob_size); return -EINVAL; } @@ -393,8 +424,9 @@ drm_context_get_shmem_blob(struct drm_context *dctx, dctx->shmem->rsp_mem_offset = shmem_size; uint8_t *ptr = (uint8_t *)dctx->shmem; - dctx->rsp_mem = &ptr[dctx->shmem->rsp_mem_offset]; - dctx->rsp_mem_sz = blob_size - dctx->shmem->rsp_mem_offset; + dctx->rsp_mem = ptr + shmem_size; + dctx->rsp_mem_sz = blob_size - shmem_size; + dctx->blob_size = blob_size; blob->u.fd = fd; blob->type = VIRGL_RESOURCE_FD_SHM; diff --git a/src/drm/drm_context.h b/src/drm/drm_context.h index 3c04b762..5c75553f 100644 --- a/src/drm/drm_context.h +++ b/src/drm/drm_context.h @@ -43,6 +43,7 @@ struct drm_context { struct vdrm_shmem *shmem; uint8_t *rsp_mem; uint32_t rsp_mem_sz; + uint32_t blob_size; struct vdrm_ccmd_rsp *current_rsp; diff --git a/src/mesa/util/anon_file.c b/src/mesa/util/anon_file.c index 9963dd00..1c22514f 100644 --- a/src/mesa/util/anon_file.c +++ b/src/mesa/util/anon_file.c @@ -135,8 +135,11 @@ os_create_anonymous_file(off_t size, const char *debug_name) char shm_name[64]; snprintf(shm_name, sizeof(shm_name), "/%s-%d-%x-%x", tag, getpid(), nonce, i); - fd = shm_open(shm_name, O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, 0600); + /* macOS shm_open() rejects O_CLOEXEC with EINVAL; set close-on-exec + * explicitly afterwards instead. */ + fd = shm_open(shm_name, O_CREAT | O_EXCL | O_RDWR, 0600); if (fd >= 0) { + fcntl(fd, F_SETFD, FD_CLOEXEC); shm_unlink(shm_name); break; } diff --git a/src/meson.build b/src/meson.build index 83eb6173..6207a743 100644 --- a/src/meson.build +++ b/src/meson.build @@ -154,6 +154,18 @@ proxy_sources = [ 'proxy/proxy_socket.c', ] +if with_render_server == 'thread' + proxy_sources += [ + '../server/render_client.c', + '../server/render_common.c', + '../server/render_context.c', + '../server/render_server.c', + '../server/render_socket.c', + '../server/render_state.c', + '../server/render_worker.c', + ] +endif + video_sources = [ 'vrend/virgl_video.c', 'vrend/vrend_video.c', @@ -230,7 +242,7 @@ if with_drm_i915 virgl_sources += drm_i915_sources endif -if with_render_server +if with_render_server != '' virgl_sources += proxy_sources endif diff --git a/src/proxy/proxy_renderer.c b/src/proxy/proxy_renderer.c index 59c2d200..b1232909 100644 --- a/src/proxy/proxy_renderer.c +++ b/src/proxy/proxy_renderer.c @@ -39,12 +39,12 @@ proxy_renderer_init(const struct proxy_renderer_cbs *cbs, uint32_t flags) void proxy_renderer_fini(void) { - if (proxy_renderer.server) - proxy_server_destroy(proxy_renderer.server); - if (proxy_renderer.client) proxy_client_destroy(proxy_renderer.client); + if (proxy_renderer.server) + proxy_server_destroy(proxy_renderer.server); + memset(&proxy_renderer, 0, sizeof(struct proxy_renderer)); } diff --git a/src/proxy/proxy_server.c b/src/proxy/proxy_server.c index c643f74d..0f24c365 100644 --- a/src/proxy/proxy_server.c +++ b/src/proxy/proxy_server.c @@ -11,6 +11,11 @@ #include "server/render_protocol.h" +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER +#include "server/render_context.h" +#include "server/render_server.h" +#endif + int proxy_server_connect(struct proxy_server *srv) { @@ -33,9 +38,14 @@ proxy_server_destroy(struct proxy_server *srv) if (srv->client_fd >= 0) close(srv->client_fd); +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER + thrd_join(srv->thread, NULL); +#endif + free(srv); } +#ifndef ENABLE_SAME_PROCESS_RENDER_SERVER static bool proxy_server_fork(struct proxy_server *srv) { @@ -95,6 +105,55 @@ proxy_server_init_fd(struct proxy_server *srv) return true; } +#else /* ENABLE_SAME_PROCESS_RENDER_SERVER */ + +static int +proxy_server_start_thread(void *args) +{ + int remote_fd = (int)(uintptr_t)args; + char fd_str[16]; + snprintf(fd_str, sizeof(fd_str), "%d", remote_fd); + char *argv[] = { + RENDER_SERVER_EXEC_PATH, + "--socket-fd", + fd_str, + NULL, + }; + struct render_context_args ctx_args; + + /* must reset the global getopt index */ + optind = 1; + + bool ok = render_server_main(ARRAY_SIZE(argv) - 1, argv, &ctx_args); + + return ok ? 0 : -1; +} + +static bool +proxy_server_init_thread(struct proxy_server *srv) +{ + int socket_fds[2]; + + if (!proxy_socket_pair(socket_fds)) + return false; + + const int client_fd = socket_fds[0]; + const int remote_fd = socket_fds[1]; + + bool ok = thrd_create(&srv->thread, proxy_server_start_thread, + (void *)(uintptr_t)remote_fd) == thrd_success; + + if (ok) { + srv->client_fd = client_fd; + } else { + close(client_fd); + close(remote_fd); + } + + return ok; +} +#endif /* ENABLE_SAME_PROCESS_RENDER_SERVER */ + struct proxy_server * proxy_server_create(void) { @@ -104,6 +163,12 @@ proxy_server_create(void) srv->pid = -1; +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER + if (!proxy_server_init_thread(srv)) { + free(srv); + return NULL; + } +#else if (!proxy_server_init_fd(srv)) { /* start the render server on demand when the client does not provide a * server fd @@ -113,6 +178,7 @@ proxy_server_create(void) return NULL; } } +#endif if (!proxy_socket_is_valid(srv->client_fd)) { proxy_log("invalid client fd type"); diff --git a/src/proxy/proxy_server.h b/src/proxy/proxy_server.h index 1b5ca02b..63b3a439 100644 --- a/src/proxy/proxy_server.h +++ b/src/proxy/proxy_server.h @@ -10,9 +10,16 @@ #include +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER +#include "c11/threads.h" +#endif + struct proxy_server { pid_t pid; int client_fd; +#ifdef ENABLE_SAME_PROCESS_RENDER_SERVER + thrd_t thread; +#endif }; struct proxy_server * diff --git a/src/venus/vkr_common.h b/src/venus/vkr_common.h index 7a0df1d1..f484bad8 100644 --- a/src/venus/vkr_common.h +++ b/src/venus/vkr_common.h @@ -29,6 +29,7 @@ #include "util/u_pointer.h" #include "util/u_thread.h" #include "venus-protocol/vulkan.h" +#include "venus-protocol/vulkan_metal.h" #include "virgl_context.h" #include "virgl_util.h" #include "virglrenderer.h" diff --git a/src/venus/vkr_device_memory.c b/src/venus/vkr_device_memory.c index 97338b63..57a108d6 100644 --- a/src/venus/vkr_device_memory.c +++ b/src/venus/vkr_device_memory.c @@ -297,34 +297,10 @@ vkr_dispatch_vkAllocateMemory(struct vn_dispatch_context *dispatch, uint32_t valid_fd_types = 0; int udmabuf_fd = -1; void *gbm_bo = NULL; - VkExportMemoryAllocateInfo local_export_info; - - /* macOS: use shared memory + Metal buffer for HOST_VISIBLE cross-process sharing. */ struct vkr_mtl_shm *mtl_shm = NULL; -#ifdef __APPLE__ - VkImportMemoryMetalHandleInfoEXT local_metal_import = { - .sType = VK_STRUCTURE_TYPE_IMPORT_MEMORY_METAL_HANDLE_INFO_EXT, - }; - - if ((property_flags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) && - physical_dev->EXT_external_memory_metal && !res_info) { - assert(!res_info); - mtl_shm = vkr_mtl_shm_alloc(dev->mtl_device, alloc_info->allocationSize); - if (!mtl_shm) { - args->ret = VK_ERROR_OUT_OF_HOST_MEMORY; - return; - } - - /* Chain Metal import into alloc_info */ - local_metal_import.pNext = alloc_info->pNext; - local_metal_import.handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT; - local_metal_import.handle = mtl_shm->mtl_buffer; - alloc_info->pNext = &local_metal_import; - alloc_info->allocationSize = mtl_shm->shm_size; + VkExportMemoryAllocateInfo local_export_info; + VkImportMemoryMetalHandleInfoEXT local_metal_import; - valid_fd_types = 1 << VIRGL_RESOURCE_FD_SHM; - } else -#endif /* __APPLE__ */ if ((property_flags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) && !res_info) { /* An implementation can support dma_buf import along with opaque fd export/import. * If the client driver is using external memory and requesting dma_buf, without @@ -373,6 +349,23 @@ vkr_dispatch_vkAllocateMemory(struct vn_dispatch_context *dispatch, align(alloc_info->allocationSize, getpagesize()); } } + } else if (physical_dev->EXT_external_memory_metal) { + /* Allocate shm and wrap as a MTLBuffer for import. */ + mtl_shm = vkr_mtl_shm_alloc(dev->mtl_device, alloc_info->allocationSize); + if (!mtl_shm) { + args->ret = VK_ERROR_OUT_OF_HOST_MEMORY; + return; + } + + local_metal_import = (VkImportMemoryMetalHandleInfoEXT){ + .sType = VK_STRUCTURE_TYPE_IMPORT_MEMORY_METAL_HANDLE_INFO_EXT, + .pNext = alloc_info->pNext, + .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT, + .handle = mtl_shm->mtl_buffer, + }; + alloc_info->pNext = &local_metal_import; + alloc_info->allocationSize = mtl_shm->shm_size; + valid_fd_types = 1 << VIRGL_RESOURCE_FD_SHM; } else if (physical_dev->EXT_external_memory_dma_buf) { /* Allocate dma_buf externally and force to import. */ if (export_info) { diff --git a/src/venus/vkr_instance.c b/src/venus/vkr_instance.c index 27bf2dca..56173b95 100644 --- a/src/venus/vkr_instance.c +++ b/src/venus/vkr_instance.c @@ -183,6 +183,17 @@ vkr_dispatch_vkCreateInstance(struct vn_dispatch_context *dispatch, create_info->flags |= VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR; ext_names[ext_count++] = VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME; } + + /* The Metal shared-memory allocator exports the underlying MTLDevice via + * vkExportMetalObjectsEXT (see vkr_metal_get_device). VK_EXT_metal_objects + * requires the export intent to be declared at instance creation, otherwise + * the export violates VUID-VkExportMetalObjectsInfoEXT-pNext-06791. */ + VkExportMetalObjectCreateInfoEXT export_metal_device = { + .sType = VK_STRUCTURE_TYPE_EXPORT_METAL_OBJECT_CREATE_INFO_EXT, + .pNext = create_info->pNext, + .exportObjectType = VK_EXPORT_METAL_OBJECT_TYPE_METAL_DEVICE_BIT_EXT, + }; + create_info->pNext = &export_metal_device; #endif /* __APPLE__ */ assert(layer_count <= ARRAY_SIZE(layer_names)); diff --git a/src/venus/vkr_renderer.c b/src/venus/vkr_renderer.c index c43a96e6..c0335ca5 100644 --- a/src/venus/vkr_renderer.c +++ b/src/venus/vkr_renderer.c @@ -69,7 +69,9 @@ vkr_renderer_init(uint32_t flags, const struct vkr_renderer_callbacks *cbs) return false; vkr_debug_init(); - virgl_log_set_handler(cbs->debug_logger, NULL, NULL); + + if (cbs->debug_logger) + virgl_log_set_handler(cbs->debug_logger, NULL, NULL); vkr_state.cbs = cbs; list_inithead(&vkr_state.contexts); diff --git a/src/virgl_util.c b/src/virgl_util.c index 81846503..8988bb76 100644 --- a/src/virgl_util.c +++ b/src/virgl_util.c @@ -257,10 +257,15 @@ void virgl_prefixed_logv(const char *domain, va_list va) { char *prefixed_fmt = NULL; + char line_end = '\0'; assert(strchr(domain,'%') == NULL); - if (asprintf(&prefixed_fmt, "%s: %s", domain, fmt) < 0) + /* add new line if needed */ + if (fmt[strlen(fmt)-1] != '\n') + line_end = '\n'; + + if (asprintf(&prefixed_fmt, "%s: %s%c", domain, fmt, line_end) < 0) return; virgl_logv(log_level, prefixed_fmt, va); diff --git a/src/virglrenderer.c b/src/virglrenderer.c index 2e64d82e..fe36775a 100644 --- a/src/virglrenderer.c +++ b/src/virglrenderer.c @@ -806,7 +806,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (state.client_initialized && (state.cookie != cookie || state.flags != flags || state.cbs != cbs)) { - virgl_error("renderer already initialized"); + virgl_error("renderer already initialized\n"); return -EBUSY; } @@ -814,7 +814,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (!cbs || cbs->version < 1 || cbs->version > VIRGL_RENDERER_CALLBACKS_VERSION) { - virgl_error("invalid renderer callbacks"); + virgl_error("invalid renderer callbacks\n"); return -1; } @@ -831,7 +831,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks ret = virgl_resource_table_init(pipe_cbs); if (ret) { - virgl_error("failed to initialize virgl resources"); + virgl_error("failed to initialize virgl resources\n"); goto fail; } state.resource_initialized = true; @@ -840,7 +840,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (!state.context_initialized) { ret = virgl_context_table_init(); if (ret) { - virgl_error("failed to initialize virgl context"); + virgl_error("failed to initialize virgl context\n"); goto fail; } state.context_initialized = true; @@ -859,7 +859,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (ret) { if (drm_fd >= 0) close(drm_fd); - virgl_error("failed to initialize vrend winsys"); + virgl_error("failed to initialize vrend winsys\n"); goto fail; } state.winsys_initialized = true; @@ -871,7 +871,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (!cbs->create_gl_context || !cbs->destroy_gl_context || !cbs->make_current) { - virgl_error("invalid renderer gl callbacks"); + virgl_error("invalid renderer gl callbacks\n"); ret = EINVAL; goto fail; } @@ -879,14 +879,14 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks egl_display = state.cbs->get_egl_display(cookie); if (!egl_display) { - virgl_error("failed to get egl display"); + virgl_error("failed to get egl display\n"); ret = -1; goto fail; } ret = vrend_winsys_init_external(egl_display); if (ret) { - virgl_error("failed to initialize vrend winsys"); + virgl_error("failed to initialize vrend winsys\n"); ret = -1; goto fail; } @@ -898,7 +898,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks uint32_t renderer_flags = 0; if (!cookie || !cbs) { - virgl_error("invalid renderer vrend callbacks"); + virgl_error("invalid renderer vrend callbacks\n"); ret = -1; goto fail; } @@ -922,7 +922,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks ret = vrend_renderer_init(&vrend_cbs, renderer_flags); if (ret) { - virgl_error("failed to initialize vrend renderer"); + virgl_error("failed to initialize vrend renderer\n"); goto fail; } state.vrend_initialized = true; @@ -931,7 +931,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (!state.proxy_initialized && (flags & VIRGL_RENDERER_RENDER_SERVER)) { ret = proxy_renderer_init(&proxy_cbs, flags | VIRGL_RENDERER_NO_VIRGL); if (ret) { - virgl_error("failed to initialize venus renderer"); + virgl_error("failed to initialize venus renderer\n"); goto fail; } state.proxy_initialized = true; @@ -946,7 +946,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks ret = drm_renderer_init(drm_fd); if (ret) { - virgl_error("failed to initialize drm renderer"); + virgl_error("failed to initialize drm renderer\n"); goto fail; } state.drm_initialized = true; @@ -955,7 +955,7 @@ int virgl_renderer_init(void *cookie, int flags, struct virgl_renderer_callbacks if (!state.fence_initialized) { ret = virgl_fence_table_init(); if (ret) { - virgl_error("failed to initialize fence table"); + virgl_error("failed to initialize fence table\n"); goto fail; } state.fence_initialized = true; diff --git a/src/vrend/vrend_blitter.c b/src/vrend/vrend_blitter.c index f8291153..eed56e4d 100644 --- a/src/vrend/vrend_blitter.c +++ b/src/vrend/vrend_blitter.c @@ -477,7 +477,7 @@ static void vrend_renderer_init_blit_ctx(struct vrend_blitter_ctx *blit_ctx) } if (!blit_ctx->gl_context) { - virgl_error("virglrenderer: Unable to create blit context"); + virgl_error("virglrenderer: Unable to create blit context\n"); abort(); } diff --git a/src/vrend/vrend_renderer.c b/src/vrend/vrend_renderer.c index 94c75b10..6c2d779a 100644 --- a/src/vrend/vrend_renderer.c +++ b/src/vrend/vrend_renderer.c @@ -9095,6 +9095,10 @@ static inline bool resource_contains_box(struct vrend_resource *res, box->x > width || box->y > height || box->z > depth)) return false; + /* Check box size sanity */ + if (unlikely(box->width < 0 || box->height < 0 || box->depth < 0)) + return false; + end_x = (int64_t) box->x + (int64_t) box->width; end_y = (int64_t) box->y + (int64_t) box->height; end_z = (int64_t) box->z + (int64_t) box->depth; @@ -9675,7 +9679,7 @@ static int vrend_transfer_send_getteximage(struct vrend_resource *res, return 0; } -static void do_readpixels(struct vrend_resource *res, +static bool do_readpixels(struct vrend_resource *res, int idx, uint32_t level, uint32_t layer, GLint x, GLint y, GLsizei width, GLsizei height, @@ -9683,6 +9687,7 @@ static void do_readpixels(struct vrend_resource *res, GLsizei bufSize, void *data) { GLuint fb_id; + GLenum err; glGenFramebuffers(1, &fb_id); glBindFramebuffer(GL_FRAMEBUFFER, fb_id); @@ -9731,7 +9736,15 @@ static void do_readpixels(struct vrend_resource *res, else glReadPixels(x, y, width, height, format, type, data); + err = glGetError(); glDeleteFramebuffers(1, &fb_id); + + if (err != GL_NO_ERROR) { + virgl_error("glReadPixels failed: GL error %d\n", err); + return false; + } + + return true; } static int vrend_transfer_send_readpixels(struct vrend_context *ctx, @@ -9742,7 +9755,7 @@ static int vrend_transfer_send_readpixels(struct vrend_context *ctx, char *myptr = (char*)iov[0].iov_base + info->offset; int need_temp = 0; char *data; - bool actually_invert, separate_invert = false; + bool actually_invert, separate_invert = false, read_pixels; GLenum format, type; GLint y1; uint64_t send_size = 0; @@ -9807,7 +9820,7 @@ static int vrend_transfer_send_readpixels(struct vrend_context *ctx, return EINVAL; } - data = malloc(send_size); + data = calloc(1, send_size); if (!data) { virgl_error("Memory allocation failed for %"PRIu64"\n", send_size); return ENOMEM; @@ -9857,8 +9870,26 @@ static int vrend_transfer_send_readpixels(struct vrend_context *ctx, } } - do_readpixels(res, 0, info->level, info->box->z, info->box->x, y1, - info->box->width, info->box->height, format, type, send_size, data); + read_pixels = do_readpixels(res, 0, info->level, info->box->z, info->box->x, y1, + info->box->width, info->box->height, format, type, + send_size, data); + + if (has_feature(feat_mesa_invert) && actually_invert) + glPixelStorei(GL_PACK_INVERT_MESA, 0); + if (!need_temp && row_stride) + glPixelStorei(GL_PACK_ROW_LENGTH, 0); + glPixelStorei(GL_PACK_ALIGNMENT, 4); + +#if UTIL_ARCH_BIG_ENDIAN + glPixelStorei(GL_PACK_SWAP_BYTES, 0); +#endif + + if (!read_pixels) { + if (need_temp) + free(data); + glBindFramebuffer(GL_FRAMEBUFFER, old_fbo); + return EINVAL; + } /* on GLES, texture-backed BGR* resources are always stored with RGB* internal format, but * the guest will expect to readback the data in BGRA format. @@ -9877,15 +9908,6 @@ static int vrend_transfer_send_readpixels(struct vrend_context *ctx, else vrend_scale_depth(data, send_size, depth_scale); } - if (has_feature(feat_mesa_invert) && actually_invert) - glPixelStorei(GL_PACK_INVERT_MESA, 0); - if (!need_temp && row_stride) - glPixelStorei(GL_PACK_ROW_LENGTH, 0); - glPixelStorei(GL_PACK_ALIGNMENT, 4); - -#if UTIL_ARCH_BIG_ENDIAN - glPixelStorei(GL_PACK_SWAP_BYTES, 0); -#endif if (need_temp) { write_transfer_data(&res->base, iov, num_iovs, data, @@ -12090,7 +12112,7 @@ static int get_glsl_version(void) version_str = glGetString(GL_SHADING_LANGUAGE_VERSION); if (!version_str) { - virgl_error("GL_SHADING_LANGUAGE_VERSION query failed with empty output."); + virgl_error("GL_SHADING_LANGUAGE_VERSION query failed with empty output.\n"); return -1; } @@ -12103,7 +12125,7 @@ static int get_glsl_version(void) } if (count != 2) { - virgl_error("GL_SHADING_LANGUAGE_VERSION query failed with unexpected version format."); + virgl_error("GL_SHADING_LANGUAGE_VERSION query failed with unexpected version format.\n"); return -1; } @@ -12973,7 +12995,11 @@ void *vrend_renderer_get_cursor_contents(struct pipe_resource *pres, glBindTexture(res->target, res->gl_id); glGetnTexImageARB(res->target, 0, format, type, size, data); } else if (vrend_state.use_gles) { - do_readpixels(res, 0, 0, 0, 0, 0, *width, *height, format, type, size, data); + if (!do_readpixels(res, 0, 0, 0, 0, 0, *width, *height, format, type, size, data)) { + free(data); + free(data2); + return NULL; + } } else { glBindTexture(res->target, res->gl_id); glGetTexImage(res->target, 0, format, type, data); diff --git a/src/vrend/vrend_shader.c b/src/vrend/vrend_shader.c index d4e815b6..4fe673c4 100644 --- a/src/vrend/vrend_shader.c +++ b/src/vrend/vrend_shader.c @@ -239,10 +239,10 @@ struct dump_ctx { uint32_t ssbo_array_base; uint32_t ssbo_atomic_array_base; uint32_t ssbo_integer_mask; - uint8_t ssbo_memory_qualifier[32]; + uint8_t ssbo_memory_qualifier[PIPE_MAX_SHADER_BUFFERS]; int32_t ssbo_last_binding; - struct vrend_shader_image images[32]; + struct vrend_shader_image images[PIPE_MAX_SHADER_IMAGES]; uint32_t images_used_mask; int32_t image_last_binding; @@ -3716,13 +3716,13 @@ static bool is_integer_memory(const struct dump_ctx *ctx, enum tgsi_file_type fi case TGSI_FILE_MEMORY: return ctx->integer_memory; default: - virgl_error("Invalid file type"); + virgl_error("Invalid file type: %d\n", file_type); } return false; } -static void set_image_qualifier(struct vrend_shader_image images[], +static bool set_image_qualifier(struct vrend_shader_image images[], uint32_t image_used_mask, const struct tgsi_full_instruction *inst, uint32_t reg_index, bool indirect) @@ -3731,12 +3731,15 @@ static void set_image_qualifier(struct vrend_shader_image images[], if (indirect) { while (image_used_mask) images[u_bit_scan(&image_used_mask)].coherent = true; - } else + } else if (reg_index < PIPE_MAX_SHADER_IMAGES) { images[reg_index].coherent = true; + } else + return false; } + return true; } -static void set_memory_qualifier(uint8_t ssbo_memory_qualifier[], +static bool set_memory_qualifier(uint8_t ssbo_memory_qualifier[], uint32_t ssbo_used_mask, const struct tgsi_full_instruction *inst, uint32_t reg_index, bool indirect) @@ -3745,9 +3748,12 @@ static void set_memory_qualifier(uint8_t ssbo_memory_qualifier[], if (indirect) { while (ssbo_used_mask) ssbo_memory_qualifier[u_bit_scan(&ssbo_used_mask)] = TGSI_MEMORY_COHERENT; - } else + } else if (reg_index < PIPE_MAX_SHADER_BUFFERS) { ssbo_memory_qualifier[reg_index] = TGSI_MEMORY_COHERENT; + } else + return false; } + return true; } static void emit_store_mem(struct vrend_glsl_strbufs *glsl_strbufs, const char *dst, int writemask, @@ -3801,11 +3807,19 @@ translate_store(const struct dump_ctx *ctx, } if (dst_reg->Register.File == TGSI_FILE_IMAGE) { + if (dinfo->dest_index >= PIPE_MAX_SHADER_IMAGES) { + set_buf_error(glsl_strbufs); + return; + } + /* bail out if we want to write to a non-existing image */ if (!((1 << dinfo->dest_index) & ctx->images_used_mask)) return; - set_image_qualifier(images, ctx->images_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect); + if (!set_image_qualifier(images, ctx->images_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect)) { + set_buf_error(glsl_strbufs); + return; + } bool is_ms = false; enum vrend_type_qualifier coord_prefix = get_coord_prefix(ctx->images[dst_reg->Register.Index].decl.Resource, &is_ms, ctx->cfg->use_gles); @@ -3852,8 +3866,17 @@ translate_store(const struct dump_ctx *ctx, } else if (dst_reg->Register.File == TGSI_FILE_BUFFER || dst_reg->Register.File == TGSI_FILE_MEMORY) { enum vrend_type_qualifier dtypeprefix; - set_memory_qualifier(ssbo_memory_qualifier, ctx->ssbo_used_mask, inst, dst_reg->Register.Index, - dst_reg->Register.Indirect); + + if (dinfo->dest_index >= PIPE_MAX_SHADER_BUFFERS) { + set_buf_error(glsl_strbufs); + return; + } + + if (!set_memory_qualifier(ssbo_memory_qualifier, ctx->ssbo_used_mask, inst, dst_reg->Register.Index, + dst_reg->Register.Indirect)) { + set_buf_error(glsl_strbufs); + return; + } dtypeprefix = is_integer_memory(ctx, dst_reg->Register.File, dst_reg->Register.Index) ? FLOAT_BITS_TO_INT : FLOAT_BITS_TO_UINT; const char *conversion = sinfo->override_no_cast[1] ? "" : get_string(dtypeprefix); @@ -3913,13 +3936,17 @@ translate_load(const struct dump_ctx *ctx, if (src->Register.File == TGSI_FILE_IMAGE) { /* Bail out if we want to load from an image that is not actually used */ - if (sinfo->sreg_index < 0) { + if (sinfo->sreg_index < 0 || sinfo->sreg_index > PIPE_MAX_SHADER_IMAGES) { return false; } + if (!((1 << sinfo->sreg_index) & ctx->images_used_mask)) return false; - set_image_qualifier(images, ctx->images_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect); + if (!set_image_qualifier(images, ctx->images_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect)) { + set_buf_error(glsl_strbufs); + return false; + } bool is_ms = false; @@ -3985,7 +4012,10 @@ translate_load(const struct dump_ctx *ctx, char mydst[255], atomic_op[9], atomic_src[10]; enum vrend_type_qualifier dtypeprefix; - set_memory_qualifier(ssbo_memory_qualifier, ctx->ssbo_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect); + if (!set_memory_qualifier(ssbo_memory_qualifier, ctx->ssbo_used_mask, inst, inst->Src[0].Register.Index, inst->Src[0].Register.Indirect)) { + set_buf_error(glsl_strbufs); + return false; + } const char *d = dst; char *md = mydst; @@ -4068,7 +4098,7 @@ static const char *get_atomic_opname(int tgsi_opcode, bool *is_cas) opname = "Max"; break; default: - virgl_error("Illegal atomic opcode"); + virgl_error("Illegal atomic opcode: %d\n", tgsi_opcode); return NULL; } return opname; @@ -4121,6 +4151,11 @@ translate_atomic(struct dump_ctx *ctx, char cas_str[128] = ""; if (src->Register.File == TGSI_FILE_IMAGE) { + if (sinfo->sreg_index < 0 || sinfo->sreg_index >= PIPE_MAX_SHADER_IMAGES) { + set_buf_error(&ctx->glsl_strbufs); + return; + } + enum tgsi_return_type itype; get_internalformat_string(ctx->images[sinfo->sreg_index].decl.Format, &itype); switch (itype) { @@ -4167,8 +4202,11 @@ translate_atomic(struct dump_ctx *ctx, snprintf(ms_str, 32, ", int(%s.w)", srcs[1]); } - set_image_qualifier(ctx->images, ctx->images_used_mask, inst, - inst->Src[0].Register.Index, inst->Src[0].Register.Indirect); + if (!set_image_qualifier(ctx->images, ctx->images_used_mask, inst, + inst->Src[0].Register.Index, inst->Src[0].Register.Indirect)) { + set_buf_error(&ctx->glsl_strbufs); + return; + } if (!ctx->cfg->use_gles || !inst->Src[0].Register.Indirect) { emit_buff(&ctx->glsl_strbufs, "%s = %s(imageAtomic%s(%s, %s(%s(%s))%s, %s(%s(%s))%s));\n", @@ -4200,6 +4238,11 @@ translate_atomic(struct dump_ctx *ctx, } if (src->Register.File == TGSI_FILE_BUFFER || src->Register.File == TGSI_FILE_MEMORY) { enum vrend_type_qualifier type; + if (src->Register.Index < 0 || src->Register.Index >= PIPE_MAX_SHADER_BUFFERS) { + set_buf_error(&ctx->glsl_strbufs); + return; + } + if ((is_integer_memory(ctx, src->Register.File, src->Register.Index))) { type = INT; dtypeprefix = INT_BITS_TO_FLOAT; diff --git a/src/vrend/vrend_winsys_egl.c b/src/vrend/vrend_winsys_egl.c index 4ddcd0df..0c99219e 100644 --- a/src/vrend/vrend_winsys_egl.c +++ b/src/vrend/vrend_winsys_egl.c @@ -431,7 +431,7 @@ struct virgl_egl *virgl_egl_init(EGLNativeDisplayType display_id, bool surfacele egl->signaled_fence = eglCreateSyncKHR(egl->egl_display, EGL_SYNC_NATIVE_FENCE_ANDROID, NULL); if (!egl->signaled_fence) { - virgl_error("Failed to create signaled fence"); + virgl_error("Failed to create signaled fence\n"); goto fail; }