linux-kernel - Re: [PATCH] PXA27x UDC 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]

Message-Id: <200707011049.42333.david-b@pacbell.net>
Date:	Sun, 1 Jul 2007 10:49:41 -0700
From:	David Brownell <david-b@...bell.net>
To:	linux-arm-kernel@...ts.arm.linux.org.uk
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Rodolfo Giometti <giometti@...eenne.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PXA27x UDC driver.

On Sunday 01 July 2007, Russell King - ARM Linux wrote:
> >  
> > +#ifdef CONFIG_USB_GADGET_PXA27X
> > +#define DEV_CONFIG_SUBSET
> > +#endif
> > +
> 
> Bad - can we not make this runtime dependent?

The #define controls which descriptors are statically linked
into every object.  I suppose someone could submit a patch which
does that differently ... I'm not keen on having lots of "will
never use" data bloating any driver, but it could be moved to
__initdata, at the cost of adding code to kmalloc (and kfree)
all the individual descriptors.  Restricting code bloat to init
sections would still be worse than no bloat...

> > +static void *pxa27x_ep_alloc_buffer(struct usb_ep *_ep, unsigned bytes,
> > +				    dma_addr_t * dma, unsigned int gfp_flags)
> > +{
> > +	...
> 
> Err, no.  There's a perfectly good replacement.  DMA pools.  Please use
> the provided infrastructure instead of inventing your own solution.

This was cloned from code which predates dma pools.  :)

Regardless, I'm planning to remove that interface from the gadget stack.
It's utility is far exceeded by the number of controller driver bugs
in that area.  Plus, if any gadget driver needs such a mechanism, the
dma_pool stuff that now exists could be called directly.

> Pointers which are cleared are set to NULL not zero.  Zero is an integer.
> NULL is a pointer.  Don't confuse the two.

ISTR this class of error would be reported by sparse ("make check").

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