On Sat, Mar 20, 2010 at 07:08:48PM +0800, Yong Wang wrote:
> On Fri, Mar 19, 2010 at 11:30:02PM -0700, Dmitry Torokhov wrote:
> >
> > > +static int __init eeepc_wmi_init(void)
> > > +{
> > > + int err;
> > > + acpi_status status;
> > > +
> > > + if (!wmi_has_guid(EEEPC_WMI_EVENT_GUID)) {
> > > + printk(KERN_WARNING "EEEPC WMI: No known WMI GUID found\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + err = eeepc_wmi_input_setup();
> > > + if (err)
> > > + return err;
> > > +
> > > + status = wmi_install_notify_handler(EEEPC_WMI_EVENT_GUID,
> > > + eeepc_wmi_notify, NULL);
> > > + if (ACPI_FAILURE(status)) {
> > > + input_unregister_device(eeepc_wmi_input_dev);
> >
> > You need to free keymap as well. BTW, you need to do that after
> > unregistering device, because otherwise there is a chance someone
> > will try to modify keymap while device is still registered. I need
> > to fix a couple of drivers in input too...
> >
>
> Is the input_dev still there after calling input_unregister_device? I
> thought input_dev is going to be released after calling
> input_unregister_device. No?
>
Sorry I was unclear. Yes, input device is [most likely] released after
calling input_unregister_device(). What I was trying to say is
currently, if you free the keymap data before unregistering the device,
there is a window where userspace can issue EVIOCGKEYCODE or
EVIOCSKEYCODE and the driver will crash because keymap is NULL. But
changing the drivers to free sparse keymap after unregistering the
device is a bit unwieldy so I will take care of getting rid of that
window in the input core so that it is safe to call sparse_keymap_free()
before calling input_unregister_device().
Regarding your latest version of the patch - please move freeing keymap
before unregistering the device - they way you do it now you are using
invalid input device pointer.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
