* [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
@ 2022-01-26 20:52 Jann Horn
2022-01-26 22:12 ` Andrey Konovalov
0 siblings, 1 reply; 6+ messages in thread
From: Jann Horn @ 2022-01-26 20:52 UTC (permalink / raw)
To: Andrey Konovalov, Felipe Balbi
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Jann Horn
Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
But with some real hardware, there are endpoints that support both IN and
OUT. In particular, the PLX 2380 has four available endpoints that each
support both IN and OUT.
raw-gadget currently gets confused and thinks that any endpoint that is
usable as an IN endpoint can never be used as an OUT endpoint.
Fix it by looking at the direction in the configured endpoint descriptor
instead of looking at the hardware capabilities.
With this change, I can use the PLX 2380 with raw-gadget.
Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
Signed-off-by: Jann Horn <jannh@google.com>
---
drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index c5a2c734234a..d86c3a36441e 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
ret = -EBUSY;
goto out_unlock;
}
- if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
+ if (in != usb_endpoint_dir_in(ep->ep->desc)) {
dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
ret = -EINVAL;
goto out_unlock;
base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
2022-01-26 20:52 [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints Jann Horn
@ 2022-01-26 22:12 ` Andrey Konovalov
2022-01-26 22:30 ` Jann Horn
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Konovalov @ 2022-01-26 22:12 UTC (permalink / raw)
To: Jann Horn; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB list, LKML
On Wed, Jan 26, 2022 at 9:52 PM Jann Horn <jannh@google.com> wrote:
>
> Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
> But with some real hardware, there are endpoints that support both IN and
> OUT. In particular, the PLX 2380 has four available endpoints that each
> support both IN and OUT.
>
> raw-gadget currently gets confused and thinks that any endpoint that is
> usable as an IN endpoint can never be used as an OUT endpoint.
>
> Fix it by looking at the direction in the configured endpoint descriptor
> instead of looking at the hardware capabilities.
>
> With this change, I can use the PLX 2380 with raw-gadget.
>
> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> index c5a2c734234a..d86c3a36441e 100644
> --- a/drivers/usb/gadget/legacy/raw_gadget.c
> +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> @@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> ret = -EBUSY;
> goto out_unlock;
> }
> - if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
> + if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> ret = -EINVAL;
> goto out_unlock;
>
> base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
Awesome! Thanks for finding this!
What do you think about using
if ((in && !ep->ep->caps.dir_in) || (!in && !ep->ep->caps.dir_out))
instead?
It looks less cryptic: if (in and no in caps) or (out and no out caps) => fail.
FTR, I tested both changes with the net2280 hardware that I have, and
they work. At least gadget.c from the test suite enumerates properly.
Most of the tests still fail. But with g_zero, even more of them fail.
P.S. I wonder why everyone suddenly got interested in Raw Gadget lately? :)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
2022-01-26 22:12 ` Andrey Konovalov
@ 2022-01-26 22:30 ` Jann Horn
2022-01-26 22:37 ` Andrey Konovalov
0 siblings, 1 reply; 6+ messages in thread
From: Jann Horn @ 2022-01-26 22:30 UTC (permalink / raw)
To: Andrey Konovalov; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB list, LKML
On Wed, Jan 26, 2022 at 11:12 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> On Wed, Jan 26, 2022 at 9:52 PM Jann Horn <jannh@google.com> wrote:
> >
> > Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
> > But with some real hardware, there are endpoints that support both IN and
> > OUT. In particular, the PLX 2380 has four available endpoints that each
> > support both IN and OUT.
> >
> > raw-gadget currently gets confused and thinks that any endpoint that is
> > usable as an IN endpoint can never be used as an OUT endpoint.
> >
> > Fix it by looking at the direction in the configured endpoint descriptor
> > instead of looking at the hardware capabilities.
> >
> > With this change, I can use the PLX 2380 with raw-gadget.
> >
> > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> > index c5a2c734234a..d86c3a36441e 100644
> > --- a/drivers/usb/gadget/legacy/raw_gadget.c
> > +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> > @@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> > ret = -EBUSY;
> > goto out_unlock;
> > }
> > - if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
> > + if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> > dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> > ret = -EINVAL;
> > goto out_unlock;
> >
> > base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
> > --
> > 2.35.0.rc0.227.g00780c9af4-goog
> >
>
> Awesome! Thanks for finding this!
>
> What do you think about using
>
> if ((in && !ep->ep->caps.dir_in) || (!in && !ep->ep->caps.dir_out))
>
> instead?
>
> It looks less cryptic: if (in and no in caps) or (out and no out caps) => fail.
That's also semantically different, right?
As I understand it, what we should be checking here is whether the
direction of the request matches the direction previously specified in
USB_RAW_IOCTL_EP_ENABLE, not whether the hardware would be capable of
using the endpoint in the requested direction if it had been
configured for that direction?
But I might also be misunderstanding what's going on - it's not like
I've looked at a spec for this or anything like that, I'm just kinda
guessing...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
2022-01-26 22:30 ` Jann Horn
@ 2022-01-26 22:37 ` Andrey Konovalov
2022-01-30 21:58 ` Andrey Konovalov
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Konovalov @ 2022-01-26 22:37 UTC (permalink / raw)
To: Jann Horn; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB list, LKML
On Wed, Jan 26, 2022 at 11:31 PM Jann Horn <jannh@google.com> wrote:
>
> On Wed, Jan 26, 2022 at 11:12 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> > On Wed, Jan 26, 2022 at 9:52 PM Jann Horn <jannh@google.com> wrote:
> > >
> > > Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
> > > But with some real hardware, there are endpoints that support both IN and
> > > OUT. In particular, the PLX 2380 has four available endpoints that each
> > > support both IN and OUT.
> > >
> > > raw-gadget currently gets confused and thinks that any endpoint that is
> > > usable as an IN endpoint can never be used as an OUT endpoint.
> > >
> > > Fix it by looking at the direction in the configured endpoint descriptor
> > > instead of looking at the hardware capabilities.
> > >
> > > With this change, I can use the PLX 2380 with raw-gadget.
> > >
> > > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> > > Signed-off-by: Jann Horn <jannh@google.com>
> > > ---
> > > drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> > > index c5a2c734234a..d86c3a36441e 100644
> > > --- a/drivers/usb/gadget/legacy/raw_gadget.c
> > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> > > @@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> > > ret = -EBUSY;
> > > goto out_unlock;
> > > }
> > > - if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
> > > + if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> > > dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> > > ret = -EINVAL;
> > > goto out_unlock;
> > >
> > > base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
> > > --
> > > 2.35.0.rc0.227.g00780c9af4-goog
> > >
> >
> > Awesome! Thanks for finding this!
> >
> > What do you think about using
> >
> > if ((in && !ep->ep->caps.dir_in) || (!in && !ep->ep->caps.dir_out))
> >
> > instead?
> >
> > It looks less cryptic: if (in and no in caps) or (out and no out caps) => fail.
>
> That's also semantically different, right?
> As I understand it, what we should be checking here is whether the
> direction of the request matches the direction previously specified in
> USB_RAW_IOCTL_EP_ENABLE, not whether the hardware would be capable of
> using the endpoint in the requested direction if it had been
> configured for that direction?
> But I might also be misunderstanding what's going on - it's not like
> I've looked at a spec for this or anything like that, I'm just kinda
> guessing...
Ah, yes, you are right. We already checked caps via
usb_gadget_ep_match_desc() in EP_ENABLE. So here it makes sense to
check that the request direction matches the one in the descriptor.
Your original patch makes sense.
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Tested-by: Andrey Konovalov <andreyknvl@gmail.com>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
2022-01-26 22:37 ` Andrey Konovalov
@ 2022-01-30 21:58 ` Andrey Konovalov
2022-01-31 13:20 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Konovalov @ 2022-01-30 21:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Felipe Balbi, USB list, LKML, Jann Horn, stable
On Wed, Jan 26, 2022 at 11:37 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Wed, Jan 26, 2022 at 11:31 PM Jann Horn <jannh@google.com> wrote:
> >
> > On Wed, Jan 26, 2022 at 11:12 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> > > On Wed, Jan 26, 2022 at 9:52 PM Jann Horn <jannh@google.com> wrote:
> > > >
> > > > Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
> > > > But with some real hardware, there are endpoints that support both IN and
> > > > OUT. In particular, the PLX 2380 has four available endpoints that each
> > > > support both IN and OUT.
> > > >
> > > > raw-gadget currently gets confused and thinks that any endpoint that is
> > > > usable as an IN endpoint can never be used as an OUT endpoint.
> > > >
> > > > Fix it by looking at the direction in the configured endpoint descriptor
> > > > instead of looking at the hardware capabilities.
> > > >
> > > > With this change, I can use the PLX 2380 with raw-gadget.
> > > >
> > > > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> > > > Signed-off-by: Jann Horn <jannh@google.com>
> > > > ---
> > > > drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> > > > index c5a2c734234a..d86c3a36441e 100644
> > > > --- a/drivers/usb/gadget/legacy/raw_gadget.c
> > > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> > > > @@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> > > > ret = -EBUSY;
> > > > goto out_unlock;
> > > > }
> > > > - if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
> > > > + if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> > > > dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> > > > ret = -EINVAL;
> > > > goto out_unlock;
> > > >
> > > > base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
> > > > --
> > > > 2.35.0.rc0.227.g00780c9af4-goog
> > > >
>
> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> Tested-by: Andrey Konovalov <andreyknvl@gmail.com>
Greg, could you PTAL and pick this up?
It also makes sense to include this fix into stable kernels.
Cc: <stable@vger.kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints
2022-01-30 21:58 ` Andrey Konovalov
@ 2022-01-31 13:20 ` Greg Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2022-01-31 13:20 UTC (permalink / raw)
To: Andrey Konovalov; +Cc: Felipe Balbi, USB list, LKML, Jann Horn, stable
On Sun, Jan 30, 2022 at 10:58:42PM +0100, Andrey Konovalov wrote:
> On Wed, Jan 26, 2022 at 11:37 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> >
> > On Wed, Jan 26, 2022 at 11:31 PM Jann Horn <jannh@google.com> wrote:
> > >
> > > On Wed, Jan 26, 2022 at 11:12 PM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> > > > On Wed, Jan 26, 2022 at 9:52 PM Jann Horn <jannh@google.com> wrote:
> > > > >
> > > > > Under dummy_hcd, every available endpoint is *either* IN or OUT capable.
> > > > > But with some real hardware, there are endpoints that support both IN and
> > > > > OUT. In particular, the PLX 2380 has four available endpoints that each
> > > > > support both IN and OUT.
> > > > >
> > > > > raw-gadget currently gets confused and thinks that any endpoint that is
> > > > > usable as an IN endpoint can never be used as an OUT endpoint.
> > > > >
> > > > > Fix it by looking at the direction in the configured endpoint descriptor
> > > > > instead of looking at the hardware capabilities.
> > > > >
> > > > > With this change, I can use the PLX 2380 with raw-gadget.
> > > > >
> > > > > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> > > > > Signed-off-by: Jann Horn <jannh@google.com>
> > > > > ---
> > > > > drivers/usb/gadget/legacy/raw_gadget.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> > > > > index c5a2c734234a..d86c3a36441e 100644
> > > > > --- a/drivers/usb/gadget/legacy/raw_gadget.c
> > > > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> > > > > @@ -1004,7 +1004,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> > > > > ret = -EBUSY;
> > > > > goto out_unlock;
> > > > > }
> > > > > - if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
> > > > > + if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> > > > > dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> > > > > ret = -EINVAL;
> > > > > goto out_unlock;
> > > > >
> > > > > base-commit: 0280e3c58f92b2fe0e8fbbdf8d386449168de4a8
> > > > > --
> > > > > 2.35.0.rc0.227.g00780c9af4-goog
> > > > >
> >
> > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> > Tested-by: Andrey Konovalov <andreyknvl@gmail.com>
>
> Greg, could you PTAL and pick this up?
>
> It also makes sense to include this fix into stable kernels.
>
> Cc: <stable@vger.kernel.org>
Now picked up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-31 13:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 20:52 [PATCH] usb: raw-gadget: fix handling of dual-direction-capable endpoints Jann Horn
2022-01-26 22:12 ` Andrey Konovalov
2022-01-26 22:30 ` Jann Horn
2022-01-26 22:37 ` Andrey Konovalov
2022-01-30 21:58 ` Andrey Konovalov
2022-01-31 13:20 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).