[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/10] xencontrol: guard against NumberPages overflow in gnttab size calculation
- To: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>, Owen Smith <owen.smith@xxxxxxxxxx>, David ambu <david.preetham@xxxxxxxxxx>, "win-pv-devel@xxxxxxxxxxxxxxxxxxxx" <win-pv-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>
- Date: Tue, 13 Oct 2026 11:09:27 +0200
- Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=fm2 header.d=invisiblethingslab.com header.i="@invisiblethingslab.com" header.h="Content-Transfer-Encoding:Content-Type:Date:From:In-Reply-To:Message-ID:MIME-Version:References:Subject:To"; dkim=pass header.s=fm2 header.d=messagingengine.com header.i="@messagingengine.com" header.h="Content-Transfer-Encoding:Content-Type:Date:Feedback-ID:From:In-Reply-To:Message-ID:MIME-Version:References:Subject:To:X-ME-Proxy:X-ME-Sender"
- Delivery-date: Tue, 13 Oct 2026 09:09:33 +0000
- Dkim2-signature: i=1; m=1; t=1791882569; d=invisiblethingslab.com; mf=PG9tZWdAaW52aXNpYmxldGhpbmdzbGFiLmNvbT4=; rt=PHdpbi1wdi1kZXZlbEBsaXN0cy54ZW5wcm9qZWN0Lm9yZz4=; s=fm2:rsa-sha256:NbkWnMXvTW77SvYbliHNAaqi9Vgm0yjg89hsA82mbNzEXCu tvRuVngh8j0qIekPskXUZarsOZA4sG2cV2AARea6Zcqv8JmB0KGd65uygdFtFeAI 34lpGrlDZxuKWTD6pROjT+r0U4yBzW5jArJBN9bkebWzRmSFBemurg7s/NdXcU8/ j8I999OQujkBjqCoYfq+XuTdmtXpjh4WhQZW5psAIfeFYpMdearTLfjx+VAiDfiv bHk70Oo2A/Duo0H7kSDKdCl8D7ZXd7zAV5LGQwojudtEMDOo9YQbl+jQLh99YLPQ 6YRSnFxOdG/H1Lk+Jqy2zm+x2cQ7Q44A/U09Nmg==;
- Feedback-id: i409c4082:Fastmail
- List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>
- Message-instance: m=1; h=sha256:AjYAHMlI94+ze2gj9ZgV1ErCliYMTbEVW+DBvp+Xmh8=:Liuap1p6ZL7nwGfuVWiVBVsPfQAAvg5Ir5df5SOcJLg=;
On 13.10.2026 10:10, Tu Dinh wrote:
On 13/10/2026 10:01, Owen Smith wrote:
diff --git a/src/xencontrol/xencontrol.c b/src/xencontrol/xencontrol.c
index a68dcf6..8f21867 100644
--- a/src/xencontrol/xencontrol.c
+++ b/src/xencontrol/xencontrol.c
@@ -407,6 +407,11 @@ XcGnttabPermitForeignAccess2(
BOOL Success;
DWORD Status;
+ if (NumberPages == 0 || NumberPages > XENCONTROL_GNTTAB_MAX_PAGES) {
I believe the boundary condition of NumberPages ==
XENCONTROL_GNTTAB_MAX_PAGES still causes an overflow? So
XENCONTROL_GNTTAB_MAX_PAGES should be (1024 * 1024 - 1) instead.
MAXULONG / PAGE_SIZE would be a better formulation still, only problem
being PAGE_SIZE is not defined for usermode targets.
Maximum number of pages allowed is also checked in IoctlGnttabMapForeignPages
against a hard coded 1024*1024.
It would be good to have the same constant/#define referred to in both places.
Eventually, this ends up in FdoPciHoleAllocate or FdoMemoryHoleAllocate,
depending
on the "UseMemoryHole" setting.
FdoPciHoleAllocate is limited to 4GiB - PAGE_SIZE bytes (i.e. 1024*1024 - 1
pages)
FdoMemoryHoleAllocate's allocations doesnt specify a maximum size, but the
chances
of allocating a contiguous 4GB set of pages would be problematic.
Its likely this check is excessively large, but I'm unsure as to what the safe
correct
maximum would be.
I think in general it'd be nice to bound the usermode grant consumption
to the actual free grant table space (and reserve some certain amount
for the kernel), but that doesn't look simple to implement.
SeLockMemoryPrivilege may also be worth checking in the caller path of
some ioctls.
I'll finally be sending my rework of the grant/map code soon, hopefully
this week. Got caught up with other projects, but I'm testing it now. It
includes new interface versions with bounded batched maps/unmaps,
per-page error reporting and other improvements.
What's the policy on retaining old xenbus interface versions?
Rafał Wojdyła
Invisible Things Lab
|
