Patchwork pci: fix device paths

login
register
mail settings
Submitter Michael S. Tsirkin
Date Jan. 19, 2011, 7:24 p.m.
Message ID <20110119192408.GA1521@redhat.com>
Download mbox | patch
Permalink /patch/79582/
State New
Headers show

Comments

Michael S. Tsirkin - Jan. 19, 2011, 7:24 p.m.
Patch a6a7005d14b3c32d4864a718fb1cb19c789f58a5 generated
broken device paths. We snprintf with a length shorter
than the output, so the last character is discarded and replaced
by the null byte. Fix it up by snprintf to a buffer
which is larger by 1 byte and then memcpy the data (without
the null byte) to where we need it.

Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

This fixes the issue for me. Could you ack pls?

 hw/pci.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
Christoph Hellwig - Jan. 20, 2011, 9:41 a.m.
On Wed, Jan 19, 2011 at 09:24:10PM +0200, Michael S. Tsirkin wrote:
> Patch a6a7005d14b3c32d4864a718fb1cb19c789f58a5 generated
> broken device paths. We snprintf with a length shorter
> than the output, so the last character is discarded and replaced
> by the null byte. Fix it up by snprintf to a buffer
> which is larger by 1 byte and then memcpy the data (without
> the null byte) to where we need it.

This fixed the boot for me.

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 8d0e3df..c77f6e9 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -2032,10 +2032,13 @@  static char *pcibus_get_dev_path(DeviceState *dev)
      * domain:Bus:Slot.Func for systems without nested PCI bridges.
      * Slot.Function list specifies the slot and function numbers for all
      * devices on the path from root to the specific device. */
-    int domain_len = strlen("DDDD:00");
-    int slot_len = strlen(":SS.F");
+    char domain[] = "DDDD:00";
+    char slot[] = ":SS.F";
+    int domain_len = sizeof domain - 1 /* For '\0' */;
+    int slot_len = sizeof slot - 1 /* For '\0' */;
     int path_len;
     char *path, *p;
+    int s;
 
     /* Calculate # of slots on path between device and root. */;
     slot_depth = 0;
@@ -2050,14 +2053,19 @@  static char *pcibus_get_dev_path(DeviceState *dev)
     path[path_len] = '\0';
 
     /* First field is the domain. */
-    snprintf(path, domain_len, "%04x:00", pci_find_domain(d->bus));
+    s = snprintf(domain, sizeof domain, "%04x:00", pci_find_domain(d->bus));
+    assert(s == domain_len);
+    memcpy(path, domain, domain_len);
 
     /* Fill in slot numbers. We walk up from device to root, so need to print
      * them in the reverse order, last to first. */
     p = path + path_len;
     for (t = d; t; t = t->bus->parent_dev) {
         p -= slot_len;
-        snprintf(p, slot_len, ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(d->devfn));
+        s = snprintf(slot, sizeof slot, ":%02x.%x",
+                     PCI_SLOT(t->devfn), PCI_FUNC(d->devfn));
+        assert(s == slot_len);
+        memcpy(p, slot, slot_len);
     }
 
     return path;