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

[PATCH v3 3/8] libxenguest: guard against overflow from too large p2m when checkpointing


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 26 Apr 2022 12:23:21 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=Poou7+ywHc4+47fsnUn586v0ma66GxF2Bm5uaMw2apo=; b=OafK9F/udmJjYocg0DJ1QQvIMYy+x4Y00+tCEMUII2OvuZKqLgqFZnqX7PyDyz4cEaO2Fs1XoWvK3n9zReJAlrSDzljO+jycBH1FB+ZkWvD8SB+G82KpnQHL7WnyoGRO0zNcMMVb3T1bfaNM610vY6r42+cSbUfF52qChtVKz5gc/ynBi7OrKUIqX8ivH8yqa3Ue+KEmbvVRmNtGd83/qt4aFn/riSNHkbAQhZrlhcjZU2WvVKS8Zame27hlS8yxSocBPrP6vfgzvzibPg9VkHWlsXKqSxzt96OFsab9Obc+uCabCj9uWk4nkdMZWNy4RfrrOM5+8nl/ETADR0F+Kw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Lqkb4fOqLW6z91GNCzD+oIYyhISDu9h7dgaadXi5FqpuISqLwzNqFwwqxVc3qRPNZ7t7qknWslMCnvHqxZVdwfWUJTowN/0BATaBFnUFDrUqUufgHLJdBH6RumldcDERrMPfnFbkOBXnzRGow9RyKv0OYDvoiMfE1vih/6JhSgX16WicLpFZAXGgSJUlAHfJHVwAUqhjXlhZs6eAhbirTuqf3HMapKBlO3V3JRyN7L52yTIJq1aRXeTkQOJhp5ES8Q9AaGr2lnXlt9KI9IBb9Bc2lqMzLhHmuIkY821WbQMpCIzvJA1fPuZhxoL3jTvWSDAlHCL0hA9R7nrUBZW4Pw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>
  • Delivery-date: Tue, 26 Apr 2022 10:23:30 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

struct xc_sr_record's length field has just 32 bits. Fill it early and
check that the calculated value hasn't overflowed. Additionally check
for counter overflow early - there's no point even trying to allocate
any memory in such an event.

While there also limit an induction variable's type to unsigned long:
There's no gain from it being uint64_t.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Of course looping over test_bit() is pretty inefficient, but given that
I have no idea how to test this code I wanted to restrict changes to
what can sensibly be seen as no worse than before from just looking at
the changes.

--- a/tools/libs/guest/xg_sr_restore.c
+++ b/tools/libs/guest/xg_sr_restore.c
@@ -424,7 +424,8 @@ static int send_checkpoint_dirty_pfn_lis
     xc_interface *xch = ctx->xch;
     int rc = -1;
     unsigned int count, written;
-    uint64_t i, *pfns = NULL;
+    unsigned long i;
+    uint64_t *pfns = NULL;
     struct iovec *iov = NULL;
     xc_shadow_op_stats_t stats = { 0, ctx->restore.p2m_size };
     struct xc_sr_record rec = {
@@ -444,16 +445,28 @@ static int send_checkpoint_dirty_pfn_lis
 
     for ( i = 0, count = 0; i < ctx->restore.p2m_size; i++ )
     {
-        if ( test_bit(i, dirty_bitmap) )
-            count++;
+        if ( test_bit(i, dirty_bitmap) && !++count )
+            break;
     }
 
+    if ( i < ctx->restore.p2m_size )
+    {
+        ERROR("Too many dirty pfns");
+        goto err;
+    }
+
+    rec.length = count * sizeof(*pfns);
+    if ( rec.length / sizeof(*pfns) != count )
+    {
+        ERROR("Too many (%u) dirty pfns", count);
+        goto err;
+    }
 
-    pfns = malloc(count * sizeof(*pfns));
+    pfns = malloc(rec.length);
     if ( !pfns )
     {
-        ERROR("Unable to allocate %zu bytes of memory for dirty pfn list",
-              count * sizeof(*pfns));
+        ERROR("Unable to allocate %u bytes of memory for dirty pfn list",
+              rec.length);
         goto err;
     }
 
@@ -479,8 +492,6 @@ static int send_checkpoint_dirty_pfn_lis
         goto err;
     }
 
-    rec.length = count * sizeof(*pfns);
-
     iov[0].iov_base = &rec.type;
     iov[0].iov_len = sizeof(rec.type);
 
@@ -488,7 +499,7 @@ static int send_checkpoint_dirty_pfn_lis
     iov[1].iov_len = sizeof(rec.length);
 
     iov[2].iov_base = pfns;
-    iov[2].iov_len = count * sizeof(*pfns);
+    iov[2].iov_len = rec.length;
 
     if ( writev_exact(ctx->restore.send_back_fd, iov, 3) )
     {




 


Rackspace

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