From 0e9d3b567d54beabd722dabaeb78001959153196 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 20 Jun 2012 19:19:16 -0600
Subject: [PATCH] util: avoid PATH_MAX-sized array
To: libvir-list@redhat.com

https://bugzilla.redhat.com/show_bug.cgi?id=816601
CVE-2012-2693

See previous patch for why this is good...

[from the previous patch aa286e5]:
    POSIX allows implementations where PATH_MAX is undefined, leading
    to compilation error.  Not to mention that even if it is defined,
    it is often wasteful in relation to the amount of data being stored.

* src/util/pci.c (struct _pciDevice, pciGetDevice, pciFreeDevice):
Manage path dynamically.  Report snprintf overflow.
* src/util/hostusb.c (struct _usbDevice, usbGetDevice)
(usbFreeDevice): Likewise.
(cherry picked from commit 60bfd5b5653b85c9c7950927c3265025630e8300)

Signed-off-by: Daniel Veillard <veillard@redhat.com>
---
 src/util/hostusb.c |   32 +++++++++++++++++++++++++-------
 src/util/pci.c     |   31 +++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 2d6e414..79bf030 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2009-2011 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -48,7 +48,7 @@ struct _usbDevice {
 
     char          name[USB_ADDR_LEN]; /* domain:bus:slot.function */
     char          id[USB_ID_LEN];     /* product vendor */
-    char          path[PATH_MAX];
+    char          *path;
 };
 
 /* For virReportOOMError()  and virReportSystemError() */
@@ -171,13 +171,30 @@ usbGetDevice(unsigned bus,
     dev->bus     = bus;
     dev->dev     = devno;
 
-    snprintf(dev->name, sizeof(dev->name), "%.3o:%.3o",
-             dev->bus, dev->dev);
-    snprintf(dev->path, sizeof(dev->path),
-             USB_DEVFS "%03d/%03d", dev->bus, dev->dev);
+    if (snprintf(dev->name, sizeof(dev->name), "%.3o:%.3o",
+                 dev->bus, dev->dev) >= sizeof(dev->name)) {
+        usbReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("dev->name buffer overflow: %.3o:%.3o"),
+                       dev->bus, dev->dev);
+        usbFreeDevice(dev);
+        return NULL;
+    }
+    if (virAsprintf(&dev->path, USB_DEVFS "%03d/%03d",
+                    dev->bus, dev->dev) < 0) {
+        virReportOOMError();
+        usbFreeDevice(dev);
+        return NULL;
+    }
 
     /* XXX fixme. this should be product/vendor */
-    snprintf(dev->id, sizeof(dev->id), "%d %d", dev->bus, dev->dev);
+    if (snprintf(dev->id, sizeof(dev->id), "%d %d", dev->bus,
+                 dev->dev) >= sizeof(dev->id)) {
+        usbReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("dev->id buffer overflow: %d %d"),
+                       dev->bus, dev->dev);
+        usbFreeDevice(dev);
+        return NULL;
+    }
 
     VIR_DEBUG("%s %s: initialized", dev->id, dev->name);
 
@@ -203,6 +220,7 @@ void
 usbFreeDevice(usbDevice *dev)
 {
     VIR_DEBUG("%s %s: freeing", dev->id, dev->name);
+    VIR_FREE(dev->path);
     VIR_FREE(dev);
 }
 
diff --git a/src/util/pci.c b/src/util/pci.c
index b07ff96..dfa539d 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -55,7 +55,7 @@ struct _pciDevice {
 
     char          name[PCI_ADDR_LEN]; /* domain:bus:slot.function */
     char          id[PCI_ID_LEN];     /* product vendor */
-    char          path[PATH_MAX];
+    char          *path;
     int           fd;
 
     unsigned      initted;
@@ -1147,10 +1147,21 @@ pciGetDevice(unsigned domain,
     dev->slot     = slot;
     dev->function = function;
 
-    snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x",
-             dev->domain, dev->bus, dev->slot, dev->function);
-    snprintf(dev->path, sizeof(dev->path),
-             PCI_SYSFS "devices/%s/config", dev->name);
+    if (snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x",
+                 dev->domain, dev->bus, dev->slot,
+                 dev->function) >= sizeof(dev->name)) {
+        pciReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("dev->name buffer overflow: %.4x:%.2x:%.2x.%.1x"),
+                       dev->domain, dev->bus, dev->slot, dev->function);
+        pciFreeDevice(dev);
+        return NULL;
+    }
+    if (virAsprintf(&dev->path, PCI_SYSFS "devices/%s/config",
+                    dev->name) < 0) {
+        virReportOOMError();
+        pciFreeDevice(dev);
+        return NULL;
+    }
 
     if (access(dev->path, F_OK) != 0) {
         virReportSystemError(errno,
@@ -1174,7 +1185,14 @@ pciGetDevice(unsigned domain,
     }
 
     /* strings contain '0x' prefix */
-    snprintf(dev->id, sizeof(dev->id), "%s %s", &vendor[2], &product[2]);
+    if (snprintf(dev->id, sizeof(dev->id), "%s %s", &vendor[2],
+                 &product[2]) >= sizeof(dev->id)) {
+        pciReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("dev->id buffer overflow: %s %s"),
+                       &vendor[2], &product[2]);
+        pciFreeDevice(dev);
+        return NULL;
+    }
 
     VIR_FREE(product);
     VIR_FREE(vendor);
@@ -1191,6 +1209,7 @@ pciFreeDevice(pciDevice *dev)
         return;
     VIR_DEBUG("%s %s: freeing", dev->id, dev->name);
     pciCloseConfig(dev);
+    VIR_FREE(dev->path);
     VIR_FREE(dev);
 }
 
-- 
1.7.7.4