Re: drivers/firmware/google/vpd.c: duplicate sysfs file

[Greg Kroah-Hartman]

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@xxxxxxxxx> 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@xxxxxxxxxxxxx <mailto:rdunlap@xxxxxxxxxxxxx>> 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