linux-kernel - Re: [RFC/PATCH v2 0/3] add gadget quirk to adapt f

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: <52713584.60804@linux.intel.com>
Date:	Wed, 30 Oct 2013 09:36:20 -0700
From:	David Cohen <david.a.cohen@...ux.intel.com>
To:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
CC:	"balbi@...com" <balbi@...com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC/PATCH v2 0/3] add gadget quirk to adapt f_fs for DWC3

On 10/29/2013 03:47 PM, Paul Zimmerman wrote:
>> From: David Cohen
>> Sent: Tuesday, October 29, 2013 2:53 PM
>>
>> These patches are a proposal to add gadget quirks in an immediate objective to
>> adapt f_fs when using DWC3 controller. But the quirk solution is generic and
>> can be used by other controllers to adapt gadget functions to their
>> non-standard restrictions.
>>
>> This change is necessary to make Android's adbd service to work on Intel
>> Merrifield with f_fs instead of out-of-tree android gadget.
>>
>> ---
>> David Cohen (3):
>>    usb: gadget: add quirks field to struct usb_gadget
>>    usb: ffs: check quirk to pad epout buf size when not aligned to
>>      maxpacketsize
>>    usb: dwc3: add quirk USB_GADGET_QUIRK_EP_OUT_ALIGNED_SIZE to gadget
>>      driver
>>
>>   drivers/usb/dwc3/gadget.c  |  1 +
>>   drivers/usb/gadget/f_fs.c  | 17 +++++++++++++++++
>>   include/linux/usb/gadget.h |  5 +++++
>>   3 files changed, 23 insertions(+)
>
> Wouldn't it be simpler and safer to just do this unconditionally? Sure,
> you need it for DWC3 because the controller refuses to do an OUT transfer
> at all if the transfer size is less than maxpacketsize. But it's possible
> that other controllers allow the transfer, and it works in most cases,
> but if an error occurs and the host sends too much data, they could
> overrun the buffer and crash your device.
>
> For example, the DWC2 databook says "For OUT transfers, the Transfer
> Size field in the endpoint's Transfer Size register must be a multiple
> of the maximum packet size of the endpoint". But I don't think the
> controller enforces that, it is up to the programmer to do the right
> thing. So that controller probably needs this quirk also. There could be
> more like that which we don't know about.

Unfortunately DWC2 is a bad example... the driver couldn't even get out
of staging. If the author was reckless to ignore this restriction (s)he
should fix.
But I don't have enough data to tell it's better to waste everybody's
memory in this case in favor of DWC3. I'd still stick with the
exception.

>
> So unless the buffer allocation code is in a real fast path, I would
> suggest to just do the aligned buffer allocation always.

This code would affect embedded devices which value too much memory
consumption (and performance on handling it!). IMO we'd need to be more
careful prior to take such decision.

Br, David Cohen
--
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/