netdev - Re: Subject: re-submit4 [ANNOUNCEMENT] NET: usb: sierra

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:	Thu, 22 Apr 2010 15:27:23 -0700
From:	Greg KH <greg@...ah.com>
To:	Dan Williams <dcbw@...hat.com>
Cc:	Elina Pasheva <epasheva@...rrawireless.com>,
	dbrownell@...rs.sourceforge.net, davem@...emloft.net,
	rfiler@...rrawireless.com, linux-usb@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: Subject: re-submit4 [ANNOUNCEMENT] NET: usb: sierra_net.c
 driver

On Thu, Apr 22, 2010 at 02:15:06PM -0700, Dan Williams wrote:
> On Thu, 2010-04-22 at 12:44 -0700, Greg KH wrote:
> > On Thu, Apr 22, 2010 at 12:19:33PM -0700, Elina Pasheva wrote:
> > > +static void sierra_net_send_cmd(struct usbnet *dev,
> > > +		u8 *cmd, int cmdlen, const char * cmd_name)
> > > +{
> > > +	struct sierra_net_data *priv = sierra_net_get_private(dev);
> > > +	int  status;
> > > +
> > > +	status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> > > +			USB_CDC_SEND_ENCAPSULATED_COMMAND,
> > > +			USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,	0,
> > > +			priv->ifnum, cmd, cmdlen, 0);
> > 
> > No timeout?
> > 
> > > +		ifnum = priv->ifnum;
> > > +		len = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > +				USB_CDC_GET_ENCAPSULATED_RESPONSE,
> > > +				USB_DIR_IN|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
> > > +				0, ifnum, buf, SIERRA_NET_USBCTL_BUF_LEN, 0);
> > 
> > No timeout?
> > 
> > > +		if (unlikely(len < 0)) {
> > > +			netdev_err(dev->net,
> > > +				"usb_control_msg failed, status %d\n", len);
> > 
> > You don't need "unlikely", this is an extreemly slow path here.
> > Also, what happens for a "short read"?  You don't handle that properly.
> 
> Is the code doc for usb_unlink_urb() in urb.c the best thing to look at
> for how to handle short reads?  I assume that involves checking if the
> returned error is -EREMOTEIO and if so, ignoring it?  I spent about an
> hour googling and poking around in the kernel sources and couldn't find
> much about how to handle short reads.  The only non-host-side driver
> that cares about EREMOTEIO is misc/rio500.c, the rest is all under host/
> or gadget/.

Well, if the return value is less than what you expect it to be,
something went wrong and you should error out.  Some of the calls handle
this properly in this driver, some do not.  Consistency is key :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html