GPUOpen-Drivers/xgl

Correctness of DefaultReallocFunc

kuhar opened this issue · 5 comments

kuhar commented

I think the implementation of DefaultReallocFunc (

DefaultReallocFunc(
) may be running into two issues:

  1. Out of bounds accesses to the source buffer when size is more than the size of the original allocation. This is because it copies up to the size of the destination buffer, which will typically be more than the original buffer.
  2. Doesn't free when size is 0.

Vulkan specs: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/PFN_vkReallocationFunction.html.

kuhar commented

Since there isn't any aligned realloc primitive in POSIX, I think a good workaround would be to prepend a header to each allocation and remember the allocation size there.

That looks fairly broken. I'm not aware of a case where we use realloc but I'm guessing you've hit one.

kuhar commented

@JonCampbell407 Are you fine with moving forward with a fix along these lines?

I think a good workaround would be to prepend a header to each allocation and remember the allocation size there.

Doing that would add a small amount of overhead to every allocation. Since realloc is never used in the driver I'd rather not do that. I think we should just change DefaultReallocFunc() to assert with VK_NEVER_CALLED(). What are you doing to hit this? I can't think of a case where we would want to use realloc in the driver.

kuhar commented

Like you suggest, bailing out sound more lightweight that doing extra work on every allocation and free. I don't know if there would be any measurable slowdown caused by adding such header to every allocation.
I'll prepare a PR to return a nullptr and assert in reallocation to make sure things don't misbehave in the future..

I hit this when working on cache creator and testing corner cases related to possible error codes after my refactoring changes. The ICD doesn't use realloc but we have a vulkan layer test that does reallocation. I wanted to move default vulkan allocation function implementation to either PAL or the XGL cache support library and use them in the cache creator tool.