Hi Hans,
Thanks for the quick review.
See answers below
On Wed, 2015-07-08 at 15:45 +0200, Hans de Goede wrote:
> Hi,
>
> On 08-07-15 15:26, Uri Lublin wrote:
> > See usbredirfilter.h for when interfaces are skipped.
> >
> > Fixes rhbz#1179210
>
> Sigh, the kvm in question is really messed up as it uses non
> bootclass hid interfaces,
> that means it won't work in many BIOS' etc. What a mess ...
>
> > ---
> > usbredirparser/usbredirfilter.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/usbredirparser/usbredirfilter.c
> > b/usbredirparser/usbredirfilter.c
> > index ef9c63a..5cbbfbf 100644
> > --- a/usbredirparser/usbredirfilter.c
> > +++ b/usbredirparser/usbredirfilter.c
> > @@ -172,7 +172,7 @@ int usbredirfilter_check(
> > uint16_t vendor_id, uint16_t product_id, uint16_t
> > device_version_bcd,
> > int flags)
> > {
> > - int i, rc;
> > + int i, rc, num_skipped=0;
> >
> > if (usbredirfilter_verify(rules, rules_count))
> > return -EINVAL;
> > @@ -190,9 +190,10 @@ int usbredirfilter_check(
> > for (i = 0; i < interface_count; i++) {
> > if (!(flags & usbredirfilter_fl_dont_skip_non_boot_hid)
> > &&
> > interface_count > 1 && interface_class[i] == 0x03
> > &&
> > - interface_subclass[i] == 0x00 &&
> > interface_protocol[i] == 0x00)
> > + interface_subclass[i] == 0x00 &&
> > interface_protocol[i] == 0x00) {
> > + num_skipped++;
> > continue;
> > -
> > + }
> > rc = usbredirfilter_check1(rules, rules_count,
> > interface_class[i],
> > vendor_id, product_id,
> > device_version_bcd,
> > flags &
> > usbredirfilter_fl_default_allow);
> > @@ -200,6 +201,10 @@ int usbredirfilter_check(
> > return rc;
> > }
> >
> > + /* If all interfaces were skipped, block the device */
> > + if (num_skipped == interface_count)
> > + return -ENOENT;
> > +
> > return 0;
> > }
>
> This seems wrong, this means that if a user wants to redirect some
> custom hid device,
> with just a single hid interface that it will always be blocked
> because of this.
In that case interface_count==1 and it will not be skipped.
If that's not enough, see more below.
>
> I suggest instead adding a vid/pid based list of devices on which to
> not skip non
> boot compliant hid devices. This way if hid devices are allowed to be
> redirected
> by the filter, the device can still be redirected, and in the default
> case where
> hid devices are not allowed, we will skip the non-boot-hid skipping,
> do the regular
> hid check, and block the device based on that.
That may work but I think it is complicated for users.
If they are going to use specific vid/pid, they might as
well add a filter-rule for those specific devices.
Also assume the filter used is block-all-devices
(-1,-1,-1,-1, 0). This will not be enough and the user
would have to provide the specific vid/uid list of such
weird devices (after failing the first time).
An alternative is to call usbredirfilter_check1 also
for cases that are skipped now, and pass an additional parameter
that means do-not-check-class.
That way a rule that allow/block a specific vid/pid will apply.
Thanks,
Uri
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel