[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-usb-devel
Subject:    Re: [linux-usb-devel] RFC: (as246) Allocate usb_interface structures
From:       David Brownell <david-b () pacbell ! net>
Date:       2004-04-19 20:02:04
Message-ID: 4084303C.5070406 () pacbell ! net
[Download RAW message or body]

Alan Stern wrote:
> 
> Several issues of varying importance came up while I was working on this.
> 
>     (1) The locking in devices.c needs to be fixed.  The USB subsystem
> rwsem should be held over a much larger part of the code.  That should be
> pretty easy to do.  I don't think it's necessary to use usb_get_dev or to
> lock usbdev->serialize, but someone should verify this.

I'm thinking the plan of record for this one is that dev->serialize
should be held when accessing dev->children[] ... though that'll take
a while to phase in.


>     (2) The exact relationships among usbdev->state, usbdev->actconfig,
> and the actual state of the physical device aren't clear.  Various error
> scenarios can leave ->state disagreeing with the device or ->state equal
> to USB_STATE_CONFIGURED even though ->actconfig is NULL.  This needs more
> thought.

Are you sure about CONFIGURED without an actconfig?  It certainly
doesn't need to be that way; the current 2.6.6 code doesn't have
that problem, it updates state to ADDRESSED when clearing actconfig
if the original state is CONFIGURED.


>     (3) I decided that it wasn't really necessary to make a deep copy of
> the altsetting and endpoint structures for the dynamic interfaces (those
> structures are read-only), so this patch just copies some pointers.  
> Unfortunately this opens up a hole:  A user program can pin an interface
> attribute file and use it to try reading an altsetting structure even
> after the device has been disconnected and the structure deallocated.  I
> added a test to the attribute read method to prevent dereferencing a NULL
> pointer, but there remained the possibility of a race between using the
> pointer and setting it to NULL.  So I added another semaphore to handle
> _that_.  All this makes those attribute methods a lot bigger than they
> used to be.

Seems like you resolved this to your satisfaction in as246c, using
the kref instead.

Other than testing, and (1) above which is deeper khubd surgery,
it looks like those issues are now gone.  Any other issues?


I skimmed as246c and didn't notice any particular issues as I
skimmed.  There were a few whitespace changes bulking it up, but
those weren't going to make trouble!

- Dave





-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
[prev in list] [next in list] [prev in thread] [next in thread]