Messages in this thread | | | From | Guenter Roeck <> | Date | Mon, 13 Nov 2017 12:14:41 -0800 | Subject | Re: drivers/firmware/google/vpd.c: duplicate sysfs file |
| |
The sysfs warning, yes. However, after unbinding and rebinding the driver, "cat /sys/firmware/vpd/rw_raw" will result in a crash.
Sequence:
echo vpd > /sys/bus/platform/drivers/vpd/unbind echo vpd > /sys/bus/platform/drivers/vpd/bind # <-- nasty message
cat /sys/firmware/vpd/rw_raw # <-- crash
Guenter
On Mon, Nov 13, 2017 at 11:23 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Mon, Nov 13, 2017 at 11:09:21AM -0800, Guenter Roeck wrote: >> On Mon, Nov 13, 2017 at 11:04 AM, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> wrote: >> > >> > On Mon, Nov 13, 2017 at 10:18:35AM -0800, Randy Dunlap wrote: >> > > On 11/13/2017 06:41 AM, Guenter Roeck wrote: >> > > > >> > > > >> > > > On Sun, Nov 12, 2017 at 10:36 PM, Randy Dunlap <rdunlap@infradead.org <mailto:rdunlap@infradead.org>> wrote: >> > > > >> > > > sysfs: cannot create duplicate filename '/devices/platform/vpd' >> > > > >> > > > on the second load of this driver. I.e., >> > > > >> > > > modprobe vpd-sysfs >> > > > rmmod vpd-sysfs >> > > > modprobe vpd-sysfs >> > > > [boom] >> > > > >> > > > Neither the platform device nor the platform driver driver are ever unregistered, so this isn't entirely surprising. I'll try to reproduce and send a patch. >> > > >> > > >> > > Seems to be a common theme: >> > > >> > > google> grep --color=never "platform.*register" *.c >> > > coreboot_table-acpi.c: return platform_driver_register(&coreboot_table_acpi_driver); >> > > coreboot_table-of.c: return platform_driver_register(&coreboot_table_of_driver); >> > >> > These are not unloadable (for better or worse) - they do not have >> > module_exit() in them. >> > >> > > >> > > gsmi.c: gsmi_dev.pdev = platform_device_register_full(&gsmi_dev_info); >> > > gsmi.c: platform_device_unregister(gsmi_dev.pdev); >> > > gsmi.c: platform_device_unregister(gsmi_dev.pdev); >> > > [looks good] >> > > >> > > memconsole-coreboot.c: pdev = platform_device_register_simple("memconsole", -1, NULL, 0); >> > > memconsole-coreboot.c: platform_driver_register(&memconsole_driver); >> > >> > Same here: not unloadable. >> > >> > > >> > > vpd.c: pdev = platform_device_register_simple("vpd", -1, NULL, 0); >> > > vpd.c: platform_driver_register(&vpd_driver); >> > >> > Arguably this should not even be a platform driver, there is no hardware >> > behind it. I was planning on purring some notifiers into coreboot table >> > driver and using notifiers to attach vpd to them. -ENOTIME though. >> > >> Two options for now: clean it up and make it unloadable, or make it bool >> and drop the exit function. Any preference ? >> >> The problem is easy to reproduce even with the driver is built into >> the kernel with a simple unbind/bind sequence. And after the unbind, >> it is easy to crash the system since the sysfs attributes are still there. > > The kernel should not 'crash', just spit out a nasty warning, right? > > thanks, > > greg k-h
| |