linux-kernel - Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

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, 12 Nov 2014 14:58:04 -0800
From:	"Sean O. Stalley" <sean.stalley@...el.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Stephanie Wallick <stephanie.s.wallick@...el.com>,
	linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org, devel@...verdev.osuosl.org
Subject: Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

Sorry, for got to respond to a couple comments. See responses below.

On Wed, Nov 12, 2014 at 01:40:21PM -0800, Sean O. Stalley wrote:
> Thanks for reviewing. My responses are inline.
> 
> Greg has asked that we clean up this code internally before we
> send out another patchset to the mailing list. I will address
> the issues you pointed out, but it may be a while before you see
> another patchset.
> 
> Thanks Again,
> Sean
> 
> On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote:
> > On Mon, 10 Nov 2014, Stephanie Wallick wrote:
> > > +	/* If the endpoint isn't activated, we can't enqueue anything. */
> > > +	if (MAUSB_EP_HANDLE_UNASSIGNED == ep->ep_handle_state) {
> > > +		mausb_err(&mhcd, "%s: endpoint handle unassigned\n", __func__);
> > > +		return -EPIPE;
> > > +	}
> > > +
> > > +	if (USB_SPEED_FULL != urb->dev->speed) /* suppress checks */
> > > +		ep->max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> > 
> > What happens to full-speed devices?  Don't they have maxpacket values?
> > 

This was part of a work-around.
Per the MA spec (section 7.3.2.2), wMaxPacketSize should be initially
set to 8 for EP0 of FS devices. The usbcore sets it to 64.

This makes sure the EPHandleReq Packet has the per-spec values.

> > > +	if (ret < 0) {
> > > +		mausb_err(&mhcd, "urb enqueue failed: error %d\n", ret);
> > > +		usb_hcd_unlink_urb_from_ep(hcd, urb);
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* get usb device and increment reference counter */
> > > +	if (!mhcd.udev) {
> > > +		mhcd.udev = urb->dev;
> > > +		usb_get_dev(mhcd.udev);
> > > +	}
> > 
> > What happens if more than one device is in use at a time?
> > 

This is wrong. This call should be in mausb_internal_alloc_dev(). Will fix.
--
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/