[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/msix: fix incorrect refcount decrease in msixtlb


  • To: xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Date: Wed, 26 Nov 2025 09:24:44 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=eupPBkUMRbSsxUj1BjojkW4iyUEmlpMAJsfZgniDz1s=; b=XA6DXA4VGhHCVOYyXH4+S/51MJUQtJIN57AWBopZzzsf5P/8bFxCaXJ1qvhu2sWC31v69+VWtusTSmCRF4FdZcpMnIEV7RomVospA5LDQ7eVUYS1B3q7R8GJZERbjgje2dW6o4qfqzAzvAD9oOpeN4A2c4UsV2+SMiQHPsWh3dAHZAA16JAnGisi852v2nhX/jgCUsitPOr5z62yEO0qwnIT153oNOFWX3ciED39KsMaucXFPVA9yctHPHYnXFNP1G1i7+fQ+cTObbWesOxsZ6N58yOw/N0Sqk4LW/s+mXkvQNDS0OO2H3ru523rfoLpRENsLn+6IRMdGXoA/OHlJw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=kQTFk5m8aRiT0g+ReGp2ksuVlwR9jvgVKqkEib/+kFvMNZ83O70pOGk44ocuqcHJaDRerSJ3+6ncyJBrlEV/J2FpE0waKvRAaytc42zuVoDwr6VrcoKCSIckb9Ya5wP036XMICntLv3TUubfXXH+sSeyWEqfXyxV6DfY5PiVouP4OMMawuYt9y2OZ9dzThp+Y1FbZ8x5UAeagvDXZoym797jVCpPF3nAv2lkZj3ozqgdCZRMM72emWrY2dPu67DcD9gB6PfdViZGrmUUqF/0k1WlUFGXPmyoclNckUlSYa8q1SyOqB7Wimtvsxj0vAEb3Nn51A4uU+wbsxAun/lmTA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Wed, 26 Nov 2025 08:25:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

The usage of atomic_dec_and_test() in msixtbl_pt_unregister() is inverted:
the function will return true when the refcount reaches 0.  The current
code does the opposite and calls del_msixtbl_entry() when there are still
refcounts held on the object.

However all callers of msixtbl_pt_unregister() are serialized on the domctl
lock, and hence there cannot be parallel calls to msixtbl_pt_unregister()
that could lead to double freeing of the same object.

The incorrect freeing with active msixtlb entries will result in a possible
guest visible malfunction, but no internal Xen state corruption.

While entries are leaked once the last pIRQ is unbound, the same entry
would get re-used if the device has pIRQs bound again.  The guest cannot
exploit this incorrect refcount check to leak arbitrary amounts of memory
by repeatedly enabling and disabling (binding and unbinding) MSI-X entries.

Fixes: 34097f0d3080 ('hvm: passthrough MSI-X mask bit acceleration')
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/hvm/vmsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 32e417bc1592..27b1f089e20b 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -716,7 +716,7 @@ out:
     return;
 
 found:
-    if ( !atomic_dec_and_test(&entry->refcnt) )
+    if ( atomic_dec_and_test(&entry->refcnt) )
         del_msixtbl_entry(entry);
 
     spin_unlock_irq(&irqd->lock);
-- 
2.51.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.