linux-kernel - Re: [PATCH] evdev: Release eventual input device grabs when getting disconnected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080331204221.GA19953@kroah.com>
Date:	Mon, 31 Mar 2008 13:42:21 -0700
From:	Greg KH <greg@...ah.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Kay Sievers <kay.sievers@...y.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Bj?rn Steinbrink <B.Steinbrink@....de>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Johannes Berg <johannes@...solutions.net>,
	Jiri Kosina <jkosina@...e.cz>
Subject: Re: [PATCH] evdev: Release eventual input device grabs when
	getting disconnected

On Mon, Mar 31, 2008 at 02:01:20PM -0400, Dmitry Torokhov wrote:
> On Mon, Mar 31, 2008 at 10:28:13AM -0700, Greg KH wrote:
> > On Mon, Mar 31, 2008 at 02:15:39AM -0400, Dmitry Torokhov wrote:
> > > Hi Linus,
> > > 
> > > On Sunday 30 March 2008, Linus Torvalds wrote:
> > > > 
> > > > On Sun, 30 Mar 2008, Bj?rn Steinbrink wrote:
> > > > >
> > > > > I can't reproduce the bug on my UP box and currently can't afford
> > > > > crashing my SMP box (all the oopses seem to come from SMP kernels, so I
> > > > > guess it needs SMP to crash), so while this doesn't show any new
> > > > > problems, I can't tell whether it actually fixes anything. Testers
> > > > > welcome!
> > > > 
> > > > Ok, I applied this because I will do an -rc8 today or tomorrow, but I 
> > > > really really hope somebody can figure out what made this all start to 
> > > > trigger. It does smell like some core device layer change, because we do 
> > > > not seem to have a lot of changes since 2.6.24 in evdev.c and input.c that 
> > > > seem relevant.
> > > > 
> > > > Greg, are there any refcounting changes that would cause the input devices 
> > > > to be free'd earlier or something?
> > > > 
> > > 
> > > The following commit changed lifetime runes on kobjects breaking input:
> > > 
> > > commit 0f4dafc0563c6c49e17fe14b3f5f356e4c4b8806
> > > Author: Kay Sievers <kay.sievers@...y.org>
> > > Date:   Wed Dec 19 01:40:42 2007 +0100
> > > 
> > >     Kobject: auto-cleanup on final unref
> > > 
> > >     We save the current state in the object itself, so we can do proper
> > >     cleanup when the last reference is dropped.
> > > 
> > >     If the initial reference is dropped, the object will be removed from
> > >     sysfs if needed, if an "add" event was sent, "remove" will be send, and
> > >     the allocated resources are released.
> > > 
> > >     This allows us to clean up some driver core usage as well as allowing us
> > >     to do other such changes to the rest of the kernel.
> > > 
> > >     Signed-off-by: Kay Sievers <kay.sievers@...y.org>
> > >     Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>
> > > 
> > > Before we dropped reference to kobject's parent only when child kobject
> > > was released (in kobject_cleanup). The changeset above moves the release
> > > to kobject_del() which is way too early in my opinion. The kobject is only
> > > marked for deletion at that time, not really deleted.
> > 
> > It was "deleted" from sysfs, and should have never been used again by
> > any callers.  If the reference count was dropped to zero with this call,
> > it would be cleaned up as well, it seems that you were assuming that it
> > would not be?  Perhaps you just need to grab another reference as this
> > would have caused you problems without this change anyway, but without
> > slab debugging, you never saw it.
> > 
> 
> Greg, please look at the change again. Before kobject_put(kobj->parent)
> was done in kobject_cleanup() and so the parent would only be freed when
> all its children are gone. Now parent is deleted early, even if its
> children are still referenced by other users. This is lifetime rule
> change and should really be announced as such.

Ugh, this was done because of scsi, they required that if you really
were deleting the parent, you wanted it gone.

> If this change it intentional and is here to stay then I will just grab
> the references myself, although I wonder what else might be broken by
> it.

Yes, if you need those references, you are going to have to hold on for
them, the kobject layer will not keep them around.  It now is a "does
what you ask for" type model :)

I fail to see where this affects the input code though, in glancing at
it, it looks like you are doing things properly.  Kay, any thoughts
here, I think you looked at the kobject input layer interaction in the
past.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/