This is the issue causing #664562 -- it seems to be a 64-bit kernel, 32-bit userland compatibility issue with structure sizes. GCC prior to version 4.7 would place the filename in a different place and so the kernel writing beyond the end of the structure was not noticed.
Simon- ----- Forwarded message from Stefan Richter <stef...@s5r6.in-berlin.de> ----- Date: Fri, 5 Oct 2012 20:32:20 +0200 From: Stefan Richter <stef...@s5r6.in-berlin.de> To: Simon Kirby <s...@hostway.ca> Cc: Kristian Hogsberg <k...@redhat.com>, linux1394-de...@lists.sourceforge.net Subject: Re: libraw1394 corruption with new GCC X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.10; x86_64-pc-linux-gnu) (adding Cc: linux1394-devel, quoting in full) On Oct 05 Simon Kirby wrote: > Hello! :) > > I'm writing you as I'm trying to figure out some corruption that seems to > be causing problems for me since GCC 4.7 started being used on compiled > libraw1394 on Debian sid (i386). If I compile "fw.c" from this package > with GCC 4.6, or with -fno-stack-protector, it works; otherwise, > filename[] in the fw_set_port() function seems to get clobbered by this > ioctl call: > > if (ioctl(fd, FW_CDEV_IOC_GET_INFO, &get_info) < 0) { > > ...you seem to have worked on this ioctl on the kernel side. If I set up > a GDB watchpoint on "filename", I can see it originally getting set, but > not when it gets corrupted. I think this must be happening in the kernel. > > I modified fw_set_port() as follows: > > 657 fd = open(filename, O_RDWR); > 658 if (fd < 0) > 659 continue; > 660 > 661 memset(&get_info, 0, sizeof(get_info)); > 662 memset(&reset, 0, sizeof(reset)); > 663 get_info.version = IMPLEMENTED_CDEV_ABI_VERSION; > 664 get_info.rom = 0; > 665 get_info.rom_length = 0; > 666 get_info.bus_reset = ptr_to_u64(&reset); > 667 asm("":::"memory"); > 668 fprintf(stderr,"HERE:%s\n", filename); > 669 asm("":::"memory"); > 670 if (ioctl(fd, FW_CDEV_IOC_GET_INFO, &get_info) < 0) { > 671 close(fd); > 672 continue; > 673 } > 674 asm("":::"memory"); > 675 fprintf(stderr,"HERE2:%s\n", filename); > 676 asm("":::"memory"); > 677 > 678 if (get_info.card != handle->ports[port].card) { > 679 close(fd); > 680 continue; > 681 } > > strace shows: > > open("/dev", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 7 > getdents(7, /* 709 entries */, 32768) = 13880 > open("/dev/fw0", O_RDWR) = 8 > write(2, "HERE:/dev/fw0\n", 14HERE:/dev/fw0 > ) = 14 > ioctl(8, FW_CDEV_IOC_GET_INFO, 0xffbb8114) = 0 > write(2, "HERE2:\377\377\377\377/fw0\n", 15HERE2:????????/fw0 > ) = 15 > epoll_ctl(3, EPOLL_CTL_ADD, 8, {EPOLLIN, {u32=167392044, u64=167392044}}) = 0 > > gdb again, from the start: > > Breakpoint 2, fw_set_port (handle=handle@entry=0x804b018, port=port@entry=0) > at fw.c:668 > 668 fprintf(stderr,"HERE:%s\n", filename); > (gdb) print filename > $1 = "/dev/fw0\000:\372\367\274\000\001\000\000 > \372\367T4\372\367\364\037\373\367\030\260\004\b" > (gdb) print get_info > $2 = {version = 4, rom_length = 0, rom = 0, bus_reset = 4294957064, > bus_reset_closure = 0, card = 0} > (gdb) print reset > $3 = {closure = 0, type = 0, node_id = 0, local_node_id = 0, bm_node_id = 0, > irm_node_id = 0, root_node_id = 0, generation = 0} > (gdb) c > Continuing. > HERE:/dev/fw0 > > Breakpoint 3, fw_set_port (handle=handle@entry=0x804b018, port=port@entry=0) > at fw.c:675 > 675 fprintf(stderr,"HERE2:%s\n", filename); > (gdb) print filename > $4 = "\000\000\000\000/fw0\000:\372\367\274\000\001\000\000 > \372\367T4\372\367\364\037\373\367\030\260\004\b" > (gdb) print get_info > $5 = {version = 5, rom_length = 88, rom = 0, bus_reset = 4294957064, > bus_reset_closure = 0, card = 0} > (gdb) print reset > $6 = {closure = 0, type = 0, node_id = 65472, local_node_id = 65472, > bm_node_id = 65472, irm_node_id = 65473, root_node_id = 65473, generation = 2} > (gdb) print &filename > $7 = (char (*)[32]) 0xffffd82c > (gdb) print &get_info > $8 = (struct fw_cdev_get_info *) 0xffffd7e4 > (gdb) print &reset > $9 = (struct fw_cdev_event_bus_reset *) 0xffffd808 > > Do you see what is wrong here? This is an amd64 kernel with i386 userland. > > Cheers, > > Simon- Sounds like a 32-bits-userland-on-64-bits-kernel compatibility issue indeed. It is quite a long time ago that I runtime tested 32-on-64 ABI compatibility; apparently we have a regression, or an old bug which only now really hits. For what it's worth, here is what I get on my amd64 PC... gcc (Gentoo 4.5.4 p1.0, pie-0.4.7) 4.5.4 Target: x86_64-pc-linux-gnu sizeof(struct fw_cdev_event_bus_reset) == 40 sizeof(struct fw_cdev_get_info) == 40 ...and on my i686 PC: gcc (Gentoo 4.5.3-r2 p1.1, pie-0.4.7) 4.5.3 Target: i686-pc-linux-gnu sizeof(struct fw_cdev_event_bus_reset) == 36 sizeof(struct fw_cdev_get_info) == 36 In the kernel, drivers/firewire/core-cdev.c::ioctl_get_info(), we copy with sizeof(struct fw_cdev_event_bus_reset)... ret = copy_to_user(u64_to_uptr(a->bus_reset), &bus_reset, sizeof(bus_reset)); ...and in dispatch_ioctl() with the size that the user gave us: if (_IOC_DIR(cmd) & _IOC_READ) if (copy_to_user(arg, &buffer, _IOC_SIZE(cmd))) In other words, we don't overrun the client's fw_cdev_get_info buffer (because we let the client tell us how many bytes it thinks its buffer has), but we obviously overrun the client's fw_cdev_event_bus_reset buffer. This really looks to me as if we always did this wrong, didn't we? Does anybody have a nifty idea how to get the compiler or preprocessor to compute the size of struct fw_cdev_event_bus_reset as if it would be __packed, IOW how to write ret = copy_to_user(u64_to_uptr(a->bus_reset), &bus_reset, 36); in a better way? PS, from grepping through core-cdev.c, it looks like all our copy_from_user are safe, and from all our copy_to_user only the one above with sizeof(bus_reset) is unsafe. We have one other copy_to_user where the kernel sets the size; this cannot overrun the buffer but it could write the results with a different alignment than the client expects. This was indeed the case with transaction response events before kernel 2.6.27. -- Stefan Richter -=====-===-- =-=- --=-= http://arcgraph.de/sr/ ----- End forwarded message ----- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org