netdev - Re: [RFC PATCH] usb: hcd: warn about URB buffers that are not DMA aligned and are about to be DMA mapped

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] [day] [month] [year] [list]

Message-ID: <3357770.hCSJD4DdXz@linux-5eaq.site>
Date:	Fri, 28 Jun 2013 14:58:23 +0200
From:	Oliver Neukum <oliver@...kum.org>
To:	Jussi Kivilinna <jussi.kivilinna@....fi>
Cc:	linux-usb@...r.kernel.org, Ming Lei <tom.leiming@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Network Development <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH] usb: hcd: warn about URB buffers that are not DMA aligned and are about to be DMA mapped

On Friday 28 June 2013 15:39:27 Jussi Kivilinna wrote:
> On 16.06.2013 13:35, Jussi Kivilinna wrote:
> > On 16.06.2013 11:21, Oliver Neukum wrote:
> >> On Saturday 15 June 2013 16:22:30 Jussi Kivilinna wrote:
> >>
> >>> Hm.. rethink this a bit.
> >>>
> >>> Transfer buffer might be dma aligned but shorter than cacheline and end of cacheline
> >>> used as something else. Manual alignment by host driver does not catch that
> >>> or fix that.
> >>> So, yes.. dma mapping should work with unaligned buffers, but maybe the actual
> >>> problem is multiple buffers from same cacheline.
> >>
> >> The buffers kmalloc() returns are OK in that regard. A driver that uses
> >> a buffer for anything but buffering is buggy.
> > 
> > Ok, I'll look at that direction. Thanks.
> > 
> 
> So if I understood correctly, drivers that allocate these as part of larger structures (struct *_device etc) are doing wrong thing and are potentially buggy. And this is because cachelines of buffers can be DMA mapped after usb_submit_urb() and editing same cacheline while URB is in-flight can therefore be hazardous.
> 
> I checked setup_packet and transfer_buffer usage of some drivers in 3.9.8 and made some observations. Should these be fixed?
> 
> URB setup_packet and transfer_buffer part of same structure (might share same cacheline for same URB):
>  * iforce:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/input/joystick/iforce/iforce-usb.c#L173
>   - http://lxr.linux.no/linux+v3.9.8/drivers/input/joystick/iforce/iforce.h#L101

Buggy

>  * usbvision:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/media/usb/usbvision/usbvision-core.c#L1445
>   - http://lxr.linux.no/linux+v3.9.8/drivers/media/usb/usbvision/usbvision.h#L366

Buggy

>  * catc:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L499
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L500
>   - ctrl_buf, ctrl_dr: http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/catc.c#L162
>  * rtl8150:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/rtl8150.c#L200
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/usb/rtl8150.c#L128
>  * rt2x000usb:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rt2x00/rt2x00usb.c#L212
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rt2x00/rt2x00usb.c#L169
>  * rtl8187:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtl818x/rtl8187/dev.c#L156
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtl818x/rtl8187/dev.c#L130
>  * uss720:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/usb/misc/uss720.c#L176
>   - http://lxr.linux.no/linux+v3.9.8/drivers/usb/misc/uss720.c#L72

Well, I didn't look through them all, but we must assume that they are buggy.

> URB transfer_buffer array (transfer buffers preloaded as array, element size less than cacheline):
>  * rtlwifi:
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/usb.c#L152
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/wifi.h#L1859
>   - http://lxr.linux.no/linux+v3.9.8/drivers/net/wireless/rtlwifi/usb.c#L980

Good catch.

This is a very large number. I suggest you split it up.

	Regards
		Oliver

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