RHEL-183408 viostor: fix overflow in bump allocator bounds checks

[rachael-george]

Issue
VioStorPoolAlloc (and the related page bump allocator in virtio_pci.c) checked whether a request fit in the pre-allocated pool using:

(offset + size) <= totalSize

With 32-bit unsigned math, offset + size can wrap around when both values are large. The result can look small, so the check incorrectly passes and the code writes past the end of the buffer via RtlZeroMemory.

The code checked the raw request size but allocated a cache-line- or page-rounded size. A small request near the end of the pool could pass the check yet still overrun when the rounded size was applied.

Fix
Updated both bump allocators to use an overflow-safe bounds check:

• Round first — compute the actual allocation size (ROUND_TO_CACHE_LINES for the pool, ROUND_TO_PAGES for pages) before checking.

• Check fit safely — instead of offset + size, use offset > totalSize - alignedSize (with a prior check that alignedSize <= totalSize). This avoids addition wraparound and correctly answers “is there enough space left?”

• Reject invalid sizes — fail if rounding overflows (alignedSize < size) or the request does not fit.

[gemini-code-assist]

Code Review

This pull request improves the safety of memory allocation in mem_alloc_contiguous_pages and VioStorPoolAlloc by introducing robust overflow and bounds checks. Specifically, it ensures that the aligned size does not overflow, does not exceed the total page or pool size, and that the offset calculation remains within bounds. There are no review comments, and we have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[YanVugenfirer]

Please squash the commits (with git rebase -i)

[kevmw]

Your description of the problem above is good; please add a condensed version of it to the commit message. It would also be good to add a note that the problem is purely theoretical (I assume it is, not the least because the pool sizes are precalculated according to the required memory anyway?) and there is no reproducer.

The fix itself is huge for a rather trivial problem. Not sure why we need all those local variables that are only a copy of struct fields without ever being changed - was this about line lengths with the conditions?

I expected something much more minimal (and obvious) that touches only two lines for each instance, like this:

diff --git a/viostor/virtio_pci.c b/viostor/virtio_pci.c
index 52e52055..50114ab3 100644
--- a/viostor/virtio_pci.c
+++ b/viostor/virtio_pci.c
@@ -124,9 +124,9 @@ static void *mem_alloc_contiguous_pages(void *context, size_t size)
     PADAPTER_EXTENSION adaptExt = (PADAPTER_EXTENSION)context;
     PVOID ptr = (PVOID)((ULONG_PTR)adaptExt->pageAllocationVa + adaptExt->pageOffset);
 
-    if ((adaptExt->pageOffset + size) <= adaptExt->pageAllocationSize)
+    size = ROUND_TO_PAGES(size);
+    if (size && size <= adaptExt->pageAllocationSize - adaptExt->pageOffset)
     {
-        size = ROUND_TO_PAGES(size);
         adaptExt->pageOffset += size;
         RtlZeroMemory(ptr, size);
         return ptr;
diff --git a/viostor/virtio_stor.c b/viostor/virtio_stor.c
index f93d0916..7539c5ad 100644
--- a/viostor/virtio_stor.c
+++ b/viostor/virtio_stor.c
@@ -2471,9 +2471,9 @@ VioStorPoolAlloc(IN PVOID DeviceExtension, IN SIZE_T size)
     PADAPTER_EXTENSION adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
     PVOID ptr = (PVOID)((ULONG_PTR)adaptExt->poolAllocationVa + adaptExt->poolOffset);
 
-    if ((adaptExt->poolOffset + size) <= adaptExt->poolAllocationSize)
+    size = ROUND_TO_CACHE_LINES(size);
+    if (size && size <= adaptExt->poolAllocationSize - adaptExt->poolOffset)
     {
-        size = ROUND_TO_CACHE_LINES(size);
         adaptExt->poolOffset += (ULONG)size;
         RtlZeroMemory(ptr, size);
         return ptr;

Not even sure if the part with checking if size is non-zero is really necessary, but I included it because you have a check if the rounding overflowed, too.

[rachael-george]

Thanks for the review @kevmw . I've reworked it to match your suggestion and updated the commit message.

[kevmw]

The commit message looks good to me now. It would be more conventional to have line breaks after 72 characters or so instead of having really long lines, but that's a minor detail. The important part is that there content is there.

As for the code, I'll have to defer to someone else because it was my own suggestion.