netdev - Re: [PATCH 1/1] SMSC LAN9500 USB2.0 10/100 ethernet adapter 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:	Tue, 09 Sep 2008 15:30:10 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Greg KH <greg@...ah.com>
Cc:	Steve Glendinning <steve.glendinning@...c.com>,
	netdev@...r.kernel.org, Ian Saturley <ian.saturley@...c.com>,
	Catalin Marinas <catalin.marinas@....com>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	linux-usb@...r.kernel.org
Subject: Re: [PATCH 1/1] SMSC LAN9500 USB2.0 10/100 ethernet adapter driver

On Tue, 2008-09-09 at 07:02 -0700, Greg KH wrote:
> On Tue, Sep 09, 2008 at 02:19:47PM +0100, Ben Hutchings wrote:
> > On Tue, 2008-09-09 at 12:36 +0100, Steve Glendinning wrote:
> > [...]
> > > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> > > new file mode 100644
> > > index 0000000..60ffd90
> > > --- /dev/null
> > > +++ b/drivers/net/usb/smsc95xx.c
> > [...]
> > > +static int smsc95xx_read_reg(struct usbnet *dev, u32 index, u32 *data)
> > > +{
> > > +	u32 *buf = kmalloc(4, GFP_KERNEL);
> > > +	int ret;
> > > +
> > > +	BUG_ON(!dev);
> > > +
> > > +	if (!buf)
> > > +		return -ENOMEM;
> > > +
> > > +	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > > +		USB_VENDOR_REQUEST_READ_REGISTER,
> > > +		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> > > +		00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
> > > +
> > > +	if (unlikely(ret < 0))
> > > +		SMSC_WARNING("Failed to read register index 0x%08x", index);
> > > +
> > > +	le32_to_cpus(buf);
> > > +	*data = *buf;
> > > +	kfree(buf);
> > > +
> > > +	return ret;
> > > +}
> > 
> > Why are you allocating a buffer on the heap?  What's wrong with
> 
> USB requires data to be allocated off of the heap when you use it to
> send or receive data.

I don't really know USB (it's not very useful for 1G/10G networking :-)
which is why I asked.  Is this because the data may be transferred by
DMA and the stack might not be DMA-mappable?

I'd be inclined to allocate a persistent buffer for register reads and
writes, but then that seems to introduce the need for another lock.
Presumably the heap allocation is reckoned to add very little overhead
compared to the inherent cost of synchronous USB requests?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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