All of lore.kernel.org
 help / color / mirror / Atom feed
* drivers/firmware/google/vpd.c: duplicate sysfs file
@ 2017-11-13  6:36 Randy Dunlap
       [not found] ` <CABXOdTfJkdWzqptbqkbJqNtxx5+qeW=+4CZFUG9UxJ6QADAsRw@mail.gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2017-11-13  6:36 UTC (permalink / raw)
  To: LKML; +Cc: Greg Kroah-Hartman, Dmitry Torokhov, Guenter Roeck, Wei-Ning Huang

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]


on 4.14-rc8

-- 
~Randy

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
       [not found] ` <CABXOdTfJkdWzqptbqkbJqNtxx5+qeW=+4CZFUG9UxJ6QADAsRw@mail.gmail.com>
@ 2017-11-13 18:18   ` Randy Dunlap
  2017-11-13 19:04     ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2017-11-13 18:18 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: LKML, Greg Kroah-Hartman, Dmitry Torokhov, Guenter Roeck, Wei-Ning Huang

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);

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);

vpd.c:	pdev = platform_device_register_simple("vpd", -1, NULL, 0);
vpd.c:	platform_driver_register(&vpd_driver);




-- 
~Randy

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 18:18   ` Randy Dunlap
@ 2017-11-13 19:04     ` Dmitry Torokhov
  2017-11-13 19:09       ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2017-11-13 19:04 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Guenter Roeck, LKML, Greg Kroah-Hartman, Guenter Roeck, Wei-Ning Huang

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.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 19:04     ` Dmitry Torokhov
@ 2017-11-13 19:09       ` Guenter Roeck
  2017-11-13 19:23         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-11-13 19:09 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Randy Dunlap, LKML, Greg Kroah-Hartman, Guenter Roeck, Wei-Ning Huang

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.

Guenter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 19:09       ` Guenter Roeck
@ 2017-11-13 19:23         ` Greg Kroah-Hartman
  2017-11-13 20:14           ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-13 19:23 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Dmitry Torokhov, Randy Dunlap, LKML, Guenter Roeck, Wei-Ning Huang

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 19:23         ` Greg Kroah-Hartman
@ 2017-11-13 20:14           ` Guenter Roeck
  2017-11-13 22:01             ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-11-13 20:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dmitry Torokhov, Randy Dunlap, LKML, Guenter Roeck, Wei-Ning Huang

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 20:14           ` Guenter Roeck
@ 2017-11-13 22:01             ` Dmitry Torokhov
  2017-11-13 22:24               ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2017-11-13 22:01 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg Kroah-Hartman, Randy Dunlap, LKML, Guenter Roeck, Wei-Ning Huang

On Mon, Nov 13, 2017 at 12:14:41PM -0800, Guenter Roeck wrote:
> 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

I'd disable unbind (.suppress_bind_attrs = true) and commented out
unload for now. And then looked into fixing properly.

> 
> 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

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 22:01             ` Dmitry Torokhov
@ 2017-11-13 22:24               ` Guenter Roeck
  2017-11-13 22:26                 ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-11-13 22:24 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Greg Kroah-Hartman, Randy Dunlap, LKML, Guenter Roeck, Wei-Ning Huang

On Mon, Nov 13, 2017 at 2:01 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Nov 13, 2017 at 12:14:41PM -0800, Guenter Roeck wrote:
>> 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
>
> I'd disable unbind (.suppress_bind_attrs = true) and commented out
> unload for now. And then looked into fixing properly.
>

Fixing properly is easy as long as we retain the platform device; I
already have a set of patches doing just that. I have no idea how to
implement the non-platform-device variant, if that is what you mean
with "properly".

Guenter

>>
>> 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
>
> --
> Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: drivers/firmware/google/vpd.c: duplicate sysfs file
  2017-11-13 22:24               ` Guenter Roeck
@ 2017-11-13 22:26                 ` Dmitry Torokhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2017-11-13 22:26 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg Kroah-Hartman, Randy Dunlap, LKML, Guenter Roeck, Wei-Ning Huang

On November 13, 2017 2:24:23 PM PST, Guenter Roeck <groeck@google.com> wrote:
>On Mon, Nov 13, 2017 at 2:01 PM, Dmitry Torokhov
><dmitry.torokhov@gmail.com> wrote:
>> On Mon, Nov 13, 2017 at 12:14:41PM -0800, Guenter Roeck wrote:
>>> 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
>>
>> I'd disable unbind (.suppress_bind_attrs = true) and commented out
>> unload for now. And then looked into fixing properly.
>>
>
>Fixing properly is easy as long as we retain the platform device; I
>already have a set of patches doing just that. I have no idea how to
>implement the non-platform-device variant, if that is what you mean
>with "properly".

Ah, ok then.


Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-11-13 22:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13  6:36 drivers/firmware/google/vpd.c: duplicate sysfs file Randy Dunlap
     [not found] ` <CABXOdTfJkdWzqptbqkbJqNtxx5+qeW=+4CZFUG9UxJ6QADAsRw@mail.gmail.com>
2017-11-13 18:18   ` Randy Dunlap
2017-11-13 19:04     ` Dmitry Torokhov
2017-11-13 19:09       ` Guenter Roeck
2017-11-13 19:23         ` Greg Kroah-Hartman
2017-11-13 20:14           ` Guenter Roeck
2017-11-13 22:01             ` Dmitry Torokhov
2017-11-13 22:24               ` Guenter Roeck
2017-11-13 22:26                 ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.