vulkan/queue: Fix VkTimelineSemaphoreSubmitInfo sanitization

We're supposed to completely ignore VkTimelineSemaphoreSubmitInfo if
there aren't any timeline semaphores, including the array lengths, which
is made clear by the various VUs already cited by the code. The
vkQueueSubmit() path correctly handled this when asserting but still
dereferenced pWaitSemaphoreValues unconditionally, which could lead to
dereferencing an invalid pointer if waitSemaphoreValueCount is less than
waitSemaphoreCount. The vkQueueSparseBind() path didn't even assert
correctly. Bring vkQueueSparseBind() in line with vkQueueSubmit()
and make both only dereference the wait/signal array once we've
determined it must be present. While we're here, also fix the assert in
vkQueueSubmit() to disallow a waitSemaphoreValueCount of 0 if there are
timeline semaphores present, which conversely is not allowed.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36989>
(cherry picked from commit bef37336fb)
This commit is contained in:
Connor Abbott 2025-08-25 11:32:10 -04:00 committed by Eric Engestrom
parent 4fdc7e990d
commit a93b1275cc
3 changed files with 45 additions and 47 deletions

View file

@ -4724,7 +4724,7 @@
"description": "vulkan/queue: Fix VkTimelineSemaphoreSubmitInfo sanitization",
"nominated": false,
"nomination_type": 0,
"resolution": 4,
"resolution": 1,
"main_sha": null,
"because_sha": null,
"notes": null

View file

@ -1457,36 +1457,6 @@ vk_common_QueueBindSparse(VkQueue _queue,
for (uint32_t i = 0; i < bindInfoCount; i++) {
const VkTimelineSemaphoreSubmitInfo *timeline_info =
vk_find_struct_const(pBindInfo[i].pNext, TIMELINE_SEMAPHORE_SUBMIT_INFO);
const uint64_t *wait_values = NULL;
const uint64_t *signal_values = NULL;
if (timeline_info && timeline_info->waitSemaphoreValueCount) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkBindSparseInfo-pNext-03248
*
* "If the pNext chain of this structure includes a VkTimelineSemaphoreSubmitInfo structure
* and any element of pSignalSemaphores was created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_TIMELINE, then its signalSemaphoreValueCount member must equal
* signalSemaphoreCount"
*/
assert(timeline_info->waitSemaphoreValueCount == pBindInfo[i].waitSemaphoreCount);
wait_values = timeline_info->pWaitSemaphoreValues;
}
if (timeline_info && timeline_info->signalSemaphoreValueCount) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkBindSparseInfo-pNext-03247
*
* "If the pNext chain of this structure includes a VkTimelineSemaphoreSubmitInfo structure
* and any element of pWaitSemaphores was created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_TIMELINE, then its waitSemaphoreValueCount member must equal
* waitSemaphoreCount"
*/
assert(timeline_info->signalSemaphoreValueCount == pBindInfo[i].signalSemaphoreCount);
signal_values = timeline_info->pSignalSemaphoreValues;
}
STACK_ARRAY(VkSemaphoreSubmitInfo, wait_semaphore_infos,
pBindInfo[i].waitSemaphoreCount);
@ -1500,18 +1470,52 @@ vk_common_QueueBindSparse(VkQueue _queue,
}
for (uint32_t j = 0; j < pBindInfo[i].waitSemaphoreCount; j++) {
VK_FROM_HANDLE(vk_semaphore, semaphore, pBindInfo[i].pWaitSemaphores[j]);
uint64_t wait_value = 0;
if (timeline_info && semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkBindSparseInfo-pNext-03248
*
* "If the pNext chain of this structure includes a VkTimelineSemaphoreSubmitInfo structure
* and any element of pSignalSemaphores was created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_TIMELINE, then its signalSemaphoreValueCount member must equal
* signalSemaphoreCount"
*/
assert(timeline_info->waitSemaphoreValueCount == pBindInfo[i].waitSemaphoreCount);
wait_value = timeline_info->pWaitSemaphoreValues[j];
}
wait_semaphore_infos[j] = (VkSemaphoreSubmitInfo) {
.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SUBMIT_INFO,
.semaphore = pBindInfo[i].pWaitSemaphores[j],
.value = wait_values ? wait_values[j] : 0,
.value = wait_value,
};
}
for (uint32_t j = 0; j < pBindInfo[i].signalSemaphoreCount; j++) {
VK_FROM_HANDLE(vk_semaphore, semaphore, pBindInfo[i].pSignalSemaphores[j]);
uint64_t signal_value = 0;
if (timeline_info && semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkBindSparseInfo-pNext-03247
*
* "If the pNext chain of this structure includes a VkTimelineSemaphoreSubmitInfo structure
* and any element of pWaitSemaphores was created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_TIMELINE, then its waitSemaphoreValueCount member must equal
* waitSemaphoreCount"
*/
assert(timeline_info->signalSemaphoreValueCount == pBindInfo[i].signalSemaphoreCount);
signal_value = timeline_info->pSignalSemaphoreValues[j];
}
signal_semaphore_infos[j] = (VkSemaphoreSubmitInfo) {
.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SUBMIT_INFO,
.semaphore = pBindInfo[i].pSignalSemaphores[j],
.value = signal_values ? signal_values[j] : 0,
.value = signal_value,
};
}
struct vulkan_submit_info info = {

View file

@ -397,14 +397,6 @@ vk_common_QueueSubmit(
const VkTimelineSemaphoreSubmitInfo *timeline_info =
vk_find_struct_const(pSubmits[s].pNext,
TIMELINE_SEMAPHORE_SUBMIT_INFO);
const uint64_t *wait_values = NULL;
const uint64_t *signal_values = NULL;
if (timeline_info && timeline_info->waitSemaphoreValueCount)
wait_values = timeline_info->pWaitSemaphoreValues;
if (timeline_info && timeline_info->signalSemaphoreValueCount)
signal_values = timeline_info->pSignalSemaphoreValues;
const VkDeviceGroupSubmitInfo *group_info =
vk_find_struct_const(pSubmits[s].pNext, DEVICE_GROUP_SUBMIT_INFO);
@ -412,8 +404,8 @@ vk_common_QueueSubmit(
for (uint32_t i = 0; i < pSubmits[s].waitSemaphoreCount; i++) {
VK_FROM_HANDLE(vk_semaphore, semaphore, pSubmits[s].pWaitSemaphores[i]);
if (semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE &&
timeline_info && timeline_info->waitSemaphoreValueCount) {
uint64_t wait_value = 0;
if (semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE && timeline_info) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkSubmitInfo-pNext-03241
@ -424,12 +416,13 @@ vk_common_QueueSubmit(
* waitSemaphoreCount"
*/
assert(timeline_info->waitSemaphoreValueCount == pSubmits[s].waitSemaphoreCount);
wait_value = timeline_info->pWaitSemaphoreValues[i];
}
wait_semaphores[n_wait_semaphores + i] = (VkSemaphoreSubmitInfo) {
.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SUBMIT_INFO,
.semaphore = pSubmits[s].pWaitSemaphores[i],
.value = wait_values ? wait_values[i] : 0,
.value = wait_value,
.stageMask = pSubmits[s].pWaitDstStageMask[i],
.deviceIndex = group_info ? group_info->pWaitSemaphoreDeviceIndices[i] : 0,
};
@ -444,8 +437,8 @@ vk_common_QueueSubmit(
for (uint32_t i = 0; i < pSubmits[s].signalSemaphoreCount; i++) {
VK_FROM_HANDLE(vk_semaphore, semaphore, pSubmits[s].pSignalSemaphores[i]);
if (semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE &&
timeline_info && timeline_info->signalSemaphoreValueCount) {
uint64_t signal_value = 0;
if (semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE && timeline_info) {
/* From the Vulkan 1.3.204 spec:
*
* VUID-VkSubmitInfo-pNext-03240
@ -456,12 +449,13 @@ vk_common_QueueSubmit(
* signalSemaphoreCount"
*/
assert(timeline_info->signalSemaphoreValueCount == pSubmits[s].signalSemaphoreCount);
signal_value = timeline_info->pSignalSemaphoreValues[i];
}
signal_semaphores[n_signal_semaphores + i] = (VkSemaphoreSubmitInfo) {
.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SUBMIT_INFO,
.semaphore = pSubmits[s].pSignalSemaphores[i],
.value = signal_values ? signal_values[i] : 0,
.value = signal_value,
.stageMask = VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT,
.deviceIndex = group_info ? group_info->pSignalSemaphoreDeviceIndices[i] : 0,
};