linux-kernel - Re: [PATCH v4 14/14] HID: hid-multitouch: forwards MSC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121116200926.GA652@polaris.bitmath.org>
Date:	Fri, 16 Nov 2012 21:09:26 +0100
From:	"Henrik Rydberg" <rydberg@...omail.se>
To:	Benjamin Tissoires <benjamin.tissoires@...il.com>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 14/14] HID: hid-multitouch: forwards MSC_TIMESTAMP

Hi Benjamin,

> > This was not what I envisioned from the discussion yesterday, I was
> > probably too vague. Firstly, the absolute time interval checked seems
> > to be 500 ms, which is arbitrary. Second, the wrap should probably use
> 
> Not exactly. The time interval was 5s, near enough the minimum requirement for this field (6.5535 s).

Sorry about that, but the argument remains; it should depend on logical_maximum.

> > (logical_maximum + 1). Third, we are still not sure whether we should
> > take the 'time = 0 means reset' logic literally, I think it needs to
> > be tested.
> 
> Again, the device I have never does any reset at the first touch, it just wraps the counter.
> The problem is that when there is no event, we now that no events occurred since the previous timestamp, modulus 6.5535 seconds.

So the very first win8 device we test already diverges from the win8
specification, how nice.

Ok, you have conviced me that comparing to a proper time makes
sense. However, I am not happy about using a separate time for this,
given that we will fill in the event time later in the
chain.

In addition, it would make perfect sense to extend the validity of the
hardware time with the event time for longer intervals. The relative
error only makes a difference on milisecond intervals.

A patch that seamlessly extends the validity of the hardware time,
ideally using the event time, seems like a viable solution.

> > +static void mt_compute_timestamp(struct mt_device *td, struct hid_field *field,
> > +				 unsigned value)
> > +{
> > +	unsigned dt;
> > +
> > +	if (value) {
> > +		dt = (value - td->dev_time) % (field->logical_maximum + 1);
> 
> The curious thing is that this is not working on my kernel:

Oups, I suspect (value - td->dev_time) needs to be explicitly converted to unsigned.

> I don't understand the meaning of adding !td->timestamp. :/

With the definition that timestamp == 0 means an unknown interval, we
do not want to send that value by accident.

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