On Tue, Mar 18, 2008 at 11:03:42AM -0700, Pete Zaitcev wrote: > On Tue, 18 Mar 2008 09:31:25 -0400, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > On Mon, Mar 17, 2008 at 11:48:07PM -0700, Pete Zaitcev wrote: > > > > If a device was grabbed through evdev and then became disconnected, > > > we oops on close. This happens because input_release_device uses memory > > > which was freed. > > > Could you tell me what memory is freed? > > The input_dev is freed. We only have two structures involved, > really: evdev and input_dev. The rest is either immaterial like > evdev_client, or embedded into the two like input_handle and device. > > It's freed by this route: > hidinput_disconnect > input_unregister_device > evdev_disconnect --- marks evdev->exists = 0 > device_unregister > put_device -- kobject_put -- kref_put -- kobject_cleanup > input_dev_release > > > [] As far as I understand the > > the input_dev structure shold be pinned in memory by the driver > > core since we have this link: > > > > evdev->dev.parent = &input_dev->dev; > > > > This should guarantee that input_device is not gone until we > > call evdev_free which should be done way after the ungrab. > > I don't think anyone checks this, unless the accompaining refcount > is set. > > Honestly, I didn't consider the implications for the integrity > of sysfs. It seems like there's no extra memory leak with my > patch, that's the only thing I can promise. Maybe you want to > poke Greg about it. > I dont oppose your patch, I am just trying to understand why it is needed because driver core should pin the parent device as far as I understand and if this does not happen there are other issues in input core that need to be taken care of. >From what I see we should be automatically taking the reference to parent kobject in kobject_add_internal(): parent = kobject_get(kobj->parent); perent is set in kobject_add_varg(): kobj->parent = parent; .. which is called from kobject_add() which is called from device_add(). Hmm, I wonder if obsolete sysfs links mess up proper parenting data... I dont think I have obsolete links set up on any of my boxes, can you see if oops goes away if you disable deprectated sysfs? If so then instead of checking exist flag we need explicitely take reference to the parent input_dev. Although I wonder what other subsystems might be affected. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
