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>
This commit is contained in:
Connor Abbott 2025-08-25 11:32:10 -04:00 committed by Marge Bot
parent 2cd564c1de
commit bef37336fb
2 changed files with 44 additions and 46 deletions

View file

@ -1271,36 +1271,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);
@ -1314,18 +1284,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

@ -396,14 +396,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);
@ -411,8 +403,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
@ -423,12 +415,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,
};
@ -443,8 +436,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
@ -455,12 +448,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,
};