linux-kernel - Re: debugging oops after disconnecting Nexio USB touchscreen

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]

Date:	Wed, 2 Dec 2009 09:52:08 +0100
From:	Ondrej Zary <linux@...nbow-software.org>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: debugging oops after disconnecting Nexio USB touchscreen

On Tuesday 01 December 2009, Alan Stern wrote:
> On Tue, 1 Dec 2009, Ondrej Zary wrote:
> > On Monday 30 November 2009, Alan Stern wrote:
> > > On Mon, 30 Nov 2009, Ondrej Zary wrote:
> > > > It does not make much sense to me but I think that it crashes iside
> > > > this list manipulation:
> > > >
> > > >         prev = ehci->async;
> > > >         while (prev->qh_next.qh != qh)
> > > >                 prev = prev->qh_next.qh;
> > >
> > > Yes, it's crashing in the "while" test because prev is NULL.  This
> > > means the code is looking for qh in the async list but not finding it.
> > > That's supposed to be impossible.
> > >
> > > The assembly code is peculiar because it includes stuff that isn't in
> > > the source code!  For example, right at this point (after the end of
> > > the loop) there's a test to see whether prev is NULL.  Where could that
> > > have come from?  Do you have any idea?
> >
> > I'm not sure, I might did something wrong and left it there from my
> > previous debugging attempt.
>
> Okay.  But it was part of the reason you had difficulty matching up the
> ehci-hcd.s file with the crash code dump.
>
> > > >         prev->hw_next = qh->hw_next;
> > > >         prev->qh_next = qh->qh_next;
> > > >         wmb ();
> > >
> > > These lines aren't reached.
> > >
> > > Does this happen every time you disconnect the Nexio?
> >
> > The crash happens almost always when disconnecting the touchscreen.
> > When booted without X, it often survives the first disconnect.
>
> Then it will be easy to recreate the problem.  :-)

Yes, seems so. But it's not that easy (see below).

> > > You can try patching that loop.  If prev is NULL then print an error
> > > message in the log, including the value of qh and the value of
> > > ehci->async, and jump past the following three statements.
> > >
> > > With that change the system shouldn't crash, although khubd might hang.
> > > But we still need to find out how this could have happened.  Try
> > > collecting a usbmon trace while running the test; then let's compare
> > > the usbmon output with the error messages in the log.
> >
> > gcc version is: gcc (Debian 4.3.4-6) 4.3.4
> >
> > Tried something like that before but it did not help at all.
> > The check is not triggered and it still oopses. Now it looks like this:
> >
> >         qh->qh_state = QH_STATE_UNLINK;
> >         ehci->reclaim = qh = qh_get (qh);
> >
> >         prev = ehci->async;
> >         if (!prev) {
> >                 printk("prev is NULL, qh=%p, ehci->async=%p\n", qh,
> > ehci->async); goto after;
> >         }
>
> [*] >         while (prev->qh_next.qh != qh) {
>
> >                 if (!prev) {
> >                         printk("prev is NULL, qh=%p, ehci->async=%p\n",
> > qh, ehci->async); goto after;
> >                 }
> >                 prev = prev->qh_next.qh;
> >         }
> >
> >         prev->hw_next = qh->hw_next;
> >         prev->qh_next = qh->qh_next;
> >         wmb ();
> >
> > after:
>
> No, that is wrong.  The [*] line can still perform an invalid
> dereference.  You need to move the inside test to the end of the loop,
> after the assignment statement.  Or else do this:

Oops, such a stupid mistake. Fixed now:
        prev = ehci->async;
        if (!prev) {
                printk("prev is NULL, qh=%p, ehci->async=%p\n", qh, ehci->async);
                goto after;
        }
        while (prev->qh_next.qh != qh) {
                prev = prev->qh_next.qh;
                if (!prev) {
                        printk("prev is NULL, qh=%p, ehci->async=%p\n", qh, ehci->async);
                        goto after;
                }
        }

        prev->hw_next = qh->hw_next;
        prev->qh_next = qh->qh_next;
        wmb ();
after:


It shows "prev is NULL, qh=f6581080, ehci->async=f6581000".

The problem is that activating usbmon causes the problem to disappear. No errors
in maybe 15 attempts. It appeared on 2nd attempt after unloading usbmon.

-- 
Ondrej Zary
--
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/