Hi,
On Mon, Feb 23, 2015 at 9:02 AM, Andrzej Pietrasiewicz
<andrzej.p@xxxxxxxxxxx> wrote:
> Non-standard requests can encode the actual interface number in a
> non-standard way. For example composite_setup() assumes
> that it is w_index && 0xFF, but the printer function encodes the interface
> number in a context-dependet way (either w_index or w_index >> 8).
> This can lead to such requests being directed to wrong functions.
>
> This patch adds req_match() method to usb_function. Its purpose is to
> verify that a given request can be handled by a given function.
> If any function within a configuration provides the method and it returns
> true, then it is assumed that the right function is found.
>
> If a function uses req_match(), it should try as hard as possible to
> determine if the request is meant for it.
>
> If no functions in a configuration provide req_match or none of them
> returns true, then fall back to the usual approach.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/composite.c | 7 ++++++-
> include/linux/usb/composite.h | 3 +++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 9fb9231..07cee80 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1758,6 +1758,11 @@ unknown:
> * take such requests too, if that's ever needed: to work
> * in config 0, etc.
> */
> + list_for_each_entry(f, &cdev->config->functions, list)
> + if (f->req_match && f->req_match(f, ctrl))
> + break;
In this loop, if f->req_match is NULL, or f->req_match() returns
false, f becomes non-NULL at the end of the loop, which causes kernel
panic later.
> + if (&f->list != &cdev->config->functions)
> + goto try_fun_setup;
The following change fixes it.
+ list_for_each_entry(f, &cdev->config->functions, list)
+ if (f->req_match && f->req_match(f, ctrl))
+ goto try_fun_setup;
+
+ f = NULL;
+
Regards,
-Bin.
> switch (ctrl->bRequestType & USB_RECIP_MASK) {
> case USB_RECIP_INTERFACE:
> if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
> @@ -1775,7 +1780,7 @@ unknown:
> f = NULL;
> break;
> }
> -
> +try_fun_setup:
> if (f && f->setup)
> value = f->setup(f, ctrl);
> else {
> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> index 3d87def..51f477a 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -147,6 +147,7 @@ struct usb_os_desc_table {
> * then only altsetting zero is supported.
> * @disable: (REQUIRED) Indicates the function should be disabled. Reasons
> * include host resetting or reconfiguring the gadget, and disconnection.
> + * @req_match: Tests if a given class request can be handled by this function.
> * @setup: Used for interface-specific control requests.
> * @suspend: Notifies functions when the host stops sending USB traffic.
> * @resume: Notifies functions when the host restarts USB traffic.
> @@ -211,6 +212,8 @@ struct usb_function {
> int (*get_alt)(struct usb_function *,
> unsigned interface);
> void (*disable)(struct usb_function *);
> + bool (*req_match)(struct usb_function *,
> + const struct usb_ctrlrequest *);
> int (*setup)(struct usb_function *,
> const struct usb_ctrlrequest *);
> void (*suspend)(struct usb_function *);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
