NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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

From: Andreas Gustafsson <gson%gson.org@localhost>
To: Nick Hudson <skrll%netbsd.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/50810: Kernel page fault trap in ugenclose()
Date: Mon, 22 Feb 2016 09:55:05 +0200

 Nick Hudson wrote:
 > I got rid of these pointless conditionals
 
 I'm just wondering if whoever put them in in the first place intended
 to check for a valid endpoint, in which case some of them might need to
 be replaced by working checks for a valid endpoint rather than just
 removed.
 
 > >   >  > 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
 > >   
 > >   I'm sorry, but I fail to see the connection between my question and
 > >   your answer.
 > 
 > I was really answering your second question and not the first. Both 
 > edesc and iface will
 > be non-null if the endpoint is valid.
 
 That still doesn't tell me which one the code should be checking.
 I guess the answer is that there is no canonical way, and that
 each function is just checking whatever field it is going to
 dereference (if it is checking at all).
 
 > Does this diff help? 
 
 Building with UGENDEBUG enabled, I got:
 
   /ssd/current/src/sys/dev/usb/ugen.c:274:1: error: no previous prototype for 'ugen_clear_endpoints' [-Werror=missing-prototypes]
    ugen_clear_endpoints(struct ugen_softc *sc)
    ^
   cc1: all warnings being treated as errors
   *** [ugen.o] Error code 1
 
 After I worked around that, the patch did prevent the crash, and not
 only that, the scan even succeeded, which it didn't with my own
 initial patch.  Thank you!
 
 > I'm not sure of retaining selinfo and the cv is correct for
 > ugen_set_interface.
 
 I'm not sure, either.
 -- 
 Andreas Gustafsson, gson%gson.org@localhost
 


Home | Main Index | Thread Index | Old Index