* Problems with setting devices' bus_id on sparc32
@ 2006-10-27 6:55 Jurij Smakov
2006-10-27 7:22 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Jurij Smakov @ 2006-10-27 6:55 UTC (permalink / raw)
To: sparclinux
[-- Attachment #1: Type: text/plain, Size: 983 bytes --]
Hi,
While investigating Debian bug #394697 [0] on sparc32, I've found that
in a couple of places the bus_id field of struct device is set by
copying the dp->path_component_name (obtained from prom) without any
length checking. In most cases it happens to work, however on
SparcStation5 for some devices this string is over 20 chars long (like
'power-management@4,a0000000'), and that exceeds the amount of memory
allocated for bus_id (which is BUS_ID_SIZE == 20). So, blindly
copying the names into bus_id using strcpy() leads to all kinds of bad
things. The attached hackish patch replaces strcpy() by strlcpy() in
two places, making it possible to boot SS5 again (tested in QEMU,
where vanilla 2.6.18 fails to boot with symptoms identical to the ones
described in the bug).
[0] http://bugs.debian.org/394697
Best regards,
--
Jurij Smakov jurij@wooyd.org
Key: http://www.wooyd.org/pgpkey/ KeyID: C99E03CC
[-- Attachment #2: bus-id-size.patch --]
[-- Type: text/plain, Size: 1035 bytes --]
diff -aur a/arch/sparc/kernel/of_device.c b/arch/sparc/kernel/of_device.c
--- a/arch/sparc/kernel/of_device.c 2006-09-19 20:42:06.000000000 -0700
+++ b/arch/sparc/kernel/of_device.c 2006-10-26 22:53:50.000000000 -0700
@@ -652,7 +652,7 @@
if (!parent)
strcpy(op->dev.bus_id, "root");
else
- strcpy(op->dev.bus_id, dp->path_component_name);
+ strlcpy(op->dev.bus_id, dp->path_component_name, BUS_ID_SIZE);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
diff -aur a/drivers/sbus/sbus.c b/drivers/sbus/sbus.c
--- a/drivers/sbus/sbus.c 2006-09-19 20:42:06.000000000 -0700
+++ b/drivers/sbus/sbus.c 2006-10-26 22:53:19.000000000 -0700
@@ -61,7 +61,7 @@
else
sdev->ofdev.dev.parent = &sdev->bus->ofdev.dev;
sdev->ofdev.dev.bus = &sbus_bus_type;
- strcpy(sdev->ofdev.dev.bus_id, dp->path_component_name);
+ strlcpy(sdev->ofdev.dev.bus_id, dp->path_component_name, BUS_ID_SIZE);
if (of_device_register(&sdev->ofdev) != 0)
printk(KERN_DEBUG "sbus: device registration error for %s!\n",
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Problems with setting devices' bus_id on sparc32
2006-10-27 6:55 Problems with setting devices' bus_id on sparc32 Jurij Smakov
@ 2006-10-27 7:22 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2006-10-27 7:22 UTC (permalink / raw)
To: sparclinux
From: Jurij Smakov <jurij@wooyd.org>
Date: Thu, 26 Oct 2006 23:55:03 -0700
> While investigating Debian bug #394697 [0] on sparc32, I've found that
> in a couple of places the bus_id field of struct device is set by
> copying the dp->path_component_name (obtained from prom) without any
> length checking. In most cases it happens to work, however on
> SparcStation5 for some devices this string is over 20 chars long (like
> 'power-management@4,a0000000'), and that exceeds the amount of memory
> allocated for bus_id (which is BUS_ID_SIZE = 20). So, blindly
> copying the names into bus_id using strcpy() leads to all kinds of bad
> things. The attached hackish patch replaces strcpy() by strlcpy() in
> two places, making it possible to boot SS5 again (tested in QEMU,
> where vanilla 2.6.18 fails to boot with symptoms identical to the ones
> described in the bug).
>
> [0] http://bugs.debian.org/394697
I happened to be playing around in this area recently.
I recently fixed the problem on sparc64 that the bus_id's being
generated by of_device.c were not globally unique (the device layer
requires that they are), so I changed it to "%s@%08x" using dp->name
and dp->node.
But that will run into the same length issue.
So let's just use "%08x" and dp->node, with option fixed sized string
prefix for ebus/isa/sbus, for all of these things.
diff --git a/arch/sparc/kernel/ebus.c b/arch/sparc/kernel/ebus.c
index 75ac24d..8ed9dcb 100644
--- a/arch/sparc/kernel/ebus.c
+++ b/arch/sparc/kernel/ebus.c
@@ -237,12 +237,12 @@ void __init fill_ebus_device(struct devi
dev->ofdev.node = dp;
dev->ofdev.dev.parent = &dev->bus->ofdev.dev;
dev->ofdev.dev.bus = &ebus_bus_type;
- strcpy(dev->ofdev.dev.bus_id, dp->path_component_name);
+ sprintf(dev->ofdev.dev.bus_id, "ebus[%08x]", dp->node);
/* Register with core */
if (of_device_register(&dev->ofdev) != 0)
printk(KERN_DEBUG "ebus: device registration error for %s!\n",
- dev->ofdev.dev.bus_id);
+ dp->path_component_name);
if ((dp = dp->child) != NULL) {
dev->children = (struct linux_ebus_child *)
diff --git a/arch/sparc/kernel/of_device.c b/arch/sparc/kernel/of_device.c
index 74bef2a..46200c4 100644
--- a/arch/sparc/kernel/of_device.c
+++ b/arch/sparc/kernel/of_device.c
@@ -651,7 +651,7 @@ build_resources:
if (!parent)
strcpy(op->dev.bus_id, "root");
else
- strcpy(op->dev.bus_id, dp->path_component_name);
+ sprintf(op->dev.bus_id, "%08x", dp->node);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
diff --git a/arch/sparc64/kernel/ebus.c b/arch/sparc64/kernel/ebus.c
index 2df25c2..9b6fdbc 100644
--- a/arch/sparc64/kernel/ebus.c
+++ b/arch/sparc64/kernel/ebus.c
@@ -389,12 +389,12 @@ static void __init fill_ebus_device(stru
dev->ofdev.node = dp;
dev->ofdev.dev.parent = &dev->bus->ofdev.dev;
dev->ofdev.dev.bus = &ebus_bus_type;
- strcpy(dev->ofdev.dev.bus_id, dp->path_component_name);
+ sprintf(dev->ofdev.dev.bus_id, "ebus[%08x]", dp->node);
/* Register with core */
if (of_device_register(&dev->ofdev) != 0)
printk(KERN_DEBUG "ebus: device registration error for %s!\n",
- dev->ofdev.dev.bus_id);
+ dp->path_component_name);
dp = dp->child;
if (dp) {
diff --git a/arch/sparc64/kernel/isa.c b/arch/sparc64/kernel/isa.c
index 0f3aec7..cc56413 100644
--- a/arch/sparc64/kernel/isa.c
+++ b/arch/sparc64/kernel/isa.c
@@ -115,12 +115,12 @@ static void __init isa_fill_devices(stru
isa_dev->ofdev.node = dp;
isa_dev->ofdev.dev.parent = &isa_br->ofdev.dev;
isa_dev->ofdev.dev.bus = &isa_bus_type;
- strcpy(isa_dev->ofdev.dev.bus_id, dp->path_component_name);
+ sprintf(isa_dev->ofdev.dev.bus_id, "isa[%08x]", dp->node);
/* Register with core */
if (of_device_register(&isa_dev->ofdev) != 0) {
printk(KERN_DEBUG "isa: device registration error for %s!\n",
- isa_dev->ofdev.dev.bus_id);
+ dp->path_component_name);
kfree(isa_dev);
goto next_sibling;
}
diff --git a/arch/sparc64/kernel/of_device.c b/arch/sparc64/kernel/of_device.c
index 983ca5f..8cc14fc 100644
--- a/arch/sparc64/kernel/of_device.c
+++ b/arch/sparc64/kernel/of_device.c
@@ -861,7 +861,7 @@ static struct of_device * __init scan_on
if (!parent)
strcpy(op->dev.bus_id, "root");
else
- sprintf(op->dev.bus_id, "%s@%08x", dp->name, dp->node);
+ sprintf(op->dev.bus_id, "%08x", dp->node);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
diff --git a/drivers/sbus/sbus.c b/drivers/sbus/sbus.c
index 935952e..98fcbb3 100644
--- a/drivers/sbus/sbus.c
+++ b/drivers/sbus/sbus.c
@@ -61,11 +61,11 @@ static void __init fill_sbus_device(stru
else
sdev->ofdev.dev.parent = &sdev->bus->ofdev.dev;
sdev->ofdev.dev.bus = &sbus_bus_type;
- strcpy(sdev->ofdev.dev.bus_id, dp->path_component_name);
+ sprintf(sdev->ofdev.dev.bus_id, "sbus[%08x]", dp->node);
if (of_device_register(&sdev->ofdev) != 0)
printk(KERN_DEBUG "sbus: device registration error for %s!\n",
- sdev->ofdev.dev.bus_id);
+ dp->path_component_name);
}
static void __init sbus_bus_ranges_init(struct device_node *dp, struct sbus_bus *sbus)
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-10-27 7:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-27 6:55 Problems with setting devices' bus_id on sparc32 Jurij Smakov
2006-10-27 7:22 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.