lkml.org 
[lkml]   [2017]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: 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

\
 
 \ /
  Last update: 2017-11-13 21:15    [W:0.949 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site