faulty 64-bit resource printk fixup in macio_asic.c

[Paul Collins <paul@briny.ondioline.org>]

Hi Greg,

The patch titled "64bit resource: fix up printks for resources in misc
drivers", committed as e29419fffceb8ec36def3c922040e1ca7bcd3de5 in
Linus's tree, causes my PowerBook to Oops early in boot and udev to
not function.

  Unable to handle kernel paging request for data at address 0x6f000000
  Faulting instruction address: 0xc00c901c
  Oops: Kernel access of bad area, sig: 11 [#1]
  PREEMPT 
  Modules linked in:
  NIP: C00C901C LR: C00C901C CTR: C00C8F78
  REGS: efed5db0 TRAP: 0300   Not tainted  (2.6.17-g9262e914)
  MSR: 00009032 <EE,ME,IR,DR>  CR: 22000484  XER: 20000000
  DAR: 6F000000, DSISR: 40000000
  TASK = efe97240[378] 'udevtrigger' THREAD: efed4000
  GPR00: C00C901C EFED5E60 EFE97240 00000000 CFC0BB40 00010001 CFC0BB40 C02E9754 
  GPR08: 00000000 00000001 EFFE8EA4 EFED4000 44000428 1001D140 100D0000 100D0000 
  GPR16: 00000000 100EBF08 100D0000 100B0000 100D0000 100B0000 100EBEA8 100EC068 
  GPR24: 00000000 EFEEDA20 C0DF7880 EFED4000 CFC0BB40 EFEEDA20 C0DF78D0 6F000000 
  NIP [C00C901C] sysfs_open_file+0xa4/0x284
  LR [C00C901C] sysfs_open_file+0xa4/0x284
  Call Trace:
  [EFED5E60] [C00C901C] sysfs_open_file+0xa4/0x284 (unreliable)
  [EFED5E90] [C007D6A8] __dentry_open+0x108/0x2a4
  [EFED5EC0] [C007D988] do_filp_open+0x5c/0x78
  [EFED5F20] [C007D9FC] do_sys_open+0x58/0xf8
  [EFED5F40] [C000FDB8] ret_from_syscall+0x0/0x38
  --- Exception: c01 at 0xff248d8
      LR = 0xffb525c
  Instruction dump:
  3be0ffea 812b0050 83c90014 419e007c 2f9e0000 419e006c 83fe0004 2f9f0000 
  419e0080 38600001 4bf5b229 4809726d <801f0000> 3ba00000 2f800002 419e0024 
   <6>note: udevtrigger[378] exited with preempt_count 1


The only hunk of that commit affecting my configuration is this one,
which when reverted lets my machine work again.

diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 431bd37..c687ac7 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -428,10 +428,10 @@ #endif
 
 	/* MacIO itself has a different reg, we use it's PCI base */
 	if (np == chip->of_node) {
-		sprintf(dev->ofdev.dev.bus_id, "%1d.%08lx:%.*s",
+		sprintf(dev->ofdev.dev.bus_id, "%1d.%016llx:%.*s",
 			chip->lbus.index,
 #ifdef CONFIG_PCI
-			pci_resource_start(chip->lbus.pdev, 0),
+			(unsigned long long)pci_resource_start(chip->lbus.pdev, 0),
 #else
 			0, /* NuBus may want to do something better here */
 #endif


When applied, this hunk yields

	/* MacIO itself has a different reg, we use it's PCI base */
	if (np == chip->of_node) {
		sprintf(dev->ofdev.dev.bus_id, "%1d.%016llx:%.*s",
			chip->lbus.index,
#ifdef CONFIG_PCI
			(unsigned long long)pci_resource_start(chip->lbus.pdev, 0),
#else
			0, /* NuBus may want to do something better here */
#endif
			MAX_NODE_NAME_SIZE, np->name);


Since dev->ofdev.dev is a struct device, bus_id is 20 bytes, of which
19 are consumed by "%1d.%016llx:".  But the field width used by "%.*s"
is MAX_NODE_NAME_SIZE, which is 8.

drivers/macintosh/macio_asic.c:36:#define MAX_NODE_NAME_SIZE (BUS_ID_SIZE - 12)

So I think the sprintf overflows bus_id and clobbers the next few
bytes of struct device.

I'm not sure what the right thing to do is.  Making bus_id bigger will
cost everyone, but right now with bus_id at 20 bytes, there's no room
after the colon for any of np->name.

-- 
Paul Collins
Melbourne, Australia

Dag vijandelijk luchtschip de huismeester is dood
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/