On Mon, Aug 07, 2006 at 03:04:36PM -0400, Dmitry Torokhov wrote: > On 8/7/06, Zephaniah E. Hull <[email protected]> wrote: > >On Mon, Aug 07, 2006 at 01:35:50PM -0400, Dmitry Torokhov wrote: > >> Hi, > >> > >> On 8/7/06, Zephaniah E. Hull <[email protected]> wrote: > >> > if (evdev->open) { > >> > input_close_device(handle); > >> > wake_up_interruptible(&evdev->wait); > >> >- list_for_each_entry(list, &evdev->list, node) > >> >+ list_for_each_entry_safe(list, next, &evdev->list, node) > >> > kill_fasync(&list->fasync, SIGIO, POLL_HUP); > >> > >> NAK. kill_fasync does not affect the list state so using _safe does > >> not buy us anything. > > > >Sorry, but you're wrong. > > > >Immediately before the kill_fasync call list->node.next is a valid > >pointer, immediately afterwords it is 0x100100, which happens to be > >list_poison. kill_fasync is triggering a close somehow, evdev_close > >deletes that element of the list, which poisons the next value, which > >can make us crash and burn. > > > >I have a 100% reproducible crash case, which is fixed by the change. > > > >If kill_fasync shouldn't be making it close that's another issue, but at > >the moment it is and this is a fairly non-invasive change which fixes > >it. > > > > Unfortunately it does not really fix the problem, it just papers over > the issue. The crash will still happen if for some reason > evdev_release runs at a bad moment. Almost agreed, it papers over the lack of locking, however most forms of locking may need this to avoid a deadlock anyhow. (I don't know the semantics of kill_fasync, and thus I don't know if attempting to take a lock in the close that is held here would deadlock, or if execution would move back to here, but I have a nasty feeling that it's the former.) That said, at the moment, we have a 100% guaranteed oops on rmmod usbhid if something has the device open, which is bad. Zephaniah E. Hull. -- 1024D/E65A7801 Zephaniah E. Hull <[email protected]> 92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801 CCs of replies from mailing lists are requested. "I would rather spend 10 hours reading someone else's source code than 10 minutes listening to Musak waiting for technical support which isn't." (By Dr. Greg Wettstein, Roger Maris Cancer Center)
Attachment:
signature.asc
Description: Digital signature
- References:
- [patch] Crash on evdev disconnect.
- From: "Zephaniah E. Hull" <[email protected]>
- Re: [patch] Crash on evdev disconnect.
- From: "Dmitry Torokhov" <[email protected]>
- Re: [patch] Crash on evdev disconnect.
- From: "Zephaniah E. Hull" <[email protected]>
- Re: [patch] Crash on evdev disconnect.
- From: "Dmitry Torokhov" <[email protected]>
- [patch] Crash on evdev disconnect.
- Prev by Date: Re: [PATCH] x86_64: Make NR_IRQS configurable in Kconfig
- Next by Date: Re: [RFC, PATCH 0/5] Going forward with Resource Management - A cpu controller
- Previous by thread: Re: [patch] Crash on evdev disconnect.
- Next by thread: [PATCH 1/3] kthread: convert arch/i386/kernel/apm.c
- Index(es):
 |