Re: [PATCH v5 1/3] usb: gadget: f_fs: fix the redundant ep files problem
On 08/24/2014 04:14 PM, Michal Nazarewicz wrote:
On Thu, Aug 21 2014, Robert Baldyga r.bald...@samsung.com wrote:
Up to now, when endpoint addresses in descriptors were non-consecutive,
there were created redundant files, which could cause problems in kernel,
when user tryed to read/write to them. It was result of fact that maximum
^ -- tried
endpoint address was taken as total number of endpoints in funciton.
This patch adds endpoint descriptors counting and storing their addresses
in eps_addrmap to verify their cohesion in each speed.
Endpoint address map would be also useful for further features, just like
vitual endpoint address mapping.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
@@ -1853,14 +1860,23 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
* Strings are indexed from 1 (0 is magic ;) reserved
* for languages list or some such)
*/
-if (*valuep ffs-strings_count)
-ffs-strings_count = *valuep;
+if (*valuep helper-ffs-strings_count)
+helper-ffs-strings_count = *valuep;
break;
case FFS_ENDPOINT:
-/* Endpoints are indexed from 1 as well. */
-if ((*valuep USB_ENDPOINT_NUMBER_MASK) ffs-eps_count)
-ffs-eps_count = (*valuep USB_ENDPOINT_NUMBER_MASK);
+d = (void *)desc;
+helper-eps_count++;
+if (helper-eps_count = 15)
+return -EINVAL;
+if (!helper-ffs-eps_count !helper-ffs-interfaces_count)
I'd check helper-ffs-eps_count only. helper-ffs-interfaces_count is
zero only because endpoints and interfaces are interpret at the same
time, but otherwise the interfaces_count is unrelated to what we're
doing here.
Besides simple descriptor counting there are done checks if endpoints
number and their addressed are identical for each speed. For this reason
we need to know inside this function if descriptors for any speed was
already done. If it's true, we check if endpoint descriptors for current
speed has proper addresses.
There is possibility that interface has no endpoints (endpoint 0 only),
so we can recognize that descriptors for one speed were already parsed
only basing on interfaces_count. In that case if FFS_ENDPOINT will
appear, eps_count will exceed number of endpoints in previously parsed
descriptors and error will be returned.
Also, perhaps adding a comment describing what !helper-ffs-eps_count
means makes sense here.
+helper-ffs-eps_addrmap[helper-eps_count] =
+d-bEndpointAddress;
+else if (helper-ffs-eps_count helper-eps_count)
+return -EINVAL;
Doesn't this duplicate check in __ffs_data_got_descs?
+else if (helper-ffs-eps_addrmap[helper-eps_count] !=
+d-bEndpointAddress)
+return -EINVAL;
break;
}
@@ -2101,13 +2118,29 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
/* Read descriptors */
raw_descs = data;
+helper.ffs = ffs;
for (i = 0; i 3; ++i) {
if (!counts[i])
continue;
+helper.interfaces_count = 0;
+helper.eps_count = 0;
ret = ffs_do_descs(counts[i], data, len,
- __ffs_data_do_entity, ffs);
+ __ffs_data_do_entity, helper);
if (ret 0)
goto error;
+if (!ffs-eps_count !ffs-interfaces_count) {
+ffs-eps_count = helper.eps_count;
+ffs-interfaces_count = helper.interfaces_count;
+} else {
+if (ffs-eps_count != helper.eps_count) {
+ret = -EINVAL;
+goto error;
+}
+if (ffs-interfaces_count != helper.interfaces_count) {
+ret = -EINVAL;
+goto error;
+}
+}
data += ret;
len -= ret;
}
@@ -2342,9 +2375,18 @@ static void ffs_event_add(struct ffs_data *ffs,
spin_unlock_irqrestore(ffs-ev.waitq.lock, flags);
}
-
/* Bind/unbind USB function hooks
***/
+static int ffs_ep_addr2idx(struct ffs_data *ffs, u8 endpoint_address)
+{
+int i;
+
+for (i = 1; i 15; ++i)
+ for (i = 1; i ARRAY_SIZE(ffs-eps_addrmap); ++i)
+if (ffs-eps_addrmap[i] == endpoint_address)
+return i;
+return -ENOENT;
+}
+
static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
struct usb_descriptor_header *desc,
Re: [PATCH v5 1/3] usb: gadget: f_fs: fix the redundant ep files problem
On Thu, Aug 21 2014, Robert Baldyga r.bald...@samsung.com wrote:
Up to now, when endpoint addresses in descriptors were non-consecutive,
there were created redundant files, which could cause problems in kernel,
when user tryed to read/write to them. It was result of fact that maximum
^ -- tried
endpoint address was taken as total number of endpoints in funciton.
This patch adds endpoint descriptors counting and storing their addresses
in eps_addrmap to verify their cohesion in each speed.
Endpoint address map would be also useful for further features, just like
vitual endpoint address mapping.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
@@ -1853,14 +1860,23 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
* Strings are indexed from 1 (0 is magic ;) reserved
* for languages list or some such)
*/
- if (*valuep ffs-strings_count)
- ffs-strings_count = *valuep;
+ if (*valuep helper-ffs-strings_count)
+ helper-ffs-strings_count = *valuep;
break;
case FFS_ENDPOINT:
- /* Endpoints are indexed from 1 as well. */
- if ((*valuep USB_ENDPOINT_NUMBER_MASK) ffs-eps_count)
- ffs-eps_count = (*valuep USB_ENDPOINT_NUMBER_MASK);
+ d = (void *)desc;
+ helper-eps_count++;
+ if (helper-eps_count = 15)
+ return -EINVAL;
+ if (!helper-ffs-eps_count !helper-ffs-interfaces_count)
I'd check helper-ffs-eps_count only. helper-ffs-interfaces_count is
zero only because endpoints and interfaces are interpret at the same
time, but otherwise the interfaces_count is unrelated to what we're
doing here.
Also, perhaps adding a comment describing what !helper-ffs-eps_count
means makes sense here.
+ helper-ffs-eps_addrmap[helper-eps_count] =
+ d-bEndpointAddress;
+ else if (helper-ffs-eps_count helper-eps_count)
+ return -EINVAL;
Doesn't this duplicate check in __ffs_data_got_descs?
+ else if (helper-ffs-eps_addrmap[helper-eps_count] !=
+ d-bEndpointAddress)
+ return -EINVAL;
break;
}
@@ -2101,13 +2118,29 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
/* Read descriptors */
raw_descs = data;
+ helper.ffs = ffs;
for (i = 0; i 3; ++i) {
if (!counts[i])
continue;
+ helper.interfaces_count = 0;
+ helper.eps_count = 0;
ret = ffs_do_descs(counts[i], data, len,
-__ffs_data_do_entity, ffs);
+__ffs_data_do_entity, helper);
if (ret 0)
goto error;
+ if (!ffs-eps_count !ffs-interfaces_count) {
+ ffs-eps_count = helper.eps_count;
+ ffs-interfaces_count = helper.interfaces_count;
+ } else {
+ if (ffs-eps_count != helper.eps_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ if (ffs-interfaces_count != helper.interfaces_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ }
data += ret;
len -= ret;
}
@@ -2342,9 +2375,18 @@ static void ffs_event_add(struct ffs_data *ffs,
spin_unlock_irqrestore(ffs-ev.waitq.lock, flags);
}
-
/* Bind/unbind USB function hooks
***/
+static int ffs_ep_addr2idx(struct ffs_data *ffs, u8 endpoint_address)
+{
+ int i;
+
+ for (i = 1; i 15; ++i)
+ for (i = 1; i ARRAY_SIZE(ffs-eps_addrmap); ++i)
+ if (ffs-eps_addrmap[i] == endpoint_address)
+ return i;
+ return -ENOENT;
+}
+
static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
struct usb_descriptor_header *desc,
void *priv)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
