Possible memory overlap on loader.c

[rodrigoffzz]

Hi, I'm going through your code as part of the Main Inclusion Review [1] process for Ubuntu [2], and noticed something on the loader code that is present since the deprecated loader repo [3].
It was warned by a Coverity execution on the deprecated repo but as this part of the code remained the same, I think it worth reporting so you can check if it is really a problem or not.

The possible problem is a memory overlap in terminator_CreateInstance() @ loader.c in memcpy(&icd_app_info, icd_create_info.pApplicationInfo, sizeof(icd_app_info)); as icd_create_info.pApplicationInfo can be assigned with icd_app_info address, and is inside a for loop (assign the same address and then overlap memory in memcpy)
A suggestion would be to replace memcpy with memmove for a safer copy without the risk of overlaping

Vulkan-Loader/loader/loader.c

Lines 5322 to 5330 in 0bcddf3

	if ((requested_version != 0) && (icd_version_nopatch == VK_API_VERSION_1_0)) {
	if (icd_create_info.pApplicationInfo == NULL) {
	memset(&icd_app_info, 0, sizeof(icd_app_info));
	} else {
	memcpy(&icd_app_info, icd_create_info.pApplicationInfo, sizeof(icd_app_info));
	}
	icd_app_info.apiVersion = icd_version;
	icd_create_info.pApplicationInfo = &icd_app_info;
	}

Do you think that this is something to be fixed?
Thanks!

[1] https://github.com/canonical/ubuntu-mir
[2] https://bugs.launchpad.net/ubuntu/+source/vulkan-tools/+bug/1946359
[3] https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers

[charles-lunarg]

Yep, this looks possible and is not a good thing to have happen. I created a PR for using memmove, assuming it passes CI I will merge it.