Re: Linux USB HID should ignore values outside Logical Minimum/Maximumrange

[Jiri Kosina]

On Fri, 28 Oct 2011, Denilson Figueiredo de Sá wrote:

> > On the other hand I feel like just dropping the report on the floor should
> > be the proper thing to do.
> 
> I'd say not the entire report, but just the invalid fields.

Right, which is what my patch actually does, just my fingers on keyboard 
were faster than my brain while writing that.

> The HID specification (HID1_11.pdf) talks about "Null Values" at the section
> 5.10 (page 20).
> 
> [quote]
> HID class devices support the ability to ignore selected fields in a 
> report at run- time. This is accomplished by declaring bit field in a 
> report that is capable of containing a range of values larger than those 
> actually generated by the control. If the host or the device receives an 
> out-of-range value then the current value for the respective control 
> will not be modified.
> [/quote]
> 
> (sidenote: Why didn't I mention this spec earlier? Because I knew I have 
> read that information somewhere, but didn't find it again until today.)

I personally find the wording of the spec here a bit unfortunate (the 
'declaring bit field in a report that is capable of containing a range of 
values largen than those actually generated by the control' seems to be a 
bit too foggy and vague).

> > Christoph, how about the (untested at my side yet) patch below? It'd be
> > nice if you could test with the device you are seeing the problem with and
> > let me know.
> 
> I can't test it *right now*, but I can probably test it in a week or two.

Thanks. No real rush here, as I will definitely not be queuing this for 
3.2 anyway, 3.3 is the first release I'd consider this for.

> > +	/* Ignore absolute values that are out of bounds */
> > +	if ((usage->type == EV_ABS && (value < field->logical_minimum ||
> > +					value > field->logical_maximum))) {
> 
> Given what the HID spec says (as I quoted above), I'd change this condition to
> just:
> 
> > +	if (value < field->logical_minimum ||
> > +					value > field->logical_maximum) {

After thinking about it a little bit more, I think I agree.

I will still try to evaluate this against a number of devices I have (or I 
have reports for), to see if we potentially really can't break something.

So please let me know the result of your testing.

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/