Re: kern/50810: Kernel page fault trap in ugenclose()

[Nick Hudson <skrll%netbsd.org@localhost>]

The following reply was made to PR kern/50810; it has been noted by GNATS.

From: Nick Hudson <skrll%netbsd.org@localhost>
To: Andreas Gustafsson <gson%gson.org@localhost>, skrll%NetBSD.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Sat, 20 Feb 2016 17:12:24 +0000

 On 02/20/16 11:11, Andreas Gustafsson wrote:
 > Nick Hudson wrote:
 >>   The ioctl (USB_SET_ALTINTERFACE) is blatting edesc and making it fail
 >>   
 >>   Not sure why it's doing this before closing the endpoints.
 >>   
 >>   Maybe the ioctl should fail here?
 > I'm having some trouble following your comment.
 Sorry for being unclear....
 
 > - What do you mean by "blatting"?  The dictionary
 >    I consulted was not helpful.
 
 overwriting / erasing
 
 >
 > - In "why it's doing this", what does "it" refer to?
 
 sane-backends
 
 >
 > - At what point should the ioctl fail, and why would it make
 >    a difference?
 >
 > I see there is a comment in ugen_set_interface() saying /* XXX should
 > only do this after setting new altno has succeeded */, which seems to
 > point at a possible cause of the bug: If setting the new altno fails,
 > sce->edesc will be NULL, but sce->pipeh has not been cleared, so if it
 > was not NULL to begin, with we have the exact combination of
 > conditions (sce->pipeh != NULL && sce->edesc == NULL) that triggers
 > the crash in ugenclose().
 
 You got what I meant here.
 
 >
 > Also, there are several places checking for sce == NULL, for example:
 >
 >     sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
 >     if (sce == NULL)
 >
 > None of these make any sense - sce is a pointer into the middle of a
 > the ugen_softc struct, so it can *never* be NULL by definition.
 > Should they all say "sce->sc == NULL"?  What is the canonical way
 > of distinguishing a valid endpoint from an invalid one?
 
 I guess edesc and/or iface get populated with something after 
 ugen_set_interface/ugen_set_config
 
 Nick