| 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
| ||
|
Message-id: <561E1477.20401@samsung.com>
Date: Wed, 14 Oct 2015 10:38:15 +0200
From: Robert Baldyga <r.baldyga@...sung.com>
To: balbi@...com
Cc: gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, andrzej.p@...sung.com,
b.zolnierkie@...sung.com, stable@...r.kernel.org
Subject: Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling
Hi Felipe,
On 09/24/2015 06:49 PM, Felipe Balbi wrote:
> On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:
>> Move function parameters to struct f_sourcesink to make them per instance
>> instead of having them as global variables. Since we can have multiple
>> instances of USB function we also want to have separate set of parameters
>> for each instance.
>>
>> Cc: <stable@...r.kernel.org> # 3.10+
>> Signed-off-by: Robert Baldyga <r.baldyga@...sung.com>
>
> why do you think this is stable material ? Looks like it's not
> fixing anything to me; just implementing a brand new requirement.
I will not insist on stable backporting of this patch, as it's actually
not very important fix (especially in case of sourcesink function).
Should I resend this patch without CC:stable?
Thanks,
Robert
>
>> ---
>> drivers/usb/gadget/function/f_sourcesink.c | 120 +++++++++++++++--------------
>> 1 file changed, 62 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
>> index 1353465..128abfb 100644
>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>> @@ -50,6 +50,13 @@ struct f_sourcesink {
>> struct usb_ep *iso_in_ep;
>> struct usb_ep *iso_out_ep;
>> int cur_alt;
>> +
>> + unsigned pattern;
>> + unsigned isoc_interval;
>> + unsigned isoc_maxpacket;
>> + unsigned isoc_mult;
>> + unsigned isoc_maxburst;
>> + unsigned buflen;
>> };
>>
>> static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
>> @@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
>> return container_of(f, struct f_sourcesink, function);
>> }
>>
>> -static unsigned pattern;
>> -static unsigned isoc_interval;
>> -static unsigned isoc_maxpacket;
>> -static unsigned isoc_mult;
>> -static unsigned isoc_maxburst;
>> -static unsigned buflen;
>> -
>> /*-------------------------------------------------------------------------*/
>>
>> static struct usb_interface_descriptor source_sink_intf_alt0 = {
>> @@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
>>
>> static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
>> {
>> - return alloc_ep_req(ep, len, buflen);
>> + struct f_sourcesink *ss = ep->driver_data;
>> +
>> + return alloc_ep_req(ep, len, ss->buflen);
>> }
>>
>> void free_ep_req(struct usb_ep *ep, struct usb_request *req)
>> @@ -357,22 +359,22 @@ autoconf_fail:
>> goto autoconf_fail;
>>
>> /* sanity check the isoc module parameters */
>> - if (isoc_interval < 1)
>> - isoc_interval = 1;
>> - if (isoc_interval > 16)
>> - isoc_interval = 16;
>> - if (isoc_mult > 2)
>> - isoc_mult = 2;
>> - if (isoc_maxburst > 15)
>> - isoc_maxburst = 15;
>> + if (ss->isoc_interval < 1)
>> + ss->isoc_interval = 1;
>> + if (ss->isoc_interval > 16)
>> + ss->isoc_interval = 16;
>> + if (ss->isoc_mult > 2)
>> + ss->isoc_mult = 2;
>> + if (ss->isoc_maxburst > 15)
>> + ss->isoc_maxburst = 15;
>>
>> /* fill in the FS isoc descriptors from the module parameters */
>> - fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
>> - 1023 : isoc_maxpacket;
>> - fs_iso_source_desc.bInterval = isoc_interval;
>> - fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
>> - 1023 : isoc_maxpacket;
>> - fs_iso_sink_desc.bInterval = isoc_interval;
>> + fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
>> + 1023 : ss->isoc_maxpacket;
>> + fs_iso_source_desc.bInterval = ss->isoc_interval;
>> + fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
>> + 1023 : ss->isoc_maxpacket;
>> + fs_iso_sink_desc.bInterval = ss->isoc_interval;
>>
>> /* allocate iso endpoints */
>> ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
>> @@ -394,8 +396,8 @@ no_iso:
>> ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>> }
>>
>> - if (isoc_maxpacket > 1024)
>> - isoc_maxpacket = 1024;
>> + if (ss->isoc_maxpacket > 1024)
>> + ss->isoc_maxpacket = 1024;
>>
>> /* support high speed hardware */
>> hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> @@ -406,15 +408,15 @@ no_iso:
>> * We assume that the user knows what they are doing and won't
>> * give parameters that their UDC doesn't support.
>> */
>> - hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
>> - hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
>> - hs_iso_source_desc.bInterval = isoc_interval;
>> + hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
>> + hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
>> + hs_iso_source_desc.bInterval = ss->isoc_interval;
>> hs_iso_source_desc.bEndpointAddress =
>> fs_iso_source_desc.bEndpointAddress;
>>
>> - hs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
>> - hs_iso_sink_desc.wMaxPacketSize |= isoc_mult << 11;
>> - hs_iso_sink_desc.bInterval = isoc_interval;
>> + hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
>> + hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
>> + hs_iso_sink_desc.bInterval = ss->isoc_interval;
>> hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>>
>> /* support super speed hardware */
>> @@ -428,21 +430,21 @@ no_iso:
>> * We assume that the user knows what they are doing and won't
>> * give parameters that their UDC doesn't support.
>> */
>> - ss_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
>> - ss_iso_source_desc.bInterval = isoc_interval;
>> - ss_iso_source_comp_desc.bmAttributes = isoc_mult;
>> - ss_iso_source_comp_desc.bMaxBurst = isoc_maxburst;
>> - ss_iso_source_comp_desc.wBytesPerInterval =
>> - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1);
>> + ss_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
>> + ss_iso_source_desc.bInterval = ss->isoc_interval;
>> + ss_iso_source_comp_desc.bmAttributes = ss->isoc_mult;
>> + ss_iso_source_comp_desc.bMaxBurst = ss->isoc_maxburst;
>> + ss_iso_source_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
>> + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
>> ss_iso_source_desc.bEndpointAddress =
>> fs_iso_source_desc.bEndpointAddress;
>>
>> - ss_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
>> - ss_iso_sink_desc.bInterval = isoc_interval;
>> - ss_iso_sink_comp_desc.bmAttributes = isoc_mult;
>> - ss_iso_sink_comp_desc.bMaxBurst = isoc_maxburst;
>> - ss_iso_sink_comp_desc.wBytesPerInterval =
>> - isoc_maxpacket * (isoc_mult + 1) * (isoc_maxburst + 1);
>> + ss_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
>> + ss_iso_sink_desc.bInterval = ss->isoc_interval;
>> + ss_iso_sink_comp_desc.bmAttributes = ss->isoc_mult;
>> + ss_iso_sink_comp_desc.bMaxBurst = ss->isoc_maxburst;
>> + ss_iso_sink_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
>> + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
>> ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>>
>> ret = usb_assign_descriptors(f, fs_source_sink_descs,
>> @@ -482,11 +484,11 @@ static int check_read_data(struct f_sourcesink *ss, struct usb_request *req)
>> struct usb_composite_dev *cdev = ss->function.config->cdev;
>> int max_packet_size = le16_to_cpu(ss->out_ep->desc->wMaxPacketSize);
>>
>> - if (pattern == 2)
>> + if (ss->pattern == 2)
>> return 0;
>>
>> for (i = 0; i < req->actual; i++, buf++) {
>> - switch (pattern) {
>> + switch (ss->pattern) {
>>
>> /* all-zeroes has no synchronization issues */
>> case 0:
>> @@ -518,8 +520,9 @@ static void reinit_write_data(struct usb_ep *ep, struct usb_request *req)
>> unsigned i;
>> u8 *buf = req->buf;
>> int max_packet_size = le16_to_cpu(ep->desc->wMaxPacketSize);
>> + struct f_sourcesink *ss = ep->driver_data;
>>
>> - switch (pattern) {
>> + switch (ss->pattern) {
>> case 0:
>> memset(req->buf, 0, req->length);
>> break;
>> @@ -549,7 +552,7 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
>> case 0: /* normal completion? */
>> if (ep == ss->out_ep) {
>> check_read_data(ss, req);
>> - if (pattern != 2)
>> + if (ss->pattern != 2)
>> memset(req->buf, 0x55, req->length);
>> }
>> break;
>> @@ -598,15 +601,16 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
>> if (is_iso) {
>> switch (speed) {
>> case USB_SPEED_SUPER:
>> - size = isoc_maxpacket * (isoc_mult + 1) *
>> - (isoc_maxburst + 1);
>> + size = ss->isoc_maxpacket *
>> + (ss->isoc_mult + 1) *
>> + (ss->isoc_maxburst + 1);
>> break;
>> case USB_SPEED_HIGH:
>> - size = isoc_maxpacket * (isoc_mult + 1);
>> + size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
>> break;
>> default:
>> - size = isoc_maxpacket > 1023 ?
>> - 1023 : isoc_maxpacket;
>> + size = ss->isoc_maxpacket > 1023 ?
>> + 1023 : ss->isoc_maxpacket;
>> break;
>> }
>> ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
>> @@ -622,7 +626,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
>> req->complete = source_sink_complete;
>> if (is_in)
>> reinit_write_data(ep, req);
>> - else if (pattern != 2)
>> + else if (ss->pattern != 2)
>> memset(req->buf, 0x55, req->length);
>>
>> status = usb_ep_queue(ep, req, GFP_ATOMIC);
>> @@ -859,12 +863,12 @@ static struct usb_function *source_sink_alloc_func(
>> ss_opts->refcnt++;
>> mutex_unlock(&ss_opts->lock);
>>
>> - pattern = ss_opts->pattern;
>> - isoc_interval = ss_opts->isoc_interval;
>> - isoc_maxpacket = ss_opts->isoc_maxpacket;
>> - isoc_mult = ss_opts->isoc_mult;
>> - isoc_maxburst = ss_opts->isoc_maxburst;
>> - buflen = ss_opts->bulk_buflen;
>> + ss->pattern = ss_opts->pattern;
>> + ss->isoc_interval = ss_opts->isoc_interval;
>> + ss->isoc_maxpacket = ss_opts->isoc_maxpacket;
>> + ss->isoc_mult = ss_opts->isoc_mult;
>> + ss->isoc_maxburst = ss_opts->isoc_maxburst;
>> + ss->buflen = ss_opts->bulk_buflen;
>>
>> ss->function.name = "source/sink";
>> ss->function.bind = sourcesink_bind;
>> --
>> 1.9.1
>>
>
--
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/
Powered by blists - more mailing lists