From f54c4e85ce089964e4d2ed39157f07226a41d11f Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Mon, 25 Nov 2019 10:08:26 +1100 Subject: [PATCH] radv: create a fresh fork for each pipeline compile In order to prevent a potential malicious pipeline tainting our secure compile process and interfering with successive pipelines we want to create a fresh fork for each pipeline compile. Benchmarking has shown that simply forking on each pipeline creation doubles the total time it takes to compile a fossilize db collection. So instead here we fork the process at device creation so that we have a slim copy of the device and then fork this otherwise idle and untainted process each time we compile a pipeline. Forking this slim copy of the device results in only a 20% increase in compile time vs a 100% increase. Fixes: cff53da3 ("radv: enable secure compile support") --- src/amd/vulkan/radv_device.c | 137 +++++++++++++++++++++++++++++---- src/amd/vulkan/radv_pipeline.c | 16 +++- 2 files changed, 139 insertions(+), 14 deletions(-) diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index 20ebc2924de..a0f7267e199 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -2253,15 +2253,25 @@ open_fifo_exit: } static void run_secure_compile_device(struct radv_device *device, unsigned process, - int fd_secure_input, int fd_secure_output) + int fd_idle_device_output) { + int fd_secure_input; + int fd_secure_output; + bool fifo_result = secure_compile_open_fifo_fds(device->sc_state, + &fd_secure_input, + &fd_secure_output, + process, false); + enum radv_secure_compile_type sc_type; const int needed_fds[] = { fd_secure_input, fd_secure_output, + fd_idle_device_output, }; - if (!radv_close_all_fds(needed_fds, ARRAY_SIZE(needed_fds)) || install_seccomp_filter() == -1) { + + if (!fifo_result || !radv_close_all_fds(needed_fds, ARRAY_SIZE(needed_fds)) || + install_seccomp_filter() == -1) { sc_type = RADV_SC_TYPE_INIT_FAILURE; } else { sc_type = RADV_SC_TYPE_INIT_SUCCESS; @@ -2269,7 +2279,7 @@ static void run_secure_compile_device(struct radv_device *device, unsigned proce device->sc_state->secure_compile_processes[process].fd_secure_output = fd_secure_output; } - write(fd_secure_output, &sc_type, sizeof(sc_type)); + write(fd_idle_device_output, &sc_type, sizeof(sc_type)); if (sc_type == RADV_SC_TYPE_INIT_FAILURE) goto secure_compile_exit; @@ -2417,6 +2427,89 @@ static void run_secure_compile_device(struct radv_device *device, unsigned proce } } +secure_compile_exit: + close(fd_secure_input); + close(fd_secure_output); + close(fd_idle_device_output); + _exit(0); +} + +static enum radv_secure_compile_type fork_secure_compile_device(struct radv_device *device, unsigned process) +{ + int fd_secure_input[2]; + int fd_secure_output[2]; + + /* create pipe descriptors (used to communicate between processes) */ + if (pipe(fd_secure_input) == -1 || pipe(fd_secure_output) == -1) + return RADV_SC_TYPE_INIT_FAILURE; + + + int sc_pid; + if ((sc_pid = fork()) == 0) { + device->sc_state->secure_compile_thread_counter = process; + run_secure_compile_device(device, process, fd_secure_output[1]); + } else { + if (sc_pid == -1) + return RADV_SC_TYPE_INIT_FAILURE; + + /* Read the init result returned from the secure process */ + enum radv_secure_compile_type sc_type; + bool sc_read = radv_sc_read(fd_secure_output[0], &sc_type, sizeof(sc_type), true); + + if (sc_type == RADV_SC_TYPE_INIT_FAILURE || !sc_read) { + close(fd_secure_input[0]); + close(fd_secure_input[1]); + close(fd_secure_output[1]); + close(fd_secure_output[0]); + int status; + waitpid(sc_pid, &status, 0); + + return RADV_SC_TYPE_INIT_FAILURE; + } else { + assert(sc_type == RADV_SC_TYPE_INIT_SUCCESS); + write(device->sc_state->secure_compile_processes[process].fd_secure_output, &sc_type, sizeof(sc_type)); + + close(fd_secure_input[0]); + close(fd_secure_input[1]); + close(fd_secure_output[1]); + close(fd_secure_output[0]); + + int status; + waitpid(sc_pid, &status, 0); + } + } + + return RADV_SC_TYPE_INIT_SUCCESS; +} + +/* Run a bare bones fork of a device that was forked right after its creation. + * This device will have low overhead when it is forked again before each + * pipeline compilation. This device sits idle and its only job is to fork + * itself. + */ +static void run_secure_compile_idle_device(struct radv_device *device, unsigned process, + int fd_secure_input, int fd_secure_output) +{ + enum radv_secure_compile_type sc_type = RADV_SC_TYPE_INIT_SUCCESS; + device->sc_state->secure_compile_processes[process].fd_secure_input = fd_secure_input; + device->sc_state->secure_compile_processes[process].fd_secure_output = fd_secure_output; + + write(fd_secure_output, &sc_type, sizeof(sc_type)); + + while (true) { + radv_sc_read(fd_secure_input, &sc_type, sizeof(sc_type), false); + + if (sc_type == RADV_SC_TYPE_FORK_DEVICE) { + sc_type = fork_secure_compile_device(device, process); + + if (sc_type == RADV_SC_TYPE_INIT_FAILURE) + goto secure_compile_exit; + + } else if (sc_type == RADV_SC_TYPE_DESTROY_DEVICE) { + goto secure_compile_exit; + } + } + secure_compile_exit: close(fd_secure_input); close(fd_secure_output); @@ -2437,7 +2530,7 @@ static void destroy_secure_compile_device(struct radv_device *device, unsigned p waitpid(device->sc_state->secure_compile_processes[process].sc_pid, &status, 0); } -static VkResult fork_secure_compile_device(struct radv_device *device) +static VkResult fork_secure_compile_idle_device(struct radv_device *device) { device->sc_state = vk_zalloc(&device->alloc, sizeof(struct radv_secure_compile_state), @@ -2445,6 +2538,15 @@ static VkResult fork_secure_compile_device(struct radv_device *device) mtx_init(&device->sc_state->secure_compile_mutex, mtx_plain); + pid_t upid = getpid(); + time_t seconds = time(NULL); + + char *uid; + if (asprintf(&uid, "%ld_%ld", (long) upid, (long) seconds) == -1) + return VK_ERROR_INITIALIZATION_FAILED; + + device->sc_state->uid = uid; + uint8_t sc_threads = device->instance->num_sc_threads; int fd_secure_input[MAX_SC_PROCS][2]; int fd_secure_output[MAX_SC_PROCS][2]; @@ -2464,7 +2566,7 @@ static VkResult fork_secure_compile_device(struct radv_device *device) for (unsigned process = 0; process < sc_threads; process++) { if ((device->sc_state->secure_compile_processes[process].sc_pid = fork()) == 0) { device->sc_state->secure_compile_thread_counter = process; - run_secure_compile_device(device, process, fd_secure_input[process][0], fd_secure_output[process][1]); + run_secure_compile_idle_device(device, process, fd_secure_input[process][0], fd_secure_output[process][1]); } else { if (device->sc_state->secure_compile_processes[process].sc_pid == -1) return VK_ERROR_INITIALIZATION_FAILED; @@ -2473,7 +2575,18 @@ static VkResult fork_secure_compile_device(struct radv_device *device) enum radv_secure_compile_type sc_type; bool sc_read = radv_sc_read(fd_secure_output[process][0], &sc_type, sizeof(sc_type), true); - if (sc_type == RADV_SC_TYPE_INIT_FAILURE || !sc_read) { + bool fifo_result; + if (sc_read && sc_type == RADV_SC_TYPE_INIT_SUCCESS) { + fifo_result = secure_compile_open_fifo_fds(device->sc_state, + &device->sc_state->secure_compile_processes[process].fd_server, + &device->sc_state->secure_compile_processes[process].fd_client, + process, true); + + device->sc_state->secure_compile_processes[process].fd_secure_input = fd_secure_input[process][1]; + device->sc_state->secure_compile_processes[process].fd_secure_output = fd_secure_output[process][0]; + } + + if (sc_type == RADV_SC_TYPE_INIT_FAILURE || !sc_read || !fifo_result) { close(fd_secure_input[process][0]); close(fd_secure_input[process][1]); close(fd_secure_output[process][1]); @@ -2487,10 +2600,6 @@ static VkResult fork_secure_compile_device(struct radv_device *device) } return VK_ERROR_INITIALIZATION_FAILED; - } else { - assert(sc_type == RADV_SC_TYPE_INIT_SUCCESS); - device->sc_state->secure_compile_processes[process].fd_secure_input = fd_secure_input[process][1]; - device->sc_state->secure_compile_processes[process].fd_secure_output = fd_secure_output[process][0]; } } } @@ -2729,7 +2838,8 @@ VkResult radv_CreateDevice( /* Fork device for secure compile as required */ device->instance->num_sc_threads = sc_threads; if (radv_device_use_secure_compile(device->instance)) { - result = fork_secure_compile_device(device); + + result = fork_secure_compile_idle_device(device); if (result != VK_SUCCESS) goto fail_meta; } @@ -2793,15 +2903,16 @@ void radv_DestroyDevice( pthread_cond_destroy(&device->timeline_cond); radv_bo_list_finish(&device->bo_list); - if (radv_device_use_secure_compile(device->instance)) { for (unsigned i = 0; i < device->instance->num_sc_threads; i++ ) { destroy_secure_compile_device(device, i); } } - if (device->sc_state) + if (device->sc_state) { + free(device->sc_state->uid); vk_free(&device->alloc, device->sc_state->secure_compile_processes); + } vk_free(&device->alloc, device->sc_state); vk_free(&device->alloc, device); } diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c index b04b24a13c1..4c3cb17b9f2 100644 --- a/src/amd/vulkan/radv_pipeline.c +++ b/src/amd/vulkan/radv_pipeline.c @@ -4708,8 +4708,19 @@ radv_secure_compile(struct radv_pipeline *pipeline, int fd_secure_input = device->sc_state->secure_compile_processes[process].fd_secure_input; int fd_secure_output = device->sc_state->secure_compile_processes[process].fd_secure_output; + /* Fork a copy of the slim untainted secure compile process */ + enum radv_secure_compile_type sc_type = RADV_SC_TYPE_FORK_DEVICE; + write(fd_secure_input, &sc_type, sizeof(sc_type)); + + if (!radv_sc_read(fd_secure_output, &sc_type, sizeof(sc_type), true) || + sc_type != RADV_SC_TYPE_INIT_SUCCESS) + return VK_ERROR_DEVICE_LOST; + + fd_secure_input = device->sc_state->secure_compile_processes[process].fd_server; + fd_secure_output = device->sc_state->secure_compile_processes[process].fd_client; + /* Write pipeline / shader module out to secure process via pipe */ - enum radv_secure_compile_type sc_type = RADV_SC_TYPE_COMPILE_PIPELINE; + sc_type = RADV_SC_TYPE_COMPILE_PIPELINE; write(fd_secure_input, &sc_type, sizeof(sc_type)); /* Write pipeline layout out to secure process */ @@ -4818,6 +4829,9 @@ radv_secure_compile(struct radv_pipeline *pipeline, } } + sc_type = RADV_SC_TYPE_DESTROY_DEVICE; + write(fd_secure_input, &sc_type, sizeof(sc_type)); + mtx_lock(&device->sc_state->secure_compile_mutex); device->sc_state->secure_compile_thread_counter--; device->sc_state->secure_compile_processes[process].in_use = false; -- 2.30.2